[PATCH 2/2] drm: plug memory leak on drm_setup() failure

[Daniel Vetter]

On Mon, Jan 14, 2019 at 08:44:10AM +0000, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov at collabora.com>
> 
> Currently we fail to free and detach the drm_file when drm_setup() fails.
> Use the drm_close_helper to do address that.
> 
> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
> ---
>  drivers/gpu/drm/drm_file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
> index 149506a20bdc..871dcd8c7545 100644
> --- a/drivers/gpu/drm/drm_file.c
> +++ b/drivers/gpu/drm/drm_file.c
> @@ -330,8 +330,10 @@ int drm_open(struct inode *inode, struct file *filp)
>  		goto err_undo;
>  	if (need_setup) {
>  		retcode = drm_setup(dev);
> -		if (retcode)
> +		if (retcode) {
> +			drm_close_helper(filp);

I freaked out mildly because drm_open_helper already adds the drm_file to
the filelist (hence publishes it), and publishing objects before they're
fully set up is a Bad Idea :-)

But drm_setup only does legacy setup, so who cares. On both patches:

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

And if you feel like doing an s/drm_setup()/drm_legacy_setup()/, with or
w/o changing the condition to if (need_setup &&
drm_core_check_feature(dev, DRIVER_LEGACY)), then that patch would also
have my

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

>  			goto err_undo;
> +		}
>  	}
>  	return 0;
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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