Hi Jonathan,

I sent this upstream, but something is not working in the submission
process as I haven't gotten any feedback. If this looks OK, should we
add the fix to our tree? This resolves the sporadic crashes on wake up on
my machine as reported to bugs@...

Thanks
Greg

>From 674554534b9f046f2a4cdab5c2dbc3d019c7daa7 Mon Sep 17 00:00:00 2001
From: Greg Steuck <g...@lenny.nest.cx>
Date: Wed, 21 Apr 2021 19:33:35 -0700
Subject: [PATCH] drm/drm_edid: Read within initialized memory bounds (fix off
 by one)

The problem manifests as a memory out of bounds kernel panic in
OpenBSD which uses this code. The buggy error reporting code path
likely never runs with nominal hardware.

drm_do_get_edid at carp used to invoke connector_bad_edid was num_exts
of 1 even though edid at that point is only allocated a memory block
of size EDID_LENGTH. This in turn led to drm_edid_block_checksum
trying to read memory in the range [edid + EDID_LENGTH, edid +
2*EDID_LENGTH) i.e. outside the allocated boundaries. A similar if a
bit more complicated analysis applies to the other call to
connector_bad_edid. Switching to using valid_extensions limits
connector_bad_edid memory reading to the memory previously written by
drm_do_get_edid.

OpenBSD bug report https://marc.info/?l=openbsd-bugs&m=161794843427437
Sent upstream: 
https://patchwork.kernel.org/project/dri-devel/patch/87wnszm0tw....@lenny.nest.cx/
---
 sys/dev/pci/drm/drm_edid.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/sys/dev/pci/drm/drm_edid.c b/sys/dev/pci/drm/drm_edid.c
index 2e2267410c5..b387864de54 100644
--- a/sys/dev/pci/drm/drm_edid.c
+++ b/sys/dev/pci/drm/drm_edid.c
@@ -1802,25 +1802,21 @@ drm_do_probe_ddc_edid(void *data, u8 *buf, unsigned int 
block, size_t len)
 }
 
 static void connector_bad_edid(struct drm_connector *connector,
-                              u8 *edid, int num_blocks)
+                              u8 *edid, int valid_extensions)
 {
        int i;
-       u8 num_of_ext = edid[0x7e];
-
-       if (num_of_ext > num_blocks)
-               num_of_ext = num_blocks;
 
        /* Calculate real checksum for the last edid extension block data */
        connector->real_edid_checksum =
-               drm_edid_block_checksum(edid + num_of_ext * EDID_LENGTH);
+           drm_edid_block_checksum(edid + valid_extensions * EDID_LENGTH);
 
        if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
                return;
 
        dev_warn(connector->dev->dev,
-                "%s: EDID is invalid:\n",
-                connector->name);
-       for (i = 0; i < num_blocks; i++) {
+                "%s: EDID is invalid, edid_corrupt %d, null_edid_counter %d, 
valid_extensions %d:\n",
+           connector->name, connector->edid_corrupt, 
connector->null_edid_counter, valid_extensions);
+       for (i = 0; i <= valid_extensions; i++) {
                u8 *block = edid + i * EDID_LENGTH;
                char prefix[20];
 
@@ -1967,7 +1963,7 @@ struct edid *drm_do_get_edid(struct drm_connector 
*connector,
        if (valid_extensions != edid[0x7e]) {
                u8 *base;
 
-               connector_bad_edid(connector, edid, edid[0x7e] + 1);
+               connector_bad_edid(connector, edid, valid_extensions);
 
                edid[EDID_LENGTH-1] += edid[0x7e] - valid_extensions;
                edid[0x7e] = valid_extensions;
@@ -1995,7 +1991,7 @@ struct edid *drm_do_get_edid(struct drm_connector 
*connector,
        return (struct edid *)edid;
 
 carp:
-       connector_bad_edid(connector, edid, 1);
+       connector_bad_edid(connector, edid, 0);
 out:
        kfree(edid);
        return NULL;
-- 
2.31.1

Reply via email to