[PATCH v6 0/4] rv/reactors: fix lockdep warning and add tests

[PATCH v6 0/4] rv/reactors: fix lockdep warning and add tests

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

We occasionally hit a lockdep "Invalid wait context" warning when
a reactor callback is preempted by a timer interrupt.
On interrupt exit the scheduler takes rq->__lock (LD_WAIT_SPIN) while
rv_react() still holds its wait-type-override map, which declared
LD_WAIT_FREE.  On any kernel where the task context has preemption
enabled (not just CONFIG_PREEMPT_RT) this triggers a spurious lockdep
report:

    [ BUG: Invalid wait context ]
    1 lock held by kunit_try_catch/209:
     #0: (rv_react_map-wait-type-override){+.+.}-{1:1}
    kunit_try_catch/209 is trying to lock:
    ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}

This series fixes the wait type (patch 1), makes reactor registration
failures propagate (patch 2), adds support for module-based
reactors (patch 3), and adds KUnit/kselftest coverage (patch 4-5).

Changes in v5:
- Patch 1: Fix typo. Functional code unchanged.
- Patch 2: Unchanged.
- Patch 3: Only exported the registration helpers as v4.
- Patch 4: Fix typo. Functional code unchanged.

v5: https://lore.kernel.org/lkml/cover.1788705281.git.wen.yang@linux.dev/
v4: https://lore.kernel.org/lkml/cover.1787854397.git.wen.yang@linux.dev/
v3: https://lore.kernel.org/lkml/cover.1786294920.git.wen.yang@linux.dev/
v2: https://lore.kernel.org/lkml/cover.1785695669.git.wen.yang@linux.dev/
v1: https://lore.kernel.org/lkml/cover.1781541556.git.wen.yang@linux.dev/

Wen Yang (4):
  rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
  rv/reactors: propagate rv_register_reactor() error from reactor init
  rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
  rv/reactors: add KUnit tests for reactor registration and dispatch

 kernel/trace/rv/Kconfig             |  12 +++
 kernel/trace/rv/Makefile            |   1 +
 kernel/trace/rv/reactor_panic.c     |   3 +-
 kernel/trace/rv/reactor_printk.c    |   3 +-
 kernel/trace/rv/rv_reactors.c       |  12 ++-
 kernel/trace/rv/rv_reactors_kunit.c | 111 ++++++++++++++++++++++++++++
 6 files changed, 137 insertions(+), 5 deletions(-)
 create mode 100644 kernel/trace/rv/rv_reactors_kunit.c

-- 
2.25.1

[PATCH v6 1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

rv_react() overrides the lockdep wait type to LD_WAIT_FREE to enforce
that reactor callbacks take no locks. But callbacks run in the context
of the triggering tracepoint, which can be preemptible task context on
any kernel. A timer interrupt firing during the callback makes the
interrupt-exit path schedule and take rq->__lock (LD_WAIT_SPIN) while
the LD_WAIT_FREE override is still held, producing a spurious
"Invalid wait context" warning:

    [ BUG: Invalid wait context ]
    context-{5:5}
    1 lock held by kunit_try_catch/209:
     #0: (rv_react_map-wait-type-override){+.+.}-{1:1}
    kunit_try_catch/209 is trying to lock:
    ffff8a743ed3e8a0 (&rq->__lock){-...}-{2:2}

Use LD_WAIT_SPIN instead of LD_WAIT_FREE, which causes false-positive
warnings in preemptible contexts due to scheduler preemption taking
rq->__lock.

Fixes: 69d8895cb9a9 ("rv: Add explicit lockdep context for reactors")
Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
Cc: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 kernel/trace/rv/rv_reactors.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index 2f5fc8d18dea..ff7d478227c3 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -465,7 +465,15 @@ int init_rv_reactors(struct dentry *root_dir)
 
 void rv_react(struct rv_monitor *monitor, const char *msg, ...)
 {
-	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_FREE);
+	/*
+	 * Reactors must not explicitly take locks, so they should be
+	 * LD_WAIT_FREE.  However, reactor callbacks can run with preemption
+	 * enabled, meaning the preempting code (e.g. the scheduler taking
+	 * rq->__lock at LD_WAIT_SPIN) may violate that constraint.  Use
+	 * LD_WAIT_SPIN to avoid false-positive lockdep reports.
+	 * But you should still NOT be using locks in reactors.
+	 */
+	static DEFINE_WAIT_OVERRIDE_MAP(rv_react_map, LD_WAIT_SPIN);
 	va_list args;
 
 	if (!rv_reacting_on() || !monitor->react)
-- 
2.25.1

[PATCH v6 2/4] rv/reactors: propagate rv_register_reactor() error from reactor init

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

Both register_react_printk() and register_react_panic() ignore the
return value of rv_register_reactor() and always return 0.  If the
registration fails (e.g. a duplicate reactor name), the init functions
silently report success even though the reactor was not registered.

Propagate the error from rv_register_reactor() so a failed registration
is reported instead of being silently ignored.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Reviewed-by: Nam Cao <namcao@linutronix.de>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/reactor_panic.c  | 3 +--
 kernel/trace/rv/reactor_printk.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/trace/rv/reactor_panic.c b/kernel/trace/rv/reactor_panic.c
index 76537b8a4343..db7116ceafff 100644
--- a/kernel/trace/rv/reactor_panic.c
+++ b/kernel/trace/rv/reactor_panic.c
@@ -26,8 +26,7 @@ static struct rv_reactor rv_panic = {
 
 static int __init register_react_panic(void)
 {
-	rv_register_reactor(&rv_panic);
-	return 0;
+	return rv_register_reactor(&rv_panic);
 }
 
 static void __exit unregister_react_panic(void)
diff --git a/kernel/trace/rv/reactor_printk.c b/kernel/trace/rv/reactor_printk.c
index 48c934e315b3..002a10f6aa7b 100644
--- a/kernel/trace/rv/reactor_printk.c
+++ b/kernel/trace/rv/reactor_printk.c
@@ -25,8 +25,7 @@ static struct rv_reactor rv_printk = {
 
 static int __init register_react_printk(void)
 {
-	rv_register_reactor(&rv_printk);
-	return 0;
+	return rv_register_reactor(&rv_printk);
 }
 
 static void __exit unregister_react_printk(void)
-- 
2.25.1

[PATCH v6 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

rv_react() is exported to modules, but the reactor registration helpers
are not.  Export them with EXPORT_SYMBOL_GPL() so reactor modules and
the tristate KUnit test module can register and unregister reactors
without hitting undefined symbol errors at link time(modpost).

Commit 3d3800b4f7f4 ("rv: Remove reactor's reference counter") noted
that module-based reactors, if supported, should be pinned with
try_module_get()/module_put() to prevent unloading under an active
monitor. This patch does not add that support. The only in-tree module
consumer, the tristate KUnit reactor test module, registers and
unregisters its reactor within a controlled test lifecycle.
The actual pinning is left to a follow-up series that adds support for
loadable-module reactors.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/rv_reactors.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
index ff7d478227c3..8e57a95d446b 100644
--- a/kernel/trace/rv/rv_reactors.c
+++ b/kernel/trace/rv/rv_reactors.c
@@ -314,6 +314,7 @@ int rv_register_reactor(struct rv_reactor *reactor)
 	guard(mutex)(&rv_interface_lock);
 	return __rv_register_reactor(reactor);
 }
+EXPORT_SYMBOL_GPL(rv_register_reactor);
 
 /**
  * rv_unregister_reactor - unregister a rv reactor.
@@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
 	list_del(&reactor->list);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(rv_unregister_reactor);
 
 /*
  * reacting_on interface.
-- 
2.25.1

Re: [PATCH v6 3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Exporting `rv_register_reactor` and `rv_unregister_reactor` allows loadable modules to register reactors, but failing to detach the reactor from active monitors creates a critical execute-after-free vulnerability when the module is unloaded.
--

commit 516262534066676c40b564e9e3018cd406dd5532
Author: Wen Yang <wen.yang@linux.dev>

rv/reactors: export rv_register_reactor() and rv_unregister_reactor()

This commit exports rv_register_reactor() and rv_unregister_reactor()
with EXPORT_SYMBOL_GPL() so that reactor modules and the tristate KUnit
test module can register and unregister reactors.

> diff --git a/kernel/trace/rv/rv_reactors.c b/kernel/trace/rv/rv_reactors.c
> index ff7d478227c32..8e57a95d446b5 100644
> --- a/kernel/trace/rv/rv_reactors.c
> +++ b/kernel/trace/rv/rv_reactors.c
> @@ -314,6 +314,7 @@ int rv_register_reactor(struct rv_reactor *reactor)
>  	guard(mutex)(&rv_interface_lock);
>  	return __rv_register_reactor(reactor);
>  }
> +EXPORT_SYMBOL_GPL(rv_register_reactor);

[Severity: High]
Does exporting this API without module pinning allow an execute-after-free
if a module is unloaded?

Although the commit message notes that actual pinning is left to a follow-up
series, exporting the API in this patch makes it immediately usable and
inherently unsafe for any module. This includes the in-tree KUnit module,
which can race with a sysfs write during its test execution.

>  
>  /**
>   * rv_unregister_reactor - unregister a rv reactor.
> @@ -327,6 +328,7 @@ int rv_unregister_reactor(struct rv_reactor *reactor)
>  	list_del(&reactor->list);
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(rv_unregister_reactor);

[Severity: High]
Can unregistering a reactor from a loadable module leave a dangling pointer
in an active monitor?

If a loadable module registers a reactor, and userspace assigns it to an
active monitor via /sys/kernel/tracing/rv/monitors/*/reactors, unloading
the module calls rv_unregister_reactor().

This function removes the reactor from the global list but fails to detach
it from the active monitor. When the monitor subsequently triggers, it will
dereference the freed memory in rv_react():

kernel/trace/rv/rv_reactors.c:rv_react() {
    ...
    monitor->react(msg, args);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789412799.git.wen.yang@linux.dev?part=3

[PATCH v6 4/4] rv/reactors: add KUnit tests for reactor registration and dispatch

[wen.yang]

From: Wen Yang <wen.yang@linux.dev>

Add KUnit tests covering the reactor register/unregister lifecycle
(including duplicate and name-length rejection) and rv_react() dispatch
(a no-op without a callback, exactly one invocation with one).  The
mdelay() callback keeps the CPU busy so a timer interrupt lands inside
rv_react()'s lockdep context, exercising the LD_WAIT_SPIN wait type
from the previous patch; a spurious lockdep splat there would show up
in the test output.

The dispatch tests rely on reacting_on being enabled, since rv_react()
returns early when it is off.

Reviewed-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Wen Yang <wen.yang@linux.dev>
---
 kernel/trace/rv/Kconfig             |  12 +++
 kernel/trace/rv/Makefile            |   1 +
 kernel/trace/rv/rv_reactors_kunit.c | 111 ++++++++++++++++++++++++++++
 3 files changed, 124 insertions(+)
 create mode 100644 kernel/trace/rv/rv_reactors_kunit.c

diff --git a/kernel/trace/rv/Kconfig b/kernel/trace/rv/Kconfig
index efa930f94ea4..9bfd429ffdea 100644
--- a/kernel/trace/rv/Kconfig
+++ b/kernel/trace/rv/Kconfig
@@ -113,6 +113,18 @@ config RV_REACT_PANIC
 	  Enables the panic reactor. The panic reactor emits a printk()
 	  message if an exception is found and panic()s the system.
 
+config RV_REACTORS_KUNIT
+	tristate "KUnit tests for RV reactors" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	depends on RV_REACTORS
+	default KUNIT_ALL_TESTS
+	help
+	  Enable KUnit tests for RV reactor registration and dispatch.
+	  These tests verify the register/unregister lifecycle, duplicate
+	  rejection, and that rv_react() correctly invokes callbacks.
+
+	  If unsure, say N.
+
 config RV_MONITORS_KUNIT_TEST
 	tristate "KUnit tests for RV monitors" if !KUNIT_ALL_TESTS
 	depends on KUNIT && RV && RV_REACTORS
diff --git a/kernel/trace/rv/Makefile b/kernel/trace/rv/Makefile
index cdbf68c84f5a..c895d81dfdad 100644
--- a/kernel/trace/rv/Makefile
+++ b/kernel/trace/rv/Makefile
@@ -25,4 +25,5 @@ obj-$(CONFIG_RV_MON_WAKEUP) += monitors/wakeup/wakeup.o
 obj-$(CONFIG_RV_REACTORS) += rv_reactors.o
 obj-$(CONFIG_RV_REACT_PRINTK) += reactor_printk.o
 obj-$(CONFIG_RV_REACT_PANIC) += reactor_panic.o
+obj-$(CONFIG_RV_REACTORS_KUNIT) += rv_reactors_kunit.o
 obj-$(CONFIG_RV_MONITORS_KUNIT_TEST) += rv_monitors_test.o
diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
new file mode 100644
index 000000000000..3850c6dda288
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for RV reactor registration and dispatch.
+ *
+ * The dispatch tests rely on reacting_on being enabled, since rv_react()
+ * returns early when it is off. It is on by default when the suites run
+ * built-in; as a module, re-enable it if disabled via
+ * /sys/kernel/tracing/rv/reacting_on.
+ */
+
+#include <kunit/test.h>
+#include <linux/build_bug.h>
+#include <linux/rv.h>
+#include <linux/delay.h>
+#include "rv.h"
+
+static struct rv_reactor test_reactor = {
+	.name		= "kunit_test_reactor",
+	.description	= "KUnit test reactor",
+};
+
+static void reactor_teardown(void *arg)
+{
+	rv_unregister_reactor(&test_reactor);
+}
+
+static void register_test_reactor(struct kunit *test)
+{
+	KUNIT_ASSERT_EQ(test, rv_register_reactor(&test_reactor), 0);
+	KUNIT_ASSERT_EQ(test,
+			kunit_add_action_or_reset(test, reactor_teardown, NULL), 0);
+}
+
+static void test_double_register(struct kunit *test)
+{
+	register_test_reactor(test);
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&test_reactor), -EINVAL);
+}
+
+static const char long_reactor_name[] = "kunit_reactor_name_too_long_xxx_";
+static_assert(sizeof(long_reactor_name) - 1 >= MAX_RV_REACTOR_NAME_SIZE,
+	       "long_reactor_name must be at least MAX_RV_REACTOR_NAME_SIZE chars");
+
+static void test_name_too_long(struct kunit *test)
+{
+	static struct rv_reactor long_reactor = {
+		.name = long_reactor_name,
+	};
+
+	KUNIT_EXPECT_EQ(test, rv_register_reactor(&long_reactor), -EINVAL);
+}
+
+static struct kunit_case rv_reactor_registration_cases[] = {
+	KUNIT_CASE(test_double_register),
+	KUNIT_CASE(test_name_too_long),
+	{}
+};
+
+static struct kunit_suite rv_reactor_registration_suite = {
+	.name		= "rv_reactor_registration",
+	.test_cases	= rv_reactor_registration_cases,
+};
+
+static int react_call_count;
+
+__printf(1, 0) static void mock_react(const char *msg, va_list args)
+{
+	react_call_count++;
+	/* Busy-wait so a timer interrupt fires inside rv_react(). */
+	mdelay(20);
+}
+
+static void test_react_no_callback(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name = "kunit_null_react",
+	};
+
+	react_call_count = 0;
+	rv_react(&monitor, "no callback");
+
+	KUNIT_EXPECT_EQ(test, react_call_count, 0);
+}
+
+static void test_react_callback_invoked(struct kunit *test)
+{
+	struct rv_monitor monitor = {
+		.name	= "kunit_dispatch_monitor",
+		.react	= mock_react,
+	};
+
+	react_call_count = 0;
+	rv_react(&monitor, "callback invocation test");
+	KUNIT_EXPECT_EQ(test, react_call_count, 1);
+}
+
+static struct kunit_case rv_react_dispatch_cases[] = {
+	KUNIT_CASE(test_react_no_callback),
+	KUNIT_CASE(test_react_callback_invoked),
+	{}
+};
+
+static struct kunit_suite rv_react_dispatch_suite = {
+	.name		= "rv_react_dispatch",
+	.test_cases	= rv_react_dispatch_cases,
+};
+
+kunit_test_suites(&rv_reactor_registration_suite, &rv_react_dispatch_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for RV reactor registration and dispatch");
-- 
2.25.1

Re: [PATCH v6 4/4] rv/reactors: add KUnit tests for reactor registration and dispatch

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing module reference counting for RV reactors leads to a Use-After-Free if a reactor module is unloaded while still assigned to a monitor.
--

commit ad2a08e6decf463e8a9bfc030b93a95b3cf6f3b1
Author: Wen Yang <wen.yang@linux.dev>

rv/reactors: add KUnit tests for reactor registration and dispatch
    
Add KUnit tests covering the reactor register/unregister lifecycle
(including duplicate and name-length rejection) and rv_react() dispatch
(a no-op without a callback, exactly one invocation with one).

> diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
> new file mode 100644
> index 0000000000000..3850c6dda288f
> --- /dev/null
> +++ b/kernel/trace/rv/rv_reactors_kunit.c
[ ... ]
> +static void reactor_teardown(void *arg)
> +{
> +	rv_unregister_reactor(&test_reactor);

[Severity: High]
This is a pre-existing issue, but could rv_unregister_reactor() leave
dangling pointers if a dynamically loaded reactor module is unloaded while
still assigned to a monitor?

Looking at rv_unregister_reactor() in kernel/trace/rv/rv_reactors.c, it
removes the reactor from the global list but does not appear to iterate over
active monitors to detach it.

Furthermore, in monitor_swap_reactors_single(), the reactor is assigned to
the monitor without taking a module reference count via try_module_get():

kernel/trace/rv/rv_reactors.c:monitor_swap_reactors_single() {
    ...
    mon->reactor = reactor;
    mon->react = reactor->react;
    ...
}

If a user with tracefs access assigns a dynamically loaded reactor (e.g.,
reactor_printk.ko) to an RV monitor and then unloads the reactor module,
when the monitor's tracepoint fires, rv_react() would invoke the dangling
monitor->react function pointer.

Could this jump into the unmapped memory of the unloaded module and cause
a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789412799.git.wen.yang@linux.dev?part=4

Re: [PATCH v6 0/4] rv/reactors: fix lockdep warning and add tests

[Gabriele Monaco]

On Tue, 15 Sep 2026 03:13:33 +0800, wen.yang@linux.dev wrote:
> From: Wen Yang <wen.yang@linux.dev>
> 
> We occasionally hit a lockdep "Invalid wait context" warning when
> a reactor callback is preempted by a timer interrupt.
> On interrupt exit the scheduler takes rq->__lock (LD_WAIT_SPIN) while
> rv_react() still holds its wait-type-override map, which declared
> LD_WAIT_FREE.  On any kernel where the task context has preemption
> enabled (not just CONFIG_PREEMPT_RT) this triggers a spurious lockdep
> report:
> 
> [...]

Applied to my tree, thanks!

[1/4] rv/reactors: use LD_WAIT_SPIN as the reactor lockdep wait type
      commit: 16249b9b6be1f05e7d4116459a527ee681d2ff9f
[2/4] rv/reactors: propagate rv_register_reactor() error from reactor init
      commit: 7439018693dd41b6578a4a036cb463e1388b7ab0
[3/4] rv/reactors: export rv_register_reactor() and rv_unregister_reactor()
      commit: 1863477db0cdad202049ce66b1a0036a87fe16dc
[4/4] rv/reactors: add KUnit tests for reactor registration and dispatch
      commit: 8b8129a31089b2430a77debea97772bb2402779f

Best regards,
-- 
Gabriele Monaco <gmonaco@redhat.com>