Re: [PATCH 2/2] drm/udl: add a release method and delay modeset teardown

["Noralf Trønnes" <noralf@tronnes.org>]

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.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel