LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Zuo, Jerry" <Jerry.Zuo@amd.com>
To: "Wentland, Harry" <Harry.Wentland@amd.com>,
	Doug Anderson <dianders@chromium.org>
Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"geert@linux-m68k.org" <geert@linux-m68k.org>,
	"oliver.sang@intel.com" <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" <linux-kernel@vger.kernel.org>,
	"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)
Date: Wed, 6 Oct 2021 12:05:26 +0000	[thread overview]
Message-ID: <DM6PR12MB49129E10E87DD88576B16A5DE5B09@DM6PR12MB4912.namprd12.prod.outlook.com> (raw)
In-Reply-To: <ba92567c-4ef3-871e-16d9-1830e2c0e929@amd.com>

[AMD Official Use Only]

> -----Original Message-----
> From: Wentland, Harry <Harry.Wentland@amd.com>
> Sent: October 5, 2021 2:04 PM
> To: Zuo, Jerry <Jerry.Zuo@amd.com>; Doug Anderson
> <dianders@chromium.org>
> 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;
> 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)
>
>
>
> 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%7CTWFpbGZsb3d8eyJ
> WIj
> >>
> 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

Yes, I'll give a try on that.

  reply	other threads:[~2021-10-06 12:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04 16:21 [PATCH] drm/edid: Fix crash with zero/invalid EDID Douglas Anderson
2021-10-04 17:14 ` Geert Uytterhoeven
2021-10-05  0:40   ` Doug Anderson
2021-10-05 13:45     ` Doug Anderson
2021-10-04 19:44 ` Ville Syrjälä
2021-10-05 13:33   ` Zuo, Jerry
2021-10-05 15:13     ` connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID) Doug Anderson
2021-10-05 15:25       ` Zuo, Jerry
2021-10-05 18:03         ` Harry Wentland
2021-10-06 12:05           ` Zuo, Jerry [this message]
2021-10-05 16:43 ` [PATCH] drm/edid: Fix crash with zero/invalid EDID Ville Syrjälä
2021-10-06  2:20   ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR12MB49129E10E87DD88576B16A5DE5B09@DM6PR12MB4912.namprd12.prod.outlook.com \
    --to=jerry.zuo@amd.com \
    --cc=Harry.Wentland@amd.com \
    --cc=Rodrigo.Siqueira@amd.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert@linux-m68k.org \
    --cc=jani.nikula@intel.com \
    --cc=khsieh@codeaurora.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=oliver.sang@intel.com \
    --cc=sam@ravnborg.org \
    --cc=tzimmermann@suse.de \
    --cc=ville.syrjala@linux.intel.com \
    --subject='RE: connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).