Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a35f2e2b authored by Roland Dreier's avatar Roland Dreier Committed by Dave Airlie
Browse files

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: default avatarJesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: default avatarJesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: default avatarRoland Dreier <rolandd@cisco.com>
Signed-off-by: default avatarEric Anholt <eric@anholt.net>
Signed-off-by: default avatarDave Airlie <airlied@linux.ie>
parent 96dec61d

drivers/gpu/drm/i915/i915_gem.c

+12 −9
Original line number Diff line number Diff line
@@ -2634,6 +2634,16 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, 


	i915_verify_inactive(dev, __FILE__, __LINE__);



err:

	for (i = 0; i < pinned; i++)

		i915_gem_object_unpin(object_list[i]);



	for (i = 0; i < args->buffer_count; i++)

		drm_gem_object_unreference(object_list[i]);



	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,
@@ -2643,14 +2653,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, 
			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]);



	for (i = 0; i < args->buffer_count; i++)

		drm_gem_object_unreference(object_list[i]);



	mutex_unlock(&dev->struct_mutex);

	}



pre_mutex_err:

	drm_free(object_list, sizeof(*object_list) * args->buffer_count,