linux-kernel - RE: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 5 Oct 2021 15:25:16 +0000
From:   "Zuo, Jerry" <Jerry.Zuo@....com>
To:     Doug Anderson <dianders@...omium.org>
CC:     Ville Syrjälä <ville.syrjala@...ux.intel.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "geert@...ux-m68k.org" <geert@...ux-m68k.org>,
        "oliver.sang@...el.com" <oliver.sang@...el.com>,
        Daniel Vetter <daniel@...ll.ch>,
        David Airlie <airlied@...ux.ie>,
        Jani Nikula <jani.nikula@...el.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Sam Ravnborg <sam@...nborg.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Wentland, Harry" <Harry.Wentland@....com>,
        "Siqueira, Rodrigo" <Rodrigo.Siqueira@....com>,
        Kuogee Hsieh <khsieh@...eaurora.org>
Subject: RE: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix
 crash with zero/invalid EDID)

[AMD Official Use Only]

> -----Original Message-----
> From: Doug Anderson <dianders@...omium.org>
> Sent: October 5, 2021 11:14 AM
> To: Zuo, Jerry <Jerry.Zuo@....com>
> Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>; dri-
> devel@...ts.freedesktop.org; geert@...ux-m68k.org; oliver.sang@...el.com;
> Daniel Vetter <daniel@...ll.ch>; David Airlie <airlied@...ux.ie>; Jani Nikula
> <jani.nikula@...el.com>; Linus Walleij <linus.walleij@...aro.org>; Maarten
> Lankhorst <maarten.lankhorst@...ux.intel.com>; Maxime Ripard
> <mripard@...nel.org>; Sam Ravnborg <sam@...nborg.org>; Thomas
> Zimmermann <tzimmermann@...e.de>; linux-kernel@...r.kernel.org;
> Wentland, Harry <Harry.Wentland@....com>; Siqueira, Rodrigo
> <Rodrigo.Siqueira@....com>; Kuogee Hsieh <khsieh@...eaurora.org>
> Subject: Re: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid:
> Fix crash with zero/invalid EDID)
>
> Hi,
>
> On Tue, Oct 5, 2021 at 6:33 AM Zuo, Jerry <Jerry.Zuo@....com> wrote:
> >
> > > BTW I believe connector_bad_edid() itself is broken since commit
> > > e11f5bd8228f ("drm: Add support for DP 1.4 Compliance edid
> > > corruption test"). Before we've even allocated the memory for the
> > > extension blocks that code now assumes edid[0x7e] is to be 100%
> > > trusted and goes and calculates the checksum on a block based on
> > > that. So that's likely going to be pointing somewhere beyond the
> > > base block into memory we've not even allocated. So anyone who
> > > wanted could craft a bogus EDID and maybe get something interesting to
> happen.
> > >
> > > Would be good if someone could fix that while at it. Or just revert
> > > the offending commit if there is no simple solution immediately in sight.
> > >
> > > The fact that we're parsing entirely untrustworthy crap in the
> > > kernel always worries me. Either we need super careful review of all
> > > relevant code, and/or we need to think about moving the parser out of
> the kernel.
> > > I was considering playing around with the usermode helper stuff.
> > > IIRC there is a way to embed the userspace binary into the kernel
> > > and just fire it up when needed. But so far it's been the usual -ENOTIME
> for me...
> > >
> > [AMD Official Use Only]
> >
> > Hi Ville:
> >
> >      Yhea, it is pretty old change from two years ago, and it is no long valid
> anymore. Please simply drop it.
> >
> > Regards,
> > Jerry
>
> I've cut out other bits from this email and changed the subject line since I
> think this is an issue unrelated to the one my original patch was fixing.
>
> I don't actually know a ton about DP compliance testing, but I attempted to
> try to be helpful and revert commit e11f5bd8228f ("drm:
> Add support for DP 1.4 Compliance edid corruption test"). It wasn't too hard
> to deal with the conflicts in the revert itself, but then things didn't compile
> because there are two places that use `real_edid_checksum` and that goes
> away if I revert the patch.
>
> I've made an attempt to fix the problem by just adding a bounds check.
> Perhaps you can see if that looks good to you:
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fr%2F20211005081022.1.Ib059f9c23c2611cb5a9d760e7d0a700c1
> 295928d%40changeid&amp;data=04%7C01%7CJerry.Zuo%40amd.com%7C90
> b948659454400cedd308d98812c339%7C3dd8961fe4884e608e11a82d994e183d
> %7C0%7C0%7C637690436453163864%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1
> 000&amp;sdata=OtSngWlYyDc1NbNSgAeALqN3nF%2Bnw08nJ068cpAKZJk%3
> D&amp;reserved=0
>
> -Doug

The patch used for DP1.4 compliance edid corruption test. Let me double check if edid corruption test could be passed without the patch.