[PATCH] drm/gud: NUL-terminate TV mode names read from the device

[PATCH] drm/gud: NUL-terminate TV mode names read from the device

[Deepanshu Kartikey]

gud_connector_add_tv_mode() reads a buffer of fixed-size mode names from
the USB device and passes pointers into it to
drm_mode_create_tv_properties_legacy(), which calls strlen() on each one.
Nothing guarantees the device NUL-terminates a name, so strlen() can run
past the end of a slot and, for the last mode, past the end of the
allocation.

Terminate each name at the end of its slot before use.

Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Reported-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=916c888ba5f1a54c9526
Tested-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/gpu/drm/gud/gud_connector.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c
index ea0cca58b7c8..5c0065c876a7 100644
--- a/drivers/gpu/drm/gud/gud_connector.c
+++ b/drivers/gpu/drm/gud/gud_connector.c
@@ -396,8 +396,13 @@ static int gud_connector_add_tv_mode(struct gud_device *gdrm, struct drm_connect
 	}
 
 	num_modes = ret / GUD_CONNECTOR_TV_MODE_NAME_LEN;
-	for (i = 0; i < num_modes; i++)
-		modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
+	for (i = 0; i < num_modes; i++) {
+		char *mode = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
+
+		/* The device is not trusted to NUL-terminate the name */
+		mode[GUD_CONNECTOR_TV_MODE_NAME_LEN - 1] = '\0';
+		modes[i] = mode;
+	}
 
 	ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes);
 free:
-- 
2.43.0

Re: [PATCH] drm/gud: NUL-terminate TV mode names read from the device

[Ruben Wauters]

[-- Attachment #1: Type: text/plain, Size: 4261 bytes --]

On Sun, 2026-08-16 at 09:00 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] A malicious or misconfigured USB device can cause a NULL pointer dereference and kernel panic in `drm_object_attach_property` by returning `GUD_PROPERTY_TV_MODE` after any other non-margin TV property (such as `GUD_PROPERTY_TV_BRIGHTNESS` or `GUD_PROPERTY_TV_HUE`).
> --
> 
> --- Patch [1]: [PATCH] drm/gud: NUL-terminate TV mode names read from the device ---
> commit 789868bcbf6cc6ddf0d3b74feab3a22914863ec8
> Author: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> drm/gud: NUL-terminate TV mode names read from the device
> 
> This commit fixes a potential out-of-bounds read by explicitly NUL-terminating
> TV mode names read from the USB device. This ensures strlen() won't run past
> the end of the allocation when passed to drm_mode_create_tv_properties_legacy().
> 
> Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
> Reported-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=916c888ba5f1a54c9526
> Tested-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> > diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c
> > index ea0cca58b7c89..5c0065c876a77 100644
> > --- a/drivers/gpu/drm/gud/gud_connector.c
> > +++ b/drivers/gpu/drm/gud/gud_connector.c
> > @@ -396,8 +396,13 @@ static int gud_connector_add_tv_mode(struct gud_device *gdrm, struct drm_connect
> >  	}
> >  
> >  	num_modes = ret / GUD_CONNECTOR_TV_MODE_NAME_LEN;
> > -	for (i = 0; i < num_modes; i++)
> > -		modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
> > +	for (i = 0; i < num_modes; i++) {
> > +		char *mode = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
> > +
> > +		/* The device is not trusted to NUL-terminate the name */
> > +		mode[GUD_CONNECTOR_TV_MODE_NAME_LEN - 1] = '\0';
> > +		modes[i] = mode;
> > +	}
> >  
> >  	ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes);
> 
> [Severity: High]
> This is a pre-existing issue, but could a malicious or misconfigured USB device
> cause a NULL pointer dereference and kernel panic by returning
> GUD_PROPERTY_TV_MODE after another non-margin TV property?
> 
> In gud_connector_add_properties(), properties from the USB device are processed
> sequentially. If a property like GUD_PROPERTY_TV_BRIGHTNESS is processed first,
> drm_mode_create_tv_properties_legacy() creates the baseline TV properties but
> leaves legacy_tv_mode_property as NULL since num_modes is 0.
> 
> If GUD_PROPERTY_TV_MODE is processed next, it calls gud_connector_add_tv_mode()
> which then calls drm_mode_create_tv_properties_legacy() here:
> 
> drivers/gpu/drm/drm_connector.c:drm_mode_create_tv_properties_legacy() {
>     ...
> 	if (dev->mode_config.tv_select_subconnector_property)
> 		return 0;
>     ...
> }
> 
> This early return prevents dev->mode_config.legacy_tv_mode_property from
> being created.
> 

Hello
> Then in gud_connector_add_properties(), gud_connector_property_lookup()
> will return NULL for legacy_tv_mode_property:
> 
> drivers/gpu/drm/gud/gud_connector.c:gud_connector_add_properties() {
>     ...
> 		property = gud_connector_property_lookup(connector, prop);
> 		if (drm_WARN_ON(drm, IS_ERR(property)))
> 			continue;
>     ...
> }
> 
It might be an idea to add a null check here, however, I am inclined to
accept this patch without it, as it does fix an issue with the current
code, and while this issue is related, it is somewhat separate. It
would be a good idea to have this as a separate patch instead.

Ruben Wauters

> Since IS_ERR() ignores NULL, the NULL property pointer bypasses this check and
> is passed to drm_object_attach_property(), which unconditionally dereferences
> it:
> 
> drivers/gpu/drm/drm_mode_object.c:drm_object_attach_property() {
>     ...
> 	struct drm_device *dev = property->dev;
>     ...
> }
> 
> Could this lead to a local denial of service?
> 
> >  free:
> >  	kfree(buf);
> >  	if (ret < 0)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

Re: [PATCH] drm/gud: NUL-terminate TV mode names read from the device

[Ruben Wauters]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

On Sun, 2026-08-16 at 14:22 +0530, Deepanshu Kartikey wrote:
> gud_connector_add_tv_mode() reads a buffer of fixed-size mode names from
> the USB device and passes pointers into it to
> drm_mode_create_tv_properties_legacy(), which calls strlen() on each one.
> Nothing guarantees the device NUL-terminates a name, so strlen() can run
> past the end of a slot and, for the last mode, past the end of the
> allocation.
> 
> Terminate each name at the end of its slot before use.
> 
> Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
> Reported-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=916c888ba5f1a54c9526
> Tested-by: syzbot+916c888ba5f1a54c9526@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
Acked-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/gpu/drm/gud/gud_connector.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c
> index ea0cca58b7c8..5c0065c876a7 100644
> --- a/drivers/gpu/drm/gud/gud_connector.c
> +++ b/drivers/gpu/drm/gud/gud_connector.c
> @@ -396,8 +396,13 @@ static int gud_connector_add_tv_mode(struct gud_device *gdrm, struct drm_connect
>  	}
>  
>  	num_modes = ret / GUD_CONNECTOR_TV_MODE_NAME_LEN;
> -	for (i = 0; i < num_modes; i++)
> -		modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
> +	for (i = 0; i < num_modes; i++) {
> +		char *mode = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
> +
> +		/* The device is not trusted to NUL-terminate the name */
> +		mode[GUD_CONNECTOR_TV_MODE_NAME_LEN - 1] = '\0';
> +		modes[i] = mode;
> +	}
>  
>  	ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes);
>  free:

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]