From: Daniel Vetter <daniel@ffwll.ch>
To: Dave Airlie <airlied@gmail.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
Dave Airlie <airlied@redhat.com>,
"intel-gfx@lists.freedesktop.org"
<intel-gfx@lists.freedesktop.org>,
stable@vger.kernel.org,
dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.
Date: Sat, 15 Aug 2015 21:12:37 +0200 [thread overview]
Message-ID: <20150815191237.GG20434@phenom.ffwll.local> (raw)
In-Reply-To: <CAPM=9twVC9OSiz3r+kHZZ1b2sAfg6GuQQ5b_emGfzMD7OnOpWg@mail.gmail.com>
On Sat, Aug 15, 2015 at 02:56:57PM +1000, Dave Airlie wrote:
> On 11 August 2015 at 17:54, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
> > The port is removed synchronously, but the connector delayed.
> > This causes a use after free which can cause a kernel BUG with
> > slug_debug=FPZU. This is fixed by freeing the port after the
> > connector.
>
> Where is the use after free btw? I'm not sure I like delaying the port
> destruction, there should be no need to.
>
> The connector->port pointer shouldn't be used without validation
> anywhere, and if it is that is a bug.
>
> I'd like to reproduce this before pulling this in.
The remove function needs to lock at the connector->port to shut down the
dp mst link. Before your patch that was done _before_ the final kfree on
the port, but with your patch that's now the other way round: First we
synchronously kfree the port, then we call the driver's connector cleanup
function asynchronously. And that is very unhappy that the port is now
gone.
So perfectly ok regression fix imo to restore the ordering we had before
your patch in the cleanup code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
prev parent reply other threads:[~2015-08-15 19:12 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-11 7:54 [PATCH] drm/dp/mst: Remove port after removing connector Maarten Lankhorst
2015-08-11 8:55 ` [Intel-gfx] " Daniel Vetter
2015-08-11 9:31 ` Jani Nikula
2015-08-15 4:56 ` Dave Airlie
2015-08-15 19:12 ` Daniel Vetter [this message]
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=20150815191237.GG20434@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=airlied@gmail.com \
--cc=airlied@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=stable@vger.kernel.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).