All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: kernel test robot <oliver.sang@intel.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, lkp@lists.01.org, lkp@intel.com
Subject: Re: [locking/ww_mutex]  c0afb0ffc0: BUG:kernel_NULL_pointer_dereference,address
Date: Tue, 24 Aug 2021 16:47:36 +0200	[thread overview]
Message-ID: <YSUGiNL0lkxCitSQ@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20210824140044.GA27667@xsang-OptiPlex-9020>

On Tue, Aug 24, 2021 at 10:00:44PM +0800, kernel test robot wrote:
> 
> 
> Greeting,
> 
> FYI, we noticed the following commit (built with gcc-9):
> 
> commit: c0afb0ffc06e6b4e492a3b711f1fb32074f9949c ("locking/ww_mutex: Gather mutex_waiter initialization")

Fixed by...

commit b857174e68e26f9c4f0796971e11eb63ad5a3eb6
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date:   Thu Aug 19 21:30:30 2021 +0200

    locking/ww_mutex: Initialize waiter.ww_ctx properly
    
    The consolidation of the debug code for mutex waiter intialization sets
    waiter::ww_ctx to a poison value unconditionally. For regular mutexes this
    is intended to catch the case where waiter_ww_ctx is dereferenced
    accidentally.
    
    For ww_mutex the poison value has to be overwritten either with a context
    pointer or NULL for ww_mutexes without context.
    
    The rework broke this as it made the store conditional on the context
    pointer instead of the argument which signals whether ww_mutex code should
    be compiled in or optiized out. As a result waiter::ww_ctx ends up with the
    poison pointer for contextless ww_mutexes which causes a later dereference of
    the poison pointer because it is != NULL.
    
    Use the build argument instead so for ww_mutex the poison value is always
    overwritten.
    
    Fixes: c0afb0ffc06e6 ("locking/ww_mutex: Gather mutex_waiter initialization")
    Reported-by: Guenter Roeck <linux@roeck-us.net>
    Suggested-by: Peter Zijlstra <peterz@infradead.org>
    Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Link: https://lore.kernel.org/r/20210819193030.zpwrpvvrmy7xxxiy@linutronix.de

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 3a65bf4bacfd..d456579d0952 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -618,7 +618,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 
 	debug_mutex_lock_common(lock, &waiter);
 	waiter.task = current;
-	if (ww_ctx)
+	if (use_ww_ctx)
 		waiter.ww_ctx = ww_ctx;
 
 	lock_contended(&lock->dep_map, ip);

WARNING: multiple messages have this Message-ID (diff)
From: Peter Zijlstra <peterz@infradead.org>
To: lkp@lists.01.org
Subject: Re: [locking/ww_mutex] c0afb0ffc0: BUG:kernel_NULL_pointer_dereference, address
Date: Tue, 24 Aug 2021 16:47:36 +0200	[thread overview]
Message-ID: <YSUGiNL0lkxCitSQ@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <20210824140044.GA27667@xsang-OptiPlex-9020>

[-- Attachment #1: Type: text/plain, Size: 2208 bytes --]

On Tue, Aug 24, 2021 at 10:00:44PM +0800, kernel test robot wrote:
> 
> 
> Greeting,
> 
> FYI, we noticed the following commit (built with gcc-9):
> 
> commit: c0afb0ffc06e6b4e492a3b711f1fb32074f9949c ("locking/ww_mutex: Gather mutex_waiter initialization")

Fixed by...

commit b857174e68e26f9c4f0796971e11eb63ad5a3eb6
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date:   Thu Aug 19 21:30:30 2021 +0200

    locking/ww_mutex: Initialize waiter.ww_ctx properly
    
    The consolidation of the debug code for mutex waiter intialization sets
    waiter::ww_ctx to a poison value unconditionally. For regular mutexes this
    is intended to catch the case where waiter_ww_ctx is dereferenced
    accidentally.
    
    For ww_mutex the poison value has to be overwritten either with a context
    pointer or NULL for ww_mutexes without context.
    
    The rework broke this as it made the store conditional on the context
    pointer instead of the argument which signals whether ww_mutex code should
    be compiled in or optiized out. As a result waiter::ww_ctx ends up with the
    poison pointer for contextless ww_mutexes which causes a later dereference of
    the poison pointer because it is != NULL.
    
    Use the build argument instead so for ww_mutex the poison value is always
    overwritten.
    
    Fixes: c0afb0ffc06e6 ("locking/ww_mutex: Gather mutex_waiter initialization")
    Reported-by: Guenter Roeck <linux@roeck-us.net>
    Suggested-by: Peter Zijlstra <peterz@infradead.org>
    Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Link: https://lore.kernel.org/r/20210819193030.zpwrpvvrmy7xxxiy(a)linutronix.de

diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 3a65bf4bacfd..d456579d0952 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -618,7 +618,7 @@ __mutex_lock_common(struct mutex *lock, unsigned int state, unsigned int subclas
 
 	debug_mutex_lock_common(lock, &waiter);
 	waiter.task = current;
-	if (ww_ctx)
+	if (use_ww_ctx)
 		waiter.ww_ctx = ww_ctx;
 
 	lock_contended(&lock->dep_map, ip);

  reply	other threads:[~2021-08-24 14:50 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 14:00 [locking/ww_mutex] c0afb0ffc0: BUG:kernel_NULL_pointer_dereference,address kernel test robot
2021-08-24 14:00 ` [locking/ww_mutex] c0afb0ffc0: BUG:kernel_NULL_pointer_dereference, address kernel test robot
2021-08-24 14:47 ` Peter Zijlstra [this message]
2021-08-24 14:47   ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YSUGiNL0lkxCitSQ@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=lkp@lists.01.org \
    --cc=mingo@kernel.org \
    --cc=oliver.sang@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.