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

[Emil Velikov]

On 2019/01/14, Daniel Vetter wrote:
> 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>
> 
Thanks.

> 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>
> 
I'll leave that for another day, since the inverse vfunc (firstopen <>
lastclose) is not a legacy only one. Which kind of irks me a bit.

-Emil