[Intel-gfx] [PATCH] drm/i915: Fix mutex->owner inspection race under DEBUG_MUTEXES

[Chris Wilson]

On Wed, Jan 07, 2015 at 03:27:41PM +0200, Jani Nikula wrote:
> On Wed, 07 Jan 2015, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Wed, Jan 07, 2015 at 02:12:14PM +0200, Jani Nikula wrote:
> >> On Mon, 05 Jan 2015, Daniel Vetter <daniel at ffwll.ch> wrote:
> >> > On Fri, Jan 02, 2015 at 09:47:10AM +0000, Chris Wilson wrote:
> >> >> If CONFIG_DEBUG_MUTEXES is set, the mutex->owner field is only cleared
> >> >> if the mutex debugging is enabled which introduces a race in our
> >> >> mutex_is_locked_by() - i.e. we may inspect the old owner value before it
> >> >> is acquired by the new task.
> >> >> 
> >> >> This is the root cause of this error:
> >> >> 
> >> >> diff --git a/kernel/locking/mutex-debug.c b/kernel/locking/mutex-debug.c
> >> >> index 5cf6731..3ef3736 100644
> >> >> --- a/kernel/locking/mutex-debug.c
> >> >> +++ b/kernel/locking/mutex-debug.c
> >> >> @@ -80,13 +80,13 @@ void debug_mutex_unlock(struct mutex *lock)
> >> >>                         DEBUG_LOCKS_WARN_ON(lock->owner != current);
> >> >> 
> >> >>                 DEBUG_LOCKS_WARN_ON(!lock->wait_list.prev && !lock->wait_list.next);
> >> >> -               mutex_clear_owner(lock);
> >> >>         }
> >> >> 
> >> >>         /*
> >> >>          * __mutex_slowpath_needs_to_unlock() is explicitly 0 for debug
> >> >>          * mutexes so that we can do it here after we've verified state.
> >> >>          */
> >> >> +       mutex_clear_owner(lock);
> >> >>         atomic_set(&lock->count, 1);
> >> >>  }
> >> >> 
> >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87955
> >> >> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> >> Cc: stable at vger.kernel.org
> >> >
> >> > Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> 
> >> Pushed to drm-intel-fixes, thanks for the patch and review.
> >
> > For the record, I plan to post a revert of this to -next if the core fix
> > lands upstream (looks good so far).
> 
> Does it look like that could be cc:stable?

I didn't suggest it should be, so I wait with bated breath. I know
disabling lock-stealing in i915 is fairly safe (some corner cases may
now get oom, but that's a perenial risk anyway) and so felt more
comfortable with pushing this i915 patch through stable.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre