If we need to take the error path and drop the references to the objects and handle we created earlier, there is a possibility that we then free the object and then attempt to free any associated PRIME resources under the prime mutex. If we are holding the prime mutex when we attempt to free the handle/object, we risk a recursive deadlock.
[ 677.932957] ============================================= [ 677.932957] [ INFO: possible recursive locking detected ] [ 677.932957] 4.3.0-rc5-virtio-gpu+ #11 Not tainted [ 677.932957] --------------------------------------------- [ 677.932957] Xorg/2661 is trying to acquire lock: [ 677.932957] (&prime_fpriv->lock){+.+.+.}, at: [<ffffffffa00151b0>] drm_gem_remove_prime_handles.isra.7+0x20/0x50 [drm] [ 677.932957] but task is already holding lock: [ 677.932957] (&prime_fpriv->lock){+.+.+.}, at: [<ffffffffa002d68b>] drm_gem_prime_fd_to_handle+0x4b/0x240 [drm]
Reported-by: Dave Airlie airlied@gmail.com Signed-off-by: Chris Wilson chris@chris-wilson.co.uk Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Dave Airlie airlied@gmail.com --- drivers/gpu/drm/drm_prime.c | 47 ++++++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 27aa7183b20b..1bdd69e1ef43 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -564,26 +564,26 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, uint32_t *handle) { struct dma_buf *dma_buf; - struct drm_gem_object *obj; + struct drm_gem_object *obj = NULL; int ret;
+ *handle = 0; + dma_buf = dma_buf_get(prime_fd); if (IS_ERR(dma_buf)) return PTR_ERR(dma_buf);
mutex_lock(&file_priv->prime.lock);
- ret = drm_prime_lookup_buf_handle(&file_priv->prime, - dma_buf, handle); + ret = drm_prime_lookup_buf_handle(&file_priv->prime, dma_buf, handle); if (ret == 0) - goto out_put; + goto out;
/* never seen this one, need to import */ - mutex_lock(&dev->object_name_lock); obj = dev->driver->gem_prime_import(dev, dma_buf); if (IS_ERR(obj)) { ret = PTR_ERR(obj); - goto out_unlock; + goto out; }
if (obj->dma_buf) { @@ -593,33 +593,24 @@ 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. */ - ret = drm_gem_handle_create_tail(file_priv, obj, handle); - drm_gem_object_unreference_unlocked(obj); + ret = drm_gem_handle_create(file_priv, obj, handle); if (ret) - goto out_put; + goto out;
- ret = drm_prime_add_buf_handle(&file_priv->prime, - dma_buf, *handle); - if (ret) - goto fail; + ret = drm_prime_add_buf_handle(&file_priv->prime, dma_buf, *handle);
+out: mutex_unlock(&file_priv->prime.lock); - - dma_buf_put(dma_buf); - - return 0; - -fail: - /* hmm, if driver attached, we are relying on the free-object path - * to detach.. which seems ok.. - */ - drm_gem_handle_delete(file_priv, *handle); -out_unlock: - mutex_unlock(&dev->object_name_lock); -out_put: + if (ret) { + /* hmm, if driver attached, we are relying on the + * free-object path to detach.. which seems ok.. + */ + if (*handle) + drm_gem_handle_delete(file_priv, *handle); + } + if (!IS_ERR_OR_NULL(obj)) + drm_gem_object_unreference_unlocked(obj); dma_buf_put(dma_buf); - mutex_unlock(&file_priv->prime.lock); return ret; } EXPORT_SYMBOL(drm_gem_prime_fd_to_handle);