LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Sean Paul <seanpaul@chromium.org>
Cc: Emil Lundmark <lndmrk@chromium.org>,
	Dave Airlie <airlied@redhat.com>,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: udl: Destroy framebuffer only if it was initialized
Date: Tue, 24 Apr 2018 15:04:45 +0200	[thread overview]
Message-ID: <20180424130445.GD31310@phenom.ffwll.local> (raw)
In-Reply-To: <20180420135532.GH73214@art_vandelay>

On Fri, Apr 20, 2018 at 09:55:32AM -0400, Sean Paul wrote:
> On Fri, Apr 20, 2018 at 01:50:01PM +0200, Emil Lundmark wrote:
> > This fixes a NULL pointer dereference that can happen if the UDL
> > driver is unloaded before the framebuffer is initialized. This can
> > happen e.g. if the USB device is unplugged right after it was plugged
> > in.
> > 
> 
> JFYI, in future, if someone makes a suggestion on how to fix a bug, it's good
> practice to add a Suggested-by tag to give credit.
> 
> Reviewed-by: Sean Paul <seanpaul@chromium.org>

I think a bit more detail in the commit message on why this is even
possible would be good. I think it can only happen when you only plug in
the udl, without anything connected.

In that case fbdev setup will be delayed until something shows up (so we
don't pin a too small fb for it, a feature requested by soc people). This
can easily be tested:
First:
- plug in udl device
- wait a minute or so (to make it clear it's not a race)
- unplug

And then:
- plug in an udl device, but with something connected.
- unplug right away.

I expect that in the first case you'll always blow up, but in the 2nd one
you'll never blow up (no matter how fast you unplug).

Cheers, Daniel



> 
> > Signed-off-by: Emil Lundmark <lndmrk@chromium.org>
> > ---
> >  drivers/gpu/drm/udl/udl_fb.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c
> > index 2ebdc6d5a76e..5754e37f741b 100644
> > --- a/drivers/gpu/drm/udl/udl_fb.c
> > +++ b/drivers/gpu/drm/udl/udl_fb.c
> > @@ -426,9 +426,11 @@ static void udl_fbdev_destroy(struct drm_device *dev,
> >  {
> >  	drm_fb_helper_unregister_fbi(&ufbdev->helper);
> >  	drm_fb_helper_fini(&ufbdev->helper);
> > -	drm_framebuffer_unregister_private(&ufbdev->ufb.base);
> > -	drm_framebuffer_cleanup(&ufbdev->ufb.base);
> > -	drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
> > +	if (ufbdev->ufb.obj) {
> > +		drm_framebuffer_unregister_private(&ufbdev->ufb.base);
> > +		drm_framebuffer_cleanup(&ufbdev->ufb.base);
> > +		drm_gem_object_put_unlocked(&ufbdev->ufb.obj->base);
> > +	}
> >  }
> >  
> >  int udl_fbdev_init(struct drm_device *dev)
> > -- 
> > 2.17.0.484.g0c8726318c-goog
> > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  reply	other threads:[~2018-04-24 13:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-20 11:50 Emil Lundmark
2018-04-20 13:55 ` Sean Paul
2018-04-24 13:04   ` Daniel Vetter [this message]
2018-04-25 22:56     ` Stéphane Marchesin
2018-04-26  9:50       ` Daniel Vetter
2018-05-28 14:27 ` [PATCH v2] " Emil Lundmark
2018-05-29  6:46   ` Daniel Vetter
2018-09-10 14:23   ` Sean Paul

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=20180424130445.GD31310@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lndmrk@chromium.org \
    --cc=seanpaul@chromium.org \
    --subject='Re: [PATCH] drm: udl: Destroy framebuffer only if it was initialized' \
    /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).