linux-kernel - Re: [Bug #12491] i915 lockdep warning

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1234312992.32506.30.camel@gaiman>
Date:	Tue, 10 Feb 2009 16:43:12 -0800
From:	Eric Anholt <eric@...olt.net>
To:	Roland Dreier <rdreier@...co.com>
Cc:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Kernel Testers List <kernel-testers@...r.kernel.org>,
	"drivers_video-dri@...nel-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

On Fri, 2009-02-06 at 17:48 -0800, Roland Dreier wrote:
> [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>

This only fixes 1 of the about 4 of this type of deadlock, but it looks
like a fine fix for that and I've put it in my tree.  Thanks!

(I'd been holding off on hitting the easy ones to figure out if we could
get the lock order the other way around.  It would be really nice if we
could have struct_mutex -> mmap_sem, but it looks like that's just not
going to happen.  pread and pwrite are going to be a mess.)

-- 
Eric Anholt
eric@...olt.net                         eric.anholt@...el.com



Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)