Skip to contentSkip to conversation

drm: Make sure that encoder_type for drm_encoder_init() is in the list

PATCH In review Jordan Crouse 2 participants 3 messages Jun 21, 2018 – Jun 22, 2018

Subsystems (2)
DRM drivers DRM misc
lore
Share & tools
lore

Jordan Crouse

author
#0 lore raw

PATCH 1/1 drm: Make sure that encoder_type for drm_encoder_init() is in the list

If a encoder name isn't specified for drm_encoder_init() it will try to construct one based on the incoming encoder_type identifier. If the caller passes an invalid encoder_type value the lookup could walk right past the end of the table.

Signed-off-by Jordan Crouse <[email protected]>
---
 drivers/gpu/drm/drm_encoder.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/drm_encoder.c +6 -0
index 273e1c59c54a..4cf6c3bb8503 100644
@@ -128,6 +128,12 @@ int drm_encoder_init(struct drm_device *dev,
 		encoder->name = kvasprintf(GFP_KERNEL, name, ap);
 		va_end(ap);
 	} else {
+		/* Make sure that the requested encoder type is in the list */
+		if (encoder_type >= ARRAY_SIZE(drm_encoder_enum_list)) {
+			ret = -EINVAL;
+			goto out_put;
+		}
+
 		encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
 					  drm_encoder_enum_list[encoder_type].name,
 					  encoder->base.id);
Signature · 6 lines
2.17.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
#1 lore raw
Quoted 15 lines · Jordan Crouse

If a encoder name isn't specified for drm_encoder_init() it will try to construct one based on the incoming encoder_type identifier. If the caller passes an invalid encoder_type value the lookup could walk right past the end of the table.

Signed-off-by Jordan Crouse <[email protected]>
---
 drivers/gpu/drm/drm_encoder.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/gpu/drm/drm_encoder.c +0 -0
index 273e1c59c54a..4cf6c3bb8503 100644

drivers/gpu/drm/drm_encoder.c int drm_encoder_init(struct drm_device *dev, · line 132

Quoted 5 lines
drivers/gpu/drm/drm_encoder.c +2 -0
@@ -128,6 +128,12 @@ int drm_encoder_init(struct drm_device *dev,
 		encoder->name = kvasprintf(GFP_KERNEL, name, ap);
 		va_end(ap);
 	} else {
+		/* Make sure that the requested encoder type is in the list */
+		if (encoder_type >= ARRAY_SIZE(drm_encoder_enum_list)) {

I think moving that to the top and wrapping it in a WARN_ON would be even better - specifying and invalid encoder type is clearly a driver bug. -Daniel

Quoted 14 lines
drivers/gpu/drm/drm_encoder.c +4 -0
+			ret = -EINVAL;
+			goto out_put;
+		}
+
 		encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
 					  drm_encoder_enum_list[encoder_type].name,
 					  encoder->base.id);
Signature · 6 lines
2.17.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
author
#2 lore raw
Quoted 11 lines · Daniel Vetter
Quoted 10 lines · Jordan Crouse

If a encoder name isn't specified for drm_encoder_init() it will try to construct one based on the incoming encoder_type identifier. If the caller passes an invalid encoder_type value the lookup could walk right past the end of the table.

Signed-off-by Jordan Crouse <[email protected]>
---
 drivers/gpu/drm/drm_encoder.c | 6 ++++++
 1 file changed, 6 insertions(+)
Quoted 14 lines

drivers/gpu/drm/drm_encoder.c int drm_encoder_init(struct drm_device *dev, · line 132

Quoted 10 lines
drivers/gpu/drm/drm_encoder.c +2 -0
index 273e1c59c54a..4cf6c3bb8503 100644
@@ -128,6 +128,12 @@ int drm_encoder_init(struct drm_device *dev,
 		encoder->name = kvasprintf(GFP_KERNEL, name, ap);
 		va_end(ap);
 	} else {
+		/* Make sure that the requested encoder type is in the list */
+		if (encoder_type >= ARRAY_SIZE(drm_encoder_enum_list)) {

I think moving that to the top and wrapping it in a WARN_ON would be even better - specifying and invalid encoder type is clearly a driver bug. -Daniel

It wasn't immediately clear to me if it was legal to pass a "custom" encoder type assuming you provided a name for it so I erred on the side of caution. I would have no problem moving it up to the top and being loud about it.

Jordan

Quoted 19 lines
Quoted 14 lines
drivers/gpu/drm/drm_encoder.c +4 -0
+			ret = -EINVAL;
+			goto out_put;
+		}
+
 		encoder->name = kasprintf(GFP_KERNEL, "%s-%d",
 					  drm_encoder_enum_list[encoder_type].name,
 					  encoder->base.id);
Signature · 6 lines
2.17.1

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel