Re: [PATCH 2/4] drm/amd/display: Use HDMI FreeSync range from common EDID parser

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing payload length checks in AMD VSDB version 2 and 3 parsing leads to out-of-bounds memory reads when processing malformed EDIDs.
--

--- Patch [2]: [PATCH 2/4] drm/amd/display: Use HDMI FreeSync range from common EDID parser ---
commit 1c50a992def070041d1c59487f692155608cc345
Author: Alex Huang <Alex.Huang2@amd.com>
Subject: drm/amd/display: Use HDMI FreeSync range from common EDID parser

This commit modifies the AMD GPU display driver to use the common DRM EDID
parser for detecting HDMI FreeSync capabilities, rather than relying on the
AMD VSDB parser in the DMUB firmware.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 9c564cd5edeea..fd9bb205c8f08 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c

[ ... ]

> @@ -13932,8 +13770,8 @@ void amdgpu_dm_update_freesync_caps(struct drm_connector *connector,
>  		}
>  
>  	} else if (drm_edid && sink->sink_signal == SIGNAL_TYPE_HDMI_TYPE_A) {
> -		i = parse_hdmi_amd_vsdb(amdgpu_dm_connector, edid, &vsdb_info);
> -		if (i >= 0) {
> +		i = get_amd_vsdb(amdgpu_dm_connector, &vsdb_info);
> +		if (i) {

[Severity: High]
Does relying on the common DRM EDID parser here expose us to out-of-bounds
memory reads when processing malformed EDIDs?

Looking at the common parser in drivers/gpu/drm/drm_edid.c,
drm_parse_amd_vsdb() fails to validate the payload length before delegating
to version-specific parsers:

drivers/gpu/drm/drm_edid.c:drm_parse_amd_vsdb() {
    ...
	info->amd_vsdb.version = common->version;

	drm_parse_amd_vsdb_v1(info, data);

	if (common->version >= 2)
		drm_parse_amd_vsdb_v2(info, data);

	if (common->version >= 3)
		drm_parse_amd_vsdb_v3(info, data, cea_db_payload_len(db));
}

The v2 parser unconditionally accesses struct fields up to offset 12 without
checking if the payload length is sufficient:

drivers/gpu/drm/drm_edid.c:drm_parse_amd_vsdb_v2() {
    ...
	info->amd_vsdb.luminance_range1.max_luminance = p->lum1_max;
	info->amd_vsdb.luminance_range1.min_luminance = p->lum1_min;
	info->amd_vsdb.luminance_range2.max_luminance = p->lum2_max;
	info->amd_vsdb.luminance_range2.min_luminance = p->lum2_min;
    ...
}

Similarly, the v3 parser unconditionally accesses fields up to offset 14:

drivers/gpu/drm/drm_edid.c:drm_parse_amd_vsdb_v3() {
    ...
	/* vfreq is provded in a different set of fields for v3. */
	max_frame_rate = p->max_vfreq_ext_low |
			 (p->max_vfreq_ext_high & 0x3) << 8;
    ...
}

If a monitor provides an AMD VSDB block with version 2 or 3 but a payload
length smaller than required, could this lead to an out-of-bounds read of
up to 7 bytes in the slab?

>  			amdgpu_dm_connector->vsdb_info = vsdb_info;
>  			sink->edid_caps.freesync_vcp_code = vsdb_info.freesync_mccs_vcp_code;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721164430.2121326-1-Alex.Huang2@amd.com?part=2