* [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
@ 2025-09-24 2:25 buckzhang1212
2025-09-24 3:10 ` Waiman Long
2025-09-24 16:15 ` kernel test robot
0 siblings, 2 replies; 5+ messages in thread
From: buckzhang1212 @ 2025-09-24 2:25 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
linux-kernel
Cc: buckzhang1212
From: "buck.zhang" <buckzhang1212@yeah.net>
Here is a kernel exception about mutex and I can recreate it stably.
First we define a custome struct that includes a mutex lock.
Then allocate this struct by kmalloc without calling mutex_init.
Finally when multiple tasks call mutex_lock together,kernel will panic.
But Kernel is good if only one task call this mutex at the same time.
the exception reason is that lock->wait_list is an invalid kernel list.
kernel crash log:
Unable to handle kernel NULL pointer dereference at virtual address 0000000
pc: __mutex_add_waiter+0x68/0x160
lr: __mutex_add_waiter+0x128/0x160
sp: ffffffc0866f3ac0
x29: ffffffc0866f3ad0 x28: ffffff8095148000 x27: 0000000000000000
x2: ffffffc0866f3b18 x1 : 0000000000000000 x0 : 0000000000000000
Call trace:
__mutex_add_waiter+0x68/0x160
__mutex_lock+0x48c/0x119c
__mutex_lock_slowpath+0x1c/0x2c
mutex_lock+0x48/0x144
Test case:
struct chip_mutex {
struct mutex tmutex;
};
static void work_handler1(struct chip_mutex *cmutex)
{
mutex_lock(&(cmutex->tmutex));
}
static void work_handler2(struct chip_mutex *cmutex)
{
mutex_lock(&(cmutex->tmutex));
}
static void chip_tmutex(void)
{
struct chip_mutex *cmutex;
cmutex = kzalloc(sizeof(struct chip_mutex),GFP_KERNEL);
work_handler1(cmutex);
------
work_handler2(cmutex);
}
Signed-off-by: buck.zhang <buckzhang1212@yeah.net>
---
kernel/locking/mutex.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index de7d6702c..8fbe858c8 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -42,6 +42,16 @@
#else
# define MUTEX_WARN_ON(cond)
#endif
+#define MUTEX_CHCEK_INIT(lock) mutex_check_waitlist(lock)
+static void mutex_check_waitlist(struct mutex *lock)
+{
+ struct list_head *list = &lock->wait_list;
+
+ if ((unsigned long)list->next < PAGE_OFFSET) {
+ pr_err("BUG: mutex lock is uninitialized,wait_list is Error\n");
+ MUTEX_WARN_ON("mutex lock is uninitialized");
+ }
+}
void
__mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
@@ -269,6 +279,7 @@ static void __sched __mutex_lock_slowpath(struct mutex *lock);
void __sched mutex_lock(struct mutex *lock)
{
might_sleep();
+ MUTEX_CHCEK_INIT(lock);
if (!__mutex_trylock_fast(lock))
__mutex_lock_slowpath(lock);
@@ -991,6 +1002,7 @@ __mutex_lock_interruptible_slowpath(struct mutex *lock);
int __sched mutex_lock_interruptible(struct mutex *lock)
{
might_sleep();
+ MUTEX_CHCEK_INIT(lock);
if (__mutex_trylock_fast(lock))
return 0;
@@ -1015,6 +1027,7 @@ EXPORT_SYMBOL(mutex_lock_interruptible);
int __sched mutex_lock_killable(struct mutex *lock)
{
might_sleep();
+ MUTEX_CHCEK_INIT(lock);
if (__mutex_trylock_fast(lock))
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
2025-09-24 2:25 [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock buckzhang1212
@ 2025-09-24 3:10 ` Waiman Long
2025-09-24 6:58 ` Peter Zijlstra
[not found] ` <377cba8b.2bd3a.19979cf5b04.Coremail.buckzhang1212@yeah.net>
2025-09-24 16:15 ` kernel test robot
1 sibling, 2 replies; 5+ messages in thread
From: Waiman Long @ 2025-09-24 3:10 UTC (permalink / raw)
To: buckzhang1212, Peter Zijlstra, Ingo Molnar, Will Deacon,
Boqun Feng, linux-kernel
On 9/23/25 10:25 PM, buckzhang1212@yeah.net wrote:
> From: "buck.zhang" <buckzhang1212@yeah.net>
>
> Here is a kernel exception about mutex and I can recreate it stably.
> First we define a custome struct that includes a mutex lock.
> Then allocate this struct by kmalloc without calling mutex_init.
> Finally when multiple tasks call mutex_lock together,kernel will panic.
> But Kernel is good if only one task call this mutex at the same time.
> the exception reason is that lock->wait_list is an invalid kernel list.
> kernel crash log:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000
> pc: __mutex_add_waiter+0x68/0x160
> lr: __mutex_add_waiter+0x128/0x160
> sp: ffffffc0866f3ac0
> x29: ffffffc0866f3ad0 x28: ffffff8095148000 x27: 0000000000000000
> x2: ffffffc0866f3b18 x1 : 0000000000000000 x0 : 0000000000000000
> Call trace:
> __mutex_add_waiter+0x68/0x160
> __mutex_lock+0x48c/0x119c
> __mutex_lock_slowpath+0x1c/0x2c
> mutex_lock+0x48/0x144
> Test case:
> struct chip_mutex {
> struct mutex tmutex;
> };
> static void work_handler1(struct chip_mutex *cmutex)
> {
> mutex_lock(&(cmutex->tmutex));
> }
> static void work_handler2(struct chip_mutex *cmutex)
> {
> mutex_lock(&(cmutex->tmutex));
> }
> static void chip_tmutex(void)
> {
> struct chip_mutex *cmutex;
> cmutex = kzalloc(sizeof(struct chip_mutex),GFP_KERNEL);
> work_handler1(cmutex);
> ------
> work_handler2(cmutex);
> }
>
> Signed-off-by: buck.zhang <buckzhang1212@yeah.net>
A mutex must be properly initialized before it can be used. The kernel
panic you listed above is expected and the panic itself indicates that
the code is wrong.
> ---
> kernel/locking/mutex.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
> index de7d6702c..8fbe858c8 100644
> --- a/kernel/locking/mutex.c
> +++ b/kernel/locking/mutex.c
> @@ -42,6 +42,16 @@
> #else
> # define MUTEX_WARN_ON(cond)
> #endif
> +#define MUTEX_CHCEK_INIT(lock) mutex_check_waitlist(lock)
> +static void mutex_check_waitlist(struct mutex *lock)
> +{
> + struct list_head *list = &lock->wait_list;
> +
> + if ((unsigned long)list->next < PAGE_OFFSET) {
> + pr_err("BUG: mutex lock is uninitialized,wait_list is Error\n");
> + MUTEX_WARN_ON("mutex lock is uninitialized");
> + }
> +}
>
> void
> __mutex_init(struct mutex *lock, const char *name, struct lock_class_key *key)
> @@ -269,6 +279,7 @@ static void __sched __mutex_lock_slowpath(struct mutex *lock);
> void __sched mutex_lock(struct mutex *lock)
> {
> might_sleep();
> + MUTEX_CHCEK_INIT(lock);
>
> if (!__mutex_trylock_fast(lock))
> __mutex_lock_slowpath(lock);
This check has provided no additional value and it slows down the
locking fast path.
NACK
> @@ -991,6 +1002,7 @@ __mutex_lock_interruptible_slowpath(struct mutex *lock);
> int __sched mutex_lock_interruptible(struct mutex *lock)
> {
> might_sleep();
> + MUTEX_CHCEK_INIT(lock);
>
> if (__mutex_trylock_fast(lock))
> return 0;
> @@ -1015,6 +1027,7 @@ EXPORT_SYMBOL(mutex_lock_interruptible);
> int __sched mutex_lock_killable(struct mutex *lock)
> {
> might_sleep();
> + MUTEX_CHCEK_INIT(lock);
>
> if (__mutex_trylock_fast(lock))
> return 0;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
2025-09-24 3:10 ` Waiman Long
@ 2025-09-24 6:58 ` Peter Zijlstra
[not found] ` <377cba8b.2bd3a.19979cf5b04.Coremail.buckzhang1212@yeah.net>
1 sibling, 0 replies; 5+ messages in thread
From: Peter Zijlstra @ 2025-09-24 6:58 UTC (permalink / raw)
To: Waiman Long
Cc: buckzhang1212, Ingo Molnar, Will Deacon, Boqun Feng, linux-kernel
On Tue, Sep 23, 2025 at 11:10:22PM -0400, Waiman Long wrote:
> On 9/23/25 10:25 PM, buckzhang1212@yeah.net wrote:
> A mutex must be properly initialized before it can be used. The kernel panic
> you listed above is expected and the panic itself indicates that the code is
> wrong.
> > @@ -269,6 +279,7 @@ static void __sched __mutex_lock_slowpath(struct mutex *lock);
> > void __sched mutex_lock(struct mutex *lock)
> > {
> > might_sleep();
> > + MUTEX_CHCEK_INIT(lock);
> > if (!__mutex_trylock_fast(lock))
> > __mutex_lock_slowpath(lock);
>
> This check has provided no additional value and it slows down the locking
> fast path.
>
> NACK
Agreed. Additionally we have CONFIG_DEBUG_MUTEXES. If you feel there is
a check missing there -- you could argue that debug_mutex_lock_common()
should have something like:
DEBUG_LOCK_WARN_ON(lock->magic != lock);
feel free to send a patch for that.
But don't go sprinkle debug code in !debug builds.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
[not found] ` <377cba8b.2bd3a.19979cf5b04.Coremail.buckzhang1212@yeah.net>
@ 2025-09-24 14:22 ` Waiman Long
0 siblings, 0 replies; 5+ messages in thread
From: Waiman Long @ 2025-09-24 14:22 UTC (permalink / raw)
To: buckzhang1212, llong@redhat.com
Cc: peterz@infradead.org, mingo@redhat.com, will@kernel.org,
boqun.feng@gmail.com, linux-kernel@vger.kernel…
On 9/23/25 11:40 PM, buckzhang1212 wrote:
> Hey Waiman
> Thank you for your reply.
> I totally agree with you that A mutex must be properly initialized
> before it can be used.
> But my customer engineer often make the mistake like this in TP or
> fingerprint driver.
> I want to warn this mistake at first because it can be used normally
> mostly.
> when enable CONFIG_DEBUG_MUTEXES,we can get more informations.
> This check has provided no additional value and it slows down the
> locking fast path.
> ---- how about move the warning to __mutex_lock_slowpath?
> Can you give me some advices?
As suggested by Peter, additional checks like that should only be
enabled in the debug portion of the code (i.e. within
CONFIG_DEBUG_MUTEXES or other debug kconfig option). The normal way we
test the kernel patches is to build a debug kernel with all the
appropriate debug options enabled and use it for testing purpose as
performance isn't a concern. This enables all the debug code to be
activated and check for correctness. Don't rely your testing just on the
production kernel unless you are testing for some performance related
metrics. Even then, both the debug and production kernels should be used.
Hope this help.
Cheers,
Longman
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
2025-09-24 2:25 [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock buckzhang1212
2025-09-24 3:10 ` Waiman Long
@ 2025-09-24 16:15 ` kernel test robot
1 sibling, 0 replies; 5+ messages in thread
From: kernel test robot @ 2025-09-24 16:15 UTC (permalink / raw)
To: buckzhang1212, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long,
linux-kernel
Cc: oe-kbuild-all, buckzhang1212
Hi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tip/locking/core]
[also build test WARNING on linus/master v6.17-rc7 next-20250923]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/buckzhang1212-yeah-net/locking-mutex-add-MUTEX_CHCEK_INIT-to-detect-uninitialized-mutex-lock/20250924-103805
base: tip/locking/core
patch link: https://lore.kernel.org/r/20250924022500.2577-1-buckzhang1212%40yeah.net
patch subject: [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock
config: i386-randconfig-014-20250924 (https://download.01.org/0day-ci/archive/20250924/202509242322.VggjpcD5-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250924/202509242322.VggjpcD5-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202509242322.VggjpcD5-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/locking/mutex.c:46:13: warning: 'mutex_check_waitlist' defined but not used [-Wunused-function]
46 | static void mutex_check_waitlist(struct mutex *lock)
| ^~~~~~~~~~~~~~~~~~~~
vim +/mutex_check_waitlist +46 kernel/locking/mutex.c
39
40 #ifdef CONFIG_DEBUG_MUTEXES
41 # define MUTEX_WARN_ON(cond) DEBUG_LOCKS_WARN_ON(cond)
42 #else
43 # define MUTEX_WARN_ON(cond)
44 #endif
45 #define MUTEX_CHCEK_INIT(lock) mutex_check_waitlist(lock)
> 46 static void mutex_check_waitlist(struct mutex *lock)
47 {
48 struct list_head *list = &lock->wait_list;
49
50 if ((unsigned long)list->next < PAGE_OFFSET) {
51 pr_err("BUG: mutex lock is uninitialized,wait_list is Error\n");
52 MUTEX_WARN_ON("mutex lock is uninitialized");
53 }
54 }
55
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-24 16:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-24 2:25 [PATCH] locking/mutex:add MUTEX_CHCEK_INIT to detect uninitialized mutex lock buckzhang1212
2025-09-24 3:10 ` Waiman Long
2025-09-24 6:58 ` Peter Zijlstra
[not found] ` <377cba8b.2bd3a.19979cf5b04.Coremail.buckzhang1212@yeah.net>
2025-09-24 14:22 ` Waiman Long
2025-09-24 16:15 ` kernel test robot
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.