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

[sashiko-bot@kernel.org]

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