Re: [PATCH v3 10/10] drm/nouveau: honor HF-EEODB EDIDs by converting to struct drm_edid

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

Pre-existing issues:
- [High] Potential out-of-bounds read when copying the EDID from a Device Tree property.
--

--- Patch [10]: [PATCH v3 10/10] drm/nouveau: honor HF-EEODB EDIDs by converting to struct drm_edid ---
commit a90c1b47260c1e8987750f3b4670510323a260f4
Author: Mohamed Ahmed <[email protected]>

drm/nouveau: honor HF-EEODB EDIDs by converting to struct drm_edid

This commit converts the nouveau driver to use the struct drm_edid API 
instead of the legacy EDID APIs. This allows the driver to properly honor 
HF-EEODB EDIDs by tracking their true block size instead of truncating them. 
The patch updates various EDID retrieval and parsing paths to use the new 
allocation and validation infrastructure.

> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index b0b0ad9a0c248..3738c23215c58 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -490,8 +520,17 @@ nouveau_connector_of_detect(struct drm_connector *connector)
>  		int idx = name ? name[strlen(name) - 1] - 'A' : 0;
>  
>  		if (nv_encoder->dcb->i2c_index == idx && edid) {
> -			nv_connector->edid =
> -				kmemdup(edid, EDID_LENGTH, GFP_KERNEL);
> +			const struct drm_edid *drm_edid =
> +				drm_edid_alloc(edid, EDID_LENGTH);

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the device
tree to provide an EDID property smaller than EDID_LENGTH?

In nouveau_connector_of_detect, of_get_property is called with a NULL length
pointer, discarding the actual size of the firmware property. If the property
is shorter than 128 bytes, drm_edid_alloc (and the previous kmemdup call)
will perform an out-of-bounds read which could potentially trigger a panic if
the memory access crosses an unmapped page boundary.

Would it make sense to pass a length pointer to of_get_property and verify
the property is at least EDID_LENGTH bytes before attempting to allocate it?

> +
> +			/* Firmware-provided, so validate it like the DDC
> +			 * readers would.
> +			 */
> +			if (drm_edid && !drm_edid_valid(drm_edid)) {
> +				drm_edid_free(drm_edid);
> +				drm_edid = NULL;
> +			}
> +			nouveau_connector_set_edid(nv_connector, drm_edid);
>  			return nv_encoder;
>  		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10