LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: "Zuo, Jerry" <Jerry.Zuo@amd.com>
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>,
	"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)
Date: Tue, 5 Oct 2021 08:13:47 -0700	[thread overview]
Message-ID: <CAD=FV=VvKsrB9RZKdB6vQJ-38BZEYLnuENxb1+1v-PahcdBtiQ@mail.gmail.com> (raw)
In-Reply-To: <DM6PR12MB49127B8B63079E6533197EA6E5AF9@DM6PR12MB4912.namprd12.prod.outlook.com>

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/r/20211005081022.1.Ib059f9c23c2611cb5a9d760e7d0a700c1295928d@changeid

-Doug

  reply	other threads:[~2021-10-05 15:14 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     ` Doug Anderson [this message]
2021-10-05 15:25       ` connector_bad_edid() is broken (was: Re: [PATCH] drm/edid: Fix crash with zero/invalid EDID) Zuo, Jerry
2021-10-05 18:03         ` Harry Wentland
2021-10-06 12:05           ` Zuo, Jerry
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='CAD=FV=VvKsrB9RZKdB6vQJ-38BZEYLnuENxb1+1v-PahcdBtiQ@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=Harry.Wentland@amd.com \
    --cc=Jerry.Zuo@amd.com \
    --cc=Rodrigo.Siqueira@amd.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --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).