[PATCH v2 0/2] drm/amd/display: Fix missing HF-EEODB blocks in EDID copies

[PATCH v2 0/2] drm/amd/display: Fix missing HF-EEODB blocks in EDID copies

[Timo Prömer]

Fix an issue in amd/display where devices with HF-EEODB blocks would 
be missing these additional blocks during EDID reads.

The driver previously used `edid->extensions + 1` to calculate the 
number of blocks to copy, but the base extension flag does not include 
HF-EEODB blocks.

Use drm_edid_block_count() directly to get the true number of blocks, 
ensuring that HF-EEODB blocks are properly copied.

Changes in v2:
- Fixed a bug in patch 2 where the bounds check still used the old
  extension count.
- Patch 1 remains unchanged.

Timo Prömer (2):
  drm/edid: Export drm_edid_block_count()
  drm/amd/display: Use drm_edid_block_count() instead of raw extensions

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 11 ++++++++---
 drivers/gpu/drm/drm_edid.c                            |  3 ++-
 include/drm/drm_edid.h                                |  1 +
 3 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.55.0

[PATCH v2 1/2] drm/edid: Export drm_edid_block_count()

[Timo Prömer]

From: Timoyoungster <timo.proemer04@gmail.com>

Drivers currently calculating EDID size by reading the `extensions`
field of the raw EDID structure (e.g., `edid->extensions + 1`) will
calculate the wrong size if the EDID contains an HF-EEODB (HDMI Forum
EDID Extension Override Data Block). The base extension flag does not
account for these override blocks, leading to truncated EDIDs.

Remove the static declaration and export drm_edid_block_count() so
drivers can safely query the true block count. This allows drivers to
leverage the core DRM's proper handling of HF-EEODB and other edge
cases without having to parse the raw EDID fields themselves.

Signed-off-by: Timo Prömer <timo.proemer04@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 3 ++-
 include/drm/drm_edid.h     | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index df3c25bac..34560b33a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1698,12 +1698,13 @@ static int __drm_edid_block_count(const struct drm_edid *drm_edid)
 }
 
 /* EDID block count, limited by allocated size */
-static int drm_edid_block_count(const struct drm_edid *drm_edid)
+int drm_edid_block_count(const struct drm_edid *drm_edid)
 {
 	/* Limit by allocated size */
 	return min(__drm_edid_block_count(drm_edid),
 		   (int)drm_edid->size / EDID_LENGTH);
 }
+EXPORT_SYMBOL(drm_edid_block_count);
 
 /* EDID extension block count, limited by allocated size */
 static int drm_edid_extension_block_count(const struct drm_edid *drm_edid)
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 04f7a7f1f..4a990bf87 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -481,6 +481,7 @@ const struct drm_edid *drm_edid_read_switcheroo(struct drm_connector *connector,
 int drm_edid_connector_update(struct drm_connector *connector,
 			      const struct drm_edid *edid);
 int drm_edid_connector_add_modes(struct drm_connector *connector);
+int drm_edid_block_count(const struct drm_edid *drm_edid);
 bool drm_edid_is_digital(const struct drm_edid *drm_edid);
 void drm_edid_get_product_id(const struct drm_edid *drm_edid,
 			     struct drm_edid_product_id *id);
-- 
2.55.0

[PATCH v2 2/2] drm/amd/display: Use drm_edid_block_count() instead of raw extensions

[Timo Prömer]

From: Timoyoungster <timo.proemer04@gmail.com>

Instead of manually calculating the EDID block count by reading the
extensions field from the raw edid structure (`edid->extensions + 1`),
utilize the core DRM helper `drm_edid_block_count()`.

This now includes possible HF-EEODB extension blocks, which are not
included in `edid->extensions` and were previously truncated with the
`memmove`.

Signed-off-by: Timo Prömer <timo.proemer04@gmail.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index c6f94eb71..d6d6ea719 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -1160,6 +1160,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
 	struct drm_connector *connector = &aconnector->base;
 	struct i2c_adapter *ddc;
 	int retry = 25;
+	int block_count;
 	enum dc_edid_status edid_status = EDID_NO_RESPONSE;
 	const struct drm_edid *drm_edid;
 	const struct edid *edid;
@@ -1201,11 +1202,15 @@ enum dc_edid_status dm_helpers_read_local_edid(
 			continue;
 
 		edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
-		if (!edid ||
-		    edid->extensions >= sizeof(sink->dc_edid.raw_edid) / EDID_LENGTH)
+		if (!edid)
 			return EDID_BAD_INPUT;
 
-		sink->dc_edid.length = EDID_LENGTH * (edid->extensions + 1);
+		block_count = drm_edid_block_count(drm_edid);
+
+		if (block_count > sizeof(sink->dc_edid.raw_edid) / EDID_LENGTH)
+			return EDID_BAD_INPUT;
+
+		sink->dc_edid.length = EDID_LENGTH * block_count;
 		memmove(sink->dc_edid.raw_edid, (uint8_t *)edid, sink->dc_edid.length);
 
 		/* We don't need the original edid anymore */
-- 
2.55.0

Re: [PATCH v2 2/2] drm/amd/display: Use drm_edid_block_count() instead of raw extensions

[sashiko-bot]

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

New issues:
- [High] Kernel heap out-of-bounds read due to mismatch between HF-EEODB block count and raw `edid->extensions` byte.

Pre-existing issues:
- [Critical] Heap buffer overflow in amdgpu_dm_connector_funcs_force() when copying EDID.
--

--- Patch [2]: [PATCH v2 2/2] drm/amd/display: Use drm_edid_block_count() instead of raw extensions ---
commit bb5e357c1182326e11f3ce4e756df91c3df854a6
Author: Timoyoungster <timo.proemer04@gmail.com>

drm/amd/display: Use drm_edid_block_count() instead of raw extensions

Instead of manually calculating the EDID block count by reading the
extensions field from the raw edid structure (edid->extensions + 1),
utilize the core DRM helper drm_edid_block_count().

This now includes possible HF-EEODB extension blocks, which are not
included in edid->extensions and were previously truncated with the
memmove.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index 71e2627f9a9d4..82f3cce870a3c 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -1220,11 +1221,15 @@ enum dc_edid_status dm_helpers_read_local_edid(
>  			continue;
>  
>  		edid = drm_edid_raw(drm_edid); // FIXME: Get rid of drm_edid_raw()
> -		if (!edid ||
> -		    edid->extensions >= sizeof(sink->dc_edid.raw_edid) / EDID_LENGTH)
> +		if (!edid)
>  			return EDID_BAD_INPUT;
>  
> -		sink->dc_edid.length = EDID_LENGTH * (edid->extensions + 1);
> +		block_count = drm_edid_block_count(drm_edid);
> +
> +		if (block_count > sizeof(sink->dc_edid.raw_edid) / EDID_LENGTH)
> +			return EDID_BAD_INPUT;
> +
> +		sink->dc_edid.length = EDID_LENGTH * block_count;
>  		memmove(sink->dc_edid.raw_edid, (uint8_t *)edid, sink->dc_edid.length);

[Severity: High]
Does this create an out-of-bounds read when the copied EDID is parsed downstream?

If a malicious display provides an EDID with an extension count of 255 but an
HF-EEODB block specifying a count of 2, drm_edid_block_count() will return 2.
The bounds check passes, and memmove() copies 256 bytes into raw_edid.

However, the raw extensions byte in the copied data remains 255.
When this is later parsed in dm_helpers_parse_edid_caps() which delegates to
drm_edid_is_valid(), doesn't that legacy helper use the unmodified extensions
byte to iterate up to 256 times, reading past the end of the 2048-byte
raw_edid array on the kernel heap?

Should the raw extensions byte in the copied buffer be manually updated to match
block_count - 1, preventing legacy functions from performing out-of-bounds
reads?

Also, looking at related EDID copying code in the same subsystem:

[Severity: Critical]
This isn't a bug introduced by this patch, but does the following code in
amdgpu_dm_connector.c:amdgpu_dm_connector_funcs_force() have a heap buffer
overflow?

amdgpu_dm_connector_funcs_force() {
    ...
    memmove(dc_em_sink->dc_edid.raw_edid, edid, (edid->extensions + 1) * EDID_LENGTH);
    ...
}

If a malicious display or I2C emulator provides an EDID with extensions set to
255 and valid block checksums, and userspace with DRM privileges forces a
connector status update, it appears this could copy up to 32KB into the 2KB
dc_em_sink->dc_edid.raw_edid buffer, corrupting the heap.

Is a bounds check needed here similar to the one performed in
dm_helpers_read_local_edid()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713193237.2639-1-timo.proemer04@gmail.com?part=2