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

[Alex Deucher]

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>

>
>  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
>