| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 06 Feb 2009 17:48:09 -0800
From: Roland Dreier <rdreier@...co.com>
To: "Brandeburg\, Jesse" <jesse.brandeburg@...el.com>
Cc: "Rafael J. Wysocki" <rjw@...k.pl>, Eric Anholt <eric@...olt.net>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Kernel Testers List <kernel-testers@...r.kernel.org>,
"drivers_video-dri\@kernel-bugs.osdl.org"
<drivers_video-dri@...nel-bugs.osdl.org>,
Jesse Barnes <jbarnes@...tuousgeek.org>,
intel-gfx@...ts.freedesktop.org
Subject: Re: [Bug #12491] i915 lockdep warning
[intel-gfx CC added]
> I tested this on my 965 based system that showed the issue and it seemed resolved, at least no warnings in dmesg. Lockdep was confirmed to still be enabled.
> I can mark the bug fixed in korg bugzilla too.
Probably we just want to add a comment saying the patch works -- the bug
should stay open until we get the fix upstream I guess.
> Thank you Roland for looking into this, good work!
Thanks for testing! Patch with updated changelog (including your
tested-by) below. Jesse [Barnes] and/or Eric please review and apply:
---
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@...el.com>
Tested-by: Jesse Brandeburg <jesse.brandeburg@...el.com>
Signed-off-by: Roland Dreier <rolandd@...co.com>
---
drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index debad5c..23aad8c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2610,15 +2610,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]);
@@ -2628,6 +2619,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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists