[PATCH] drm/prime: fix error path deadlock fail

[Daniel Vetter]

On Mon, Jun 13, 2016 at 11:19:20AM -0400, Alex Deucher wrote:
> On Thu, Jun 9, 2016 at 3:29 PM, Rob Clark <robdclark at gmail.com> wrote:
> > There were a couple messed up things about this fail path.
> > (1) it would drop object_name_lock twice
> > (2) drm_gem_handle_delete() (in drm_gem_remove_prime_handles())
> >     needs to grab prime_lock
> >
> > Reported-by: Alex Deucher <alexdeucher at gmail.com>
> > Signed-off-by: Rob Clark <robdclark at gmail.com>
> > ---
> > Untested, but I think this should fix the potential deadlock that Alex
> > reported.  Would be nice if someone with setup to reproduce could test
> > this.
> 
> Sorry for the confusion.  There were actually two deadlocks.  The
> first one (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1579610
> fixed by https://github.com/torvalds/linux/commit/6984128d01cf935820a0563f3a00c6623ba58109)
> was that one we hit in testing.  This one was just one that Qiang
> noticed by inspection while debugging the first.  That said, the error
> path is obviously wrong and the patch looks correct to me.
> 
> Reviewed-by: Alex Deucher <alexander.deucher at amd.com>

Ok, I didn't add the bugzilla to this one. Applied to drm-misc, thanks.
-Daniel

> 
> >
> >  drivers/gpu/drm/drm_prime.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index aab0f3f..780589b 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -593,7 +593,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> >                 get_dma_buf(dma_buf);
> >         }
> >
> > -       /* drm_gem_handle_create_tail unlocks dev->object_name_lock. */
> > +       /* _handle_create_tail unconditionally unlocks dev->object_name_lock. */
> >         ret = drm_gem_handle_create_tail(file_priv, obj, handle);
> >         drm_gem_object_unreference_unlocked(obj);
> >         if (ret)
> > @@ -601,11 +601,10 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
> >
> >         ret = drm_prime_add_buf_handle(&file_priv->prime,
> >                         dma_buf, *handle);
> > +       mutex_unlock(&file_priv->prime.lock);
> >         if (ret)
> >                 goto fail;
> >
> > -       mutex_unlock(&file_priv->prime.lock);
> > -
> >         dma_buf_put(dma_buf);
> >
> >         return 0;
> > @@ -615,11 +614,14 @@ fail:
> >          * to detach.. which seems ok..
> >          */
> >         drm_gem_handle_delete(file_priv, *handle);
> > +       dma_buf_put(dma_buf);
> > +       return ret;
> > +
> >  out_unlock:
> >         mutex_unlock(&dev->object_name_lock);
> >  out_put:
> > -       dma_buf_put(dma_buf);
> >         mutex_unlock(&file_priv->prime.lock);
> > +       dma_buf_put(dma_buf);
> >         return ret;
> >  }
> >  EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);
> > --
> > 2.5.5
> >

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