[AMD Official Use Only] > -----Original Message----- > From: Doug Anderson <dianders@xxxxxxxxxxxx> > Sent: October 5, 2021 11:14 AM > To: Zuo, Jerry <Jerry.Zuo@xxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>; dri- > devel@xxxxxxxxxxxxxxxxxxxxx; geert@xxxxxxxxxxxxxx; oliver.sang@xxxxxxxxx; > Daniel Vetter <daniel@xxxxxxxx>; David Airlie <airlied@xxxxxxxx>; Jani Nikula > <jani.nikula@xxxxxxxxx>; Linus Walleij <linus.walleij@xxxxxxxxxx>; Maarten > Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>; Maxime Ripard > <mripard@xxxxxxxxxx>; Sam Ravnborg <sam@xxxxxxxxxxxx>; Thomas > Zimmermann <tzimmermann@xxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; > Wentland, Harry <Harry.Wentland@xxxxxxx>; Siqueira, Rodrigo > <Rodrigo.Siqueira@xxxxxxx>; Kuogee Hsieh <khsieh@xxxxxxxxxxxxxx> > 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@xxxxxxx> 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&data=04%7C01%7CJerry.Zuo%40amd.com%7C90 > b948659454400cedd308d98812c339%7C3dd8961fe4884e608e11a82d994e183d > %7C0%7C0%7C637690436453163864%7CUnknown%7CTWFpbGZsb3d8eyJWIj > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1 > 000&sdata=OtSngWlYyDc1NbNSgAeALqN3nF%2Bnw08nJ068cpAKZJk%3 > D&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.