| Message ID | 20180528142711.142466-1-lndmrk@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, May 28, 2018 at 10:27 AM Emil Lundmark <lndmrk@chromium.org> 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. > > As explained by Stéphane Marchesin: > > It happens when fbdev is disabled (which is the case for Chrome OS). > Even though intialization of the fbdev part is optional (it's done in > udlfb_create which is the callback for fb_probe()), the teardown isn't > optional (udl_driver_unload -> udl_fbdev_cleanup -> > udl_fbdev_destroy). > > Note that udl_fbdev_cleanup *tries* to be conditional (you can see it > does if (!udl->fbdev)) but that doesn't work, because udl->fbdev is > always set during udl_fbdev_init. > > Suggested-by: Sean Paul <seanpaul@chromium.org> > Signed-off-by: Emil Lundmark <lndmrk@chromium.org> Many apologies, Emil, I completely dropped the ball on this one. I've just applied it to -misc-fixes. Sean > --- > Changes in v2: > - Updated commit message with explanation from Stéphane Marchesin > > 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.921.gf22659ad46-goog >
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)
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. As explained by Stéphane Marchesin: It happens when fbdev is disabled (which is the case for Chrome OS). Even though intialization of the fbdev part is optional (it's done in udlfb_create which is the callback for fb_probe()), the teardown isn't optional (udl_driver_unload -> udl_fbdev_cleanup -> udl_fbdev_destroy). Note that udl_fbdev_cleanup *tries* to be conditional (you can see it does if (!udl->fbdev)) but that doesn't work, because udl->fbdev is always set during udl_fbdev_init. Suggested-by: Sean Paul <seanpaul@chromium.org> Signed-off-by: Emil Lundmark <lndmrk@chromium.org> --- Changes in v2: - Updated commit message with explanation from Stéphane Marchesin drivers/gpu/drm/udl/udl_fb.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)