On Sun, Nov 13, 2011 at 02:04:54AM +0100, Christian Schmidt wrote:
> The current logic misunderstands the spec about CEA 18byte descriptors.
> First, the spec doesn't state "detailed timing descriptors" but "18 byte
> descriptors", so any data record could be stored, mixed timings and
> other data, just as in the standard EDID.
> Second, the lower four bit of byte 3 of the CEA record do not contain
> the number of descriptors, but "the total number of DTDs defining native
> formats in the whole EDID [...], starting with the first DTD in the DTD
> list (which starts in the base EDID block)." A device can of course
> support non-native formats.
>
> As such the number can't be used to determine n, and the existing code
> will filter non-timing 18byte descriptors anyway.
>
> Signed-off-by: Christian Schmidt <schmidt@digadd,de>
> diff -ur linux-3.2-rc1.orig/drivers/gpu/drm/drm_edid.c linux-3.2-rc1/drivers/gpu/drm/drm_edid.c
> --- linux-3.2-rc1.orig/drivers/gpu/drm/drm_edid.c 2011-11-13 01:42:29.771092473 +0100
> +++ linux-3.2-rc1/drivers/gpu/drm/drm_edid.c 2011-11-13 01:54:32.031062983 +0100
> @@ -511,22 +511,7 @@
> u8 rev = ext[0x01], d = ext[0x02];
> u8 *det_base = ext + d;
>
> - switch (rev) {
> - case 0:
> - /* can't happen */
> - return;
> - case 1:
> - /* have to infer how many blocks we have, check pixel clock */
> - for (i = 0; i < 6; i++)
> - if (det_base[18*i] || det_base[18*i+1])
> - n++;
> - break;
> - default:
> - /* explicit count */
> - n = min(ext[0x03] & 0x0f, 6);
> - break;
> - }
> -
> + n = (127 - d) / 18;
> for (i = 0; i < n; i++)
> cb((struct detailed_timing *)(det_base + 18 * i), closure);
> }
I just stumbled on this same thing when looking at some internal patch.
Looks good, except you should also check that 'd' is less than 127.
I do wonder how may other unchecked buffer accesses there are in the
EDID code...
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
