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] [day] [month] [year] [list]
Message-ID: <aRxS9Nocb-e7Jjo3@google.com>
Date: Tue, 18 Nov 2025 11:05:24 +0000
From: Tzung-Bi Shih <tzungbi@...nel.org>
To: Kees Cook <kees@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
	Boqun Feng <boqun.feng@...il.com>, Waiman Long <longman@...hat.com>
Subject: Re: [PATCH v2] lkdtm: Add lockdep related crash tests

On Mon, Nov 17, 2025 at 12:55:37PM -0800, Kees Cook wrote:
> On Mon, Nov 17, 2025 at 03:33:37AM +0000, Tzung-Bi Shih wrote:
> > Introduce various lockdep related crash tests.
> 
> Cool! Do the existing lockdep tests not cover these things? And are
> these tests "survivable"? I ask because I'd like to see
> tools/testing/selftests/lkdtm/tests.txt updated at the same time when
> new tests are added, so they can either get tested, or have some
> documentation about why that can't be tested by a CI. e.g.:
> 
> #PANIC_STOP_IRQOFF Crashes entire system

To provide more context: I trigger them manually for testing [1].

Some of them should just comment out in the tests.txt as they crash the
system.  As for the rest of them, theoretically they can only be triggered 
once to see the target warning message for each boot:
- lockdep_reset() might help somehow but we need to figure out a way for
  kselftest to call it.
- lkdtm_LOCKDEP_HELD_LOCK() acquires lock_A which makes the lock is
  unavailable for further tests.
- ...

I'm wondering if we might comment all of them out in the end.

[1] https://lore.kernel.org/lkml/20251114062730.1828416-1-tzungbi@kernel.org/T/#u

> 
> Otherwise looks good. It'd be nice to get lockdep maintainers on CC as
> well too.

Added them to the CC list to get further inputs.

> 
> -Kees
> 
> > 
> > Signed-off-by: Tzung-Bi Shih <tzungbi@...nel.org>
> > ---
> > v2:
> > - Fix "warning: suggest braces around empty body in an 'else' statement [-Wempty-body]"
> >   reported by 0day test robot.
> > 
> > v1: https://lore.kernel.org/lkml/20251114062535.1827309-1-tzungbi@kernel.org/T/#u
> > 
> >  drivers/misc/lkdtm/Makefile  |  1 +
> >  drivers/misc/lkdtm/core.c    |  1 +
> >  drivers/misc/lkdtm/lkdtm.h   |  1 +
> >  drivers/misc/lkdtm/lockdep.c | 98 ++++++++++++++++++++++++++++++++++++
> >  4 files changed, 101 insertions(+)
> >  create mode 100644 drivers/misc/lkdtm/lockdep.c
> > 
> > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> > index 03ebe33185f9..830b71c8e6a0 100644
> > --- a/drivers/misc/lkdtm/Makefile
> > +++ b/drivers/misc/lkdtm/Makefile
> > @@ -11,6 +11,7 @@ lkdtm-$(CONFIG_LKDTM)		+= usercopy.o
> >  lkdtm-$(CONFIG_LKDTM)		+= kstack_erase.o
> >  lkdtm-$(CONFIG_LKDTM)		+= cfi.o
> >  lkdtm-$(CONFIG_LKDTM)		+= fortify.o
> > +lkdtm-$(CONFIG_LKDTM)		+= lockdep.o
> >  lkdtm-$(CONFIG_PPC_64S_HASH_MMU)	+= powerpc.o
> >  
> >  KASAN_SANITIZE_stackleak.o	:= n
> > diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
> > index 5732fd59a227..43e91388940f 100644
> > --- a/drivers/misc/lkdtm/core.c
> > +++ b/drivers/misc/lkdtm/core.c
> > @@ -96,6 +96,7 @@ static const struct crashtype_category *crashtype_categories[] = {
> >  	&stackleak_crashtypes,
> >  	&cfi_crashtypes,
> >  	&fortify_crashtypes,
> > +	&lockdep_crashtypes,
> >  #ifdef CONFIG_PPC_64S_HASH_MMU
> >  	&powerpc_crashtypes,
> >  #endif
> > diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
> > index 015e0484026b..d2d97e6f323e 100644
> > --- a/drivers/misc/lkdtm/lkdtm.h
> > +++ b/drivers/misc/lkdtm/lkdtm.h
> > @@ -84,6 +84,7 @@ extern struct crashtype_category usercopy_crashtypes;
> >  extern struct crashtype_category stackleak_crashtypes;
> >  extern struct crashtype_category cfi_crashtypes;
> >  extern struct crashtype_category fortify_crashtypes;
> > +extern struct crashtype_category lockdep_crashtypes;
> >  extern struct crashtype_category powerpc_crashtypes;
> >  
> >  /* Each category's init/exit routines. */
> > diff --git a/drivers/misc/lkdtm/lockdep.c b/drivers/misc/lkdtm/lockdep.c
> > new file mode 100644
> > index 000000000000..e029e9e60ce6
> > --- /dev/null
> > +++ b/drivers/misc/lkdtm/lockdep.c
> > @@ -0,0 +1,98 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2025 Google LLC
> > + *
> > + * Tests related to lockdep warnings.
> > + */
> > +#include "lkdtm.h"
> > +#include <linux/cleanup.h>
> > +#include <linux/irqflags.h>
> > +#include <linux/mutex.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/srcu.h>
> > +
> > +static DEFINE_SPINLOCK(lock_A);
> > +static DEFINE_SPINLOCK(lock_B);
> > +
> > +/* For "WARNING: possible circular locking dependency detected". */
> > +static void lkdtm_LOCKDEP_CIRCULAR_LOCK(void)
> > +{
> > +	scoped_guard(spinlock, &lock_A)
> > +		scoped_guard(spinlock, &lock_B) {}
> > +	scoped_guard(spinlock, &lock_B)
> > +		scoped_guard(spinlock, &lock_A) {}
> > +}
> > +
> > +/* For "WARNING: possible recursive locking detected". */
> > +static void lkdtm_LOCKDEP_RECURSIVE_LOCK(void)
> > +{
> > +	guard(spinlock)(&lock_A);
> > +	guard(spinlock)(&lock_A);
> > +}
> > +
> > +/* For "WARNING: inconsistent lock state". */
> > +static void lkdtm_LOCKDEP_INCONSISTENT_LOCK(void)
> > +{
> > +	lockdep_softirq_enter();
> > +	scoped_guard(spinlock, &lock_A) {}
> > +	lockdep_softirq_exit();
> > +
> > +	scoped_guard(spinlock, &lock_A) {}
> > +}
> > +
> > +/* For "WARNING: Nested lock was not taken". */
> > +static void lkdtm_LOCKDEP_NESTED_LOCK_NOT_HELD(void)
> > +{
> > +	spin_lock_nest_lock(&lock_B, &lock_A);
> > +}
> > +
> > +/* For "WARNING: bad unlock balance detected!". */
> > +static void lkdtm_LOCKDEP_BAD_UNLOCK_BALANCE(void)
> > +{
> > +	spin_unlock(&lock_A);
> > +}
> > +
> > +/* For "WARNING: held lock freed!". */
> > +static void lkdtm_LOCKDEP_HELD_LOCK_FREED(void)
> > +{
> > +	spin_lock(&lock_A);
> > +	spin_lock_init(&lock_A);
> > +}
> > +
> > +/* For "WARNING: lock held when returning to user space!". */
> > +static void lkdtm_LOCKDEP_HELD_LOCK(void)
> > +{
> > +	spin_lock(&lock_A);
> > +}
> > +
> > +/* For "WARNING: suspicious RCU usage". */
> > +static void lkdtm_LOCKDEP_SUSPICIOUS_RCU(void)
> > +{
> > +	struct srcu_struct srcu;
> > +	void __rcu *res = NULL;
> > +	int idx;
> > +
> > +	init_srcu_struct(&srcu);
> > +
> > +	idx = srcu_read_lock(&srcu);
> > +	rcu_dereference(res);
> > +	srcu_read_unlock(&srcu, idx);
> > +
> > +	cleanup_srcu_struct(&srcu);
> > +}
> > +
> > +static struct crashtype crashtypes[] = {
> > +	CRASHTYPE(LOCKDEP_CIRCULAR_LOCK),
> > +	CRASHTYPE(LOCKDEP_RECURSIVE_LOCK),
> > +	CRASHTYPE(LOCKDEP_INCONSISTENT_LOCK),
> > +	CRASHTYPE(LOCKDEP_NESTED_LOCK_NOT_HELD),
> > +	CRASHTYPE(LOCKDEP_BAD_UNLOCK_BALANCE),
> > +	CRASHTYPE(LOCKDEP_HELD_LOCK_FREED),
> > +	CRASHTYPE(LOCKDEP_HELD_LOCK),
> > +	CRASHTYPE(LOCKDEP_SUSPICIOUS_RCU),
> > +};
> > +
> > +struct crashtype_category lockdep_crashtypes = {
> > +	.crashtypes = crashtypes,
> > +	.len	    = ARRAY_SIZE(crashtypes),
> > +};
> > -- 
> > 2.52.0.rc1.455.g30608eb744-goog
> > 
> 
> -- 
> Kees Cook

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ