[<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