lkml.org 
[lkml]   [2021]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID)
From


On 2021-10-05 11:25, Zuo, Jerry wrote:
> [AMD Official Use Only]
>
>> -----Original Message-----
>> From: Doug Anderson <dianders@chromium.org>
>> Sent: October 5, 2021 11:14 AM
>> To: Zuo, Jerry <Jerry.Zuo@amd.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>; dri-
>> devel@lists.freedesktop.org; geert@linux-m68k.org; oliver.sang@intel.com;
>> Daniel Vetter <daniel@ffwll.ch>; David Airlie <airlied@linux.ie>; Jani Nikula
>> <jani.nikula@intel.com>; Linus Walleij <linus.walleij@linaro.org>; Maarten
>> Lankhorst <maarten.lankhorst@linux.intel.com>; Maxime Ripard
>> <mripard@kernel.org>; Sam Ravnborg <sam@ravnborg.org>; Thomas
>> Zimmermann <tzimmermann@suse.de>; linux-kernel@vger.kernel.org;
>> Wentland, Harry <Harry.Wentland@amd.com>; Siqueira, Rodrigo
>> <Rodrigo.Siqueira@amd.com>; Kuogee Hsieh <khsieh@codeaurora.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@amd.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://lore.
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.
>

Can you try the CTS test with Doug's fix?

https://patchwork.freedesktop.org/patch/457537/

Harry

\
 
 \ /
  Last update: 2021-10-05 20:04    [W:0.065 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site