[PATCH] drm/gud: validate TV mode names before creating enum property

[PATCH] drm/gud: validate TV mode names before creating enum property

[Tao Yu]

The GUD protocol returns TV mode names as fixed-size
GUD_CONNECTOR_TV_MODE_NAME_LEN entries and requires each name to be
NUL-terminated.

gud_connector_add_tv_mode() currently passes each fixed-size entry
directly to drm_mode_create_tv_properties_legacy(), which eventually
reaches drm_property_add_enum() and strlen(). If a device returns an
entry without a terminating NUL byte, strlen() reads past the end of
the slot and can run beyond the allocated buffer, triggering an
out-of-bounds read.

Validate that each returned TV mode name contains a NUL terminator
within its fixed-size slot before passing it to the DRM property code.
If a malformed entry is found, reject the device response with -EIO.

This fixes the out-of-bounds read without changing the handling of
valid devices, and avoids silently truncating malformed protocol data.

Reported-by: syzbot+9ae8e7884e451eaed5b4@syzkaller.appspotmail.com
Fixes: 40e1a70b4aed ("drm: Add GUD USB Display driver")
Signed-off-by: Tao Yu <tao1.yu@intel.com>
---
 drivers/gpu/drm/gud/gud_connector.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c
index ea0cca58b7c89..8141c3a1e30ad 100644
--- a/drivers/gpu/drm/gud/gud_connector.c
+++ b/drivers/gpu/drm/gud/gud_connector.c
@@ -396,8 +396,16 @@ 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];
+
+		if (!memchr(mode, '\0', GUD_CONNECTOR_TV_MODE_NAME_LEN)) {
+			ret = -EIO;
+			goto free;
+		}
+
+		modes[i] = mode;
+	}
 
 	ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes);
 free:
-- 
2.34.1

Re: [PATCH] drm/gud: validate TV mode names before creating enum property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A malicious or non-compliant USB device can cause a kernel NULL pointer dereference in `drm_object_attach_property` by ordering TV properties such that `GUD_PROPERTY_TV_MODE` is returned after other TV properties.
--

--- Patch [1]: [PATCH] drm/gud: validate TV mode names before creating enum property ---
commit db9280e75e6e04eacd06127a0240a5d76af230f9
Author: Tao Yu <tao1.yu@intel.com>

drm/gud: validate TV mode names before creating enum property

The GUD protocol returns TV mode names as fixed-size entries that require
NUL-termination. This commit validates that each returned TV mode name
contains a NUL terminator within its fixed-size slot before passing it to
the DRM property code, preventing an out-of-bounds read in strlen().

> diff --git a/drivers/gpu/drm/gud/gud_connector.c b/drivers/gpu/drm/gud/gud_connector.c
> index ea0cca58b7c89..8141c3a1e30ad 100644
> --- a/drivers/gpu/drm/gud/gud_connector.c
> +++ b/drivers/gpu/drm/gud/gud_connector.c
> @@ -396,8 +396,16 @@ 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];
> +
> +		if (!memchr(mode, '\0', GUD_CONNECTOR_TV_MODE_NAME_LEN)) {
> +			ret = -EIO;
> +			goto free;
> +		}
> +
> +		modes[i] = mode;
> +	}
>  
>  	ret = drm_mode_create_tv_properties_legacy(connector->dev, num_modes, modes);

[Severity: High]
This is a pre-existing issue, but can a malicious or non-compliant USB device
cause a kernel NULL pointer dereference in drm_object_attach_property() by
ordering TV properties such that GUD_PROPERTY_TV_MODE is returned after other
TV properties?

If a USB device returns GUD_PROPERTY_TV_HUE (or similar TV properties) before
GUD_PROPERTY_TV_MODE, gud_connector_add_properties() processes it and calls
drm_mode_create_tv_properties_legacy(drm, 0, NULL). This initializes
dev->mode_config.tv_select_subconnector_property but leaves
legacy_tv_mode_property as NULL since num_modes is 0.

Later, when GUD_PROPERTY_TV_MODE is processed, gud_connector_add_tv_mode()
calls drm_mode_create_tv_properties_legacy() here with actual modes. Because
tv_select_subconnector_property is already set, this second call hits an
early return in drm_mode_create_tv_properties_legacy():

drivers/gpu/drm/drm_connector.c:drm_mode_create_tv_properties_legacy() {
	if (dev->mode_config.tv_select_subconnector_property)
		return 0;
...

This immediately succeeds without creating legacy_tv_mode_property.
gud_connector_property_lookup() is then called for GUD_PROPERTY_TV_MODE and
returns the uninitialized config->legacy_tv_mode_property, which is NULL.

In gud_connector_add_properties():

	property = gud_connector_property_lookup(connector, prop);
	if (drm_WARN_ON(drm, IS_ERR(property)))
		continue;

The IS_ERR(property) check fails to catch NULL, and
drm_object_attach_property() is called with the NULL property,
unconditionally dereferencing property->dev.

>  free:
>  	kfree(buf);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819072835.4074130-1-tao1.yu@intel.com?part=1

Re: [PATCH] drm/gud: validate TV mode names before creating enum property

[Tao Yu]

Thanks for the review.

This patch is intentionally limited to fixing the out-of-bounds read caused
by malformed TV mode names. The NULL-dereference risk caused by TV property
ordering is a separate pre-existing issue, and I am addressing it in a
separate follow-up patch.

Thanks,
Tao