On Thu, Mar 01, 2012 at 01:53:01PM +0200, Ville Syrjälä wrote:
> 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...
Ah, didn't realize this was in already. I was looking at an older tree.
I'll send a patch to do the bounds checking...
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
