[PATCH] drm/prime: Move all unreferences on fd_to_handle error paths to after unlock

[Chris Wilson <chris@chris-wilson.co.uk>]

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);
-- 
2.6.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel