[PATCH] drm/drm_lease: Do not call drm_master_put() twice in case drm_lease_create() fails

[Daniel Vetter]

On Tue, Dec 12, 2017 at 03:44:07PM +0000, Marius-cristian Vlad wrote:
> drm_mode_create_lease_ioctl() -> drm_lease_create()
> 
> drm_lease_create() -> fails and drm_master_put() is called
> twice: once in drm_lease_create() and once in
> drm_mode_create_lease_ioctl().
> 
> From drm_mode_create_lease_ioctl():
> 
> 	lessee = drm_lease_create(lessor, &leases);
>         if (IS_ERR(lessee)) {
>                 ret = PTR_ERR(lessee);
>                 goto out_leases;
>         }
> ....
> out_lessee:

out_lessee != out_leases

>         drm_master_put(&lessee); <- but we already done this in
> drm_lease_create().

This is the path I checked, looks all correct to me. Where exactly have
you observed the leak? Do we have a testcase (igt very much preferred,
sicne then at least the intel team will CI it constantly) that reproduces
the leak?
-Daniel

> 
> 
> On Tue, 2017-12-12 at 16:30 +0100, Daniel Vetter wrote:
> > On Tue, Dec 12, 2017 at 02:04:14PM +0200, Marius Vlad wrote:
> > > This case can been seen when creating the lease with same objects
> > > passed.
> > > 
> > > Signed-off-by: Marius Vlad <marius-cristian.vlad at nxp.com>
> > > ---
> > >  drivers/gpu/drm/drm_lease.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_lease.c
> > > b/drivers/gpu/drm/drm_lease.c
> > > index d1eb56a..ae57f33 100644
> > > --- a/drivers/gpu/drm/drm_lease.c
> > > +++ b/drivers/gpu/drm/drm_lease.c
> > > @@ -254,8 +254,6 @@ static struct drm_master
> > > *drm_lease_create(struct drm_master *lessor, struct idr
> > >  	return lessee;
> > >  
> > >  out_lessee:
> > > -	drm_master_put(&lessee);
> > 
> > I'm not really following here ... the lessee reference we're dropping
> > here
> > is created in drm_master_create. We're only calling drm_master_put if
> > that
> > succeeded. Removing this line here looks like now we're leaking.
> > 
> > Where is the double-free? I don't see the 2nd drm_master_put()
> > anywhere
> > ... drm_mode_create_lease_ioctl also seems to be doing the right
> > thing
> > from just staring at it.
> > -Daniel

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