[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <202511171253.0E307E6@keescook>
Date: Mon, 17 Nov 2025 12:55:37 -0800
From: Kees Cook <kees@...nel.org>
To: Tzung-Bi Shih <tzungbi@...nel.org>
Cc: Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] lkdtm: Add lockdep related crash tests
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
Otherwise looks good. It'd be nice to get lockdep maintainers on CC as
well too.
-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