drm/i915: Fix potential AB-BA deadlock in i915_gem_execbuffer()

Lockdep warns that i915_gem_execbuffer() can trigger a page fault (which
takes mmap_sem) while holding dev->struct_mutex, while drm_vm_open()
(which is called with mmap_sem already held) takes dev->struct_mutex.
So this is a potential AB-BA deadlock.

The way that i915_gem_execbuffer() triggers a page fault is by doing
copy_to_user() when returning new buffer offsets back to userspace;
however there is no reason to hold the struct_mutex when doing this
copy, since what is being copied is the contents of an array private to
i915_gem_execbuffer() anyway.  So we can fix the potential deadlock (and
get rid of the lockdep warning) by simply moving the copy_to_user()
outside of where struct_mutex is held.

This fixes <http://bugzilla.kernel.org/show_bug.cgi?id=12491>.

Reported-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Roland Dreier <rolandd@cisco.com>
Signed-off-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Dave Airlie <airlied@linux.ie>

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 55f4c060fa01d83b8f066376431e1a02803e0f4c..ff0d94dd35505c794ba92580ddeb9510ff829de8 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2634,15 +2634,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
 
        i915_verify_inactive(dev, __FILE__, __LINE__);
 
-       /* Copy the new buffer offsets back to the user's exec list. */
-       ret = copy_to_user((struct drm_i915_relocation_entry __user *)
-                          (uintptr_t) args->buffers_ptr,
-                          exec_list,
-                          sizeof(*exec_list) * args->buffer_count);
-       if (ret)
-               DRM_ERROR("failed to copy %d exec entries "
-                         "back to user (%d)\n",
-                          args->buffer_count, ret);
 err:
        for (i = 0; i < pinned; i++)
                i915_gem_object_unpin(object_list[i]);
@@ -2652,6 +2643,18 @@ err:
 
        mutex_unlock(&dev->struct_mutex);
 
+       if (!ret) {
+               /* Copy the new buffer offsets back to the user's exec list. */
+               ret = copy_to_user((struct drm_i915_relocation_entry __user *)
+                                  (uintptr_t) args->buffers_ptr,
+                                  exec_list,
+                                  sizeof(*exec_list) * args->buffer_count);
+               if (ret)
+                       DRM_ERROR("failed to copy %d exec entries "
+                                 "back to user (%d)\n",
+                                 args->buffer_count, ret);
+       }
+
 pre_mutex_err:
        drm_free(object_list, sizeof(*object_list) * args->buffer_count,
                 DRM_MEM_DRIVER);