Message ID | 20190315051330.18871-3-airlied@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] drm: do lower level device put on unplugged releases | expand |
On Fri, Mar 15, 2019 at 03:13:30PM +1000, Dave Airlie wrote: > From: Dave Airlie <airlied@redhat.com> > > If we unplug a udl device, the usb callback with deinit the > mode_config struct, however userspace will still have an open > file descriptor and a framebuffer on that device. When userspace > closes the fd, we'll oops because it'll try and look stuff up > in the object idr which we've destroyed. > > This punts destroying the mode objects until release time instead. > > Signed-off-by: Dave Airlie <airlied@redhat.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> For the first one pls ping Noralf to make sure it all aligns with the Grand Plan. -Daniel > --- > drivers/gpu/drm/udl/udl_drv.c | 1 + > drivers/gpu/drm/udl/udl_drv.h | 1 + > drivers/gpu/drm/udl/udl_main.c | 8 +++++++- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index 22cd2d13e272..ff47f890e6ad 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -52,6 +52,7 @@ static struct drm_driver driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, > .load = udl_driver_load, > .unload = udl_driver_unload, > + .release = udl_driver_release, > > /* gem hooks */ > .gem_free_object_unlocked = udl_gem_free_object, > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index e9e9b1ff678e..4ae67d882eae 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb); > > int udl_driver_load(struct drm_device *dev, unsigned long flags); > void udl_driver_unload(struct drm_device *dev); > +void udl_driver_release(struct drm_device *dev); > > int udl_fbdev_init(struct drm_device *dev); > void udl_fbdev_cleanup(struct drm_device *dev); > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 9086d0d1b880..5626e1f11852 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) > udl_free_urb_list(dev); > > udl_fbdev_cleanup(dev); > - udl_modeset_cleanup(dev); > kfree(udl); > } > + > +void udl_driver_release(struct drm_device *dev) > +{ > + drm_mode_config_cleanup(dev); > + drm_dev_fini(dev); > + kfree(dev); > +} > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 15.03.2019 06.13, skrev Dave Airlie: > From: Dave Airlie <airlied@redhat.com> > > If we unplug a udl device, the usb callback with deinit the > mode_config struct, however userspace will still have an open > file descriptor and a framebuffer on that device. When userspace > closes the fd, we'll oops because it'll try and look stuff up > in the object idr which we've destroyed. > > This punts destroying the mode objects until release time instead. > > Signed-off-by: Dave Airlie <airlied@redhat.com> > --- > drivers/gpu/drm/udl/udl_drv.c | 1 + > drivers/gpu/drm/udl/udl_drv.h | 1 + > drivers/gpu/drm/udl/udl_main.c | 8 +++++++- > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index 22cd2d13e272..ff47f890e6ad 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -52,6 +52,7 @@ static struct drm_driver driver = { > .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, > .load = udl_driver_load, > .unload = udl_driver_unload, > + .release = udl_driver_release, > > /* gem hooks */ > .gem_free_object_unlocked = udl_gem_free_object, > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index e9e9b1ff678e..4ae67d882eae 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb); > > int udl_driver_load(struct drm_device *dev, unsigned long flags); > void udl_driver_unload(struct drm_device *dev); > +void udl_driver_release(struct drm_device *dev); > > int udl_fbdev_init(struct drm_device *dev); > void udl_fbdev_cleanup(struct drm_device *dev); > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 9086d0d1b880..5626e1f11852 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) > udl_free_urb_list(dev); > > udl_fbdev_cleanup(dev); > - udl_modeset_cleanup(dev); > kfree(udl); > } > + > +void udl_driver_release(struct drm_device *dev) > +{ > + drm_mode_config_cleanup(dev); > + drm_dev_fini(dev); > + kfree(dev); > +} > You can remove udl_modeset_cleanup() now. There's still a problem here and that is if udl_driver_load() errors out before drm_mode_config_init() is called, drm_mode_config_cleanup() will crash on uninitialized lists etc. I have solved this by calling drm_mode_config_init() right after drm_device has been initialized. Noralf.
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index 22cd2d13e272..ff47f890e6ad 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -52,6 +52,7 @@ static struct drm_driver driver = { .driver_features = DRIVER_MODESET | DRIVER_GEM | DRIVER_PRIME, .load = udl_driver_load, .unload = udl_driver_unload, + .release = udl_driver_release, /* gem hooks */ .gem_free_object_unlocked = udl_gem_free_object, diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index e9e9b1ff678e..4ae67d882eae 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -104,6 +104,7 @@ void udl_urb_completion(struct urb *urb); int udl_driver_load(struct drm_device *dev, unsigned long flags); void udl_driver_unload(struct drm_device *dev); +void udl_driver_release(struct drm_device *dev); int udl_fbdev_init(struct drm_device *dev); void udl_fbdev_cleanup(struct drm_device *dev); diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 9086d0d1b880..5626e1f11852 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -379,6 +379,12 @@ void udl_driver_unload(struct drm_device *dev) udl_free_urb_list(dev); udl_fbdev_cleanup(dev); - udl_modeset_cleanup(dev); kfree(udl); } + +void udl_driver_release(struct drm_device *dev) +{ + drm_mode_config_cleanup(dev); + drm_dev_fini(dev); + kfree(dev); +}