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
