rv/reactors: fix lockdep warning and add tests

PATCH Applied [email protected] 3 participants 8 messages Sep 14 – Sep 17

Subsystems (2)

Tracing Runtime verification (RV)

lore

Share & tools

lore

[email protected]

author

2026-09-14 19:13 UTC #0 lore raw

v6 cover rv/reactors: fix lockdep warning and add tests

From: Wen Yang <[email protected]>

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/[email protected]/
v4: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v1: https://lore.kernel.org/lkml/[email protected]/

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

Signature · 1 line

2.25.1

[email protected]

author

2026-09-14 19:13 UTC #1 lore raw

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

From: Wen Yang <[email protected]>

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 <[email protected]>

Signed-off-by Wen Yang <[email protected]>

Cc Thomas Weißschuh <[email protected]>

---
 kernel/trace/rv/rv_reactors.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

kernel/trace/rv/rv_reactors.c +9 -1

index 2f5fc8d18dea..ff7d478227c3 100644
@@ -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)

Signature · 1 line

2.25.1

[email protected]

author

2026-09-14 19:14 UTC #2 lore raw

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

In reply to [email protected]

From: Wen Yang <[email protected]>

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 <[email protected]>

Reviewed-by Nam Cao <[email protected]>

Signed-off-by Wen Yang <[email protected]>

---
 kernel/trace/rv/reactor_panic.c  | 3 +--
 kernel/trace/rv/reactor_printk.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

kernel/trace/rv/reactor_panic.c +1 -2

index 76537b8a4343..db7116ceafff 100644
@@ -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)

kernel/trace/rv/reactor_printk.c +1 -2

index 48c934e315b3..002a10f6aa7b 100644
@@ -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)

Signature · 1 line

2.25.1

[email protected]

author

2026-09-14 19:14 UTC #3 lore raw

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

In reply to [email protected]

From: Wen Yang <[email protected]>

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 <[email protected]>

Signed-off-by Wen Yang <[email protected]>

---
 kernel/trace/rv/rv_reactors.c | 2 ++
 1 file changed, 2 insertions(+)

kernel/trace/rv/rv_reactors.c +2 -0

index ff7d478227c3..8e57a95d446b 100644
@@ -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.

Signature · 1 line

2.25.1

[email protected]

AI review

2026-09-14 19:25 UTC #4 lore raw

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

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…

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 <[email protected]>

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.

kernel/trace/rv/rv_reactors.c int rv_register_reactor(struct rv_reactor *reactor) · line 317

Quoted 9 lines

kernel/trace/rv/rv_reactors.c +1 -0

index ff7d478227c32..8e57a95d446b5 100644
@@ -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.

kernel/trace/rv/rv_reactors.c int rv_unregister_reactor(struct rv_reactor *reactor) · line 331

Quoted 8 lines

kernel/trace/rv/rv_reactors.c +1 -0

 /**
  * 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);
    ...
}

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

[email protected]

author

2026-09-14 19:14 UTC #5 lore raw

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

In reply to [email protected]

From: Wen Yang <[email protected]>

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 <[email protected]>

Signed-off-by Wen Yang <[email protected]>

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

kernel/trace/rv/Kconfig +12 -0

index efa930f94ea4..9bfd429ffdea 100644
@@ -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

kernel/trace/rv/Makefile +1 -0

index cdbf68c84f5a..c895d81dfdad 100644
@@ -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

kernel/trace/rv/rv_reactors_kunit.c new file +111 -0

111 lines not on this page · raw message

Signature · 1 line

2.25.1

[email protected]

AI review

2026-09-14 19:28 UTC #6 lore raw

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

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 <[email protected]>

rv/reactors: add KUnit tests for reactor registration and dispatch

Quoted 5 lines

kernel/trace/rv/rv_reactors_kunit.c new file +0 -0
new file mode 100644
index 0000000000000..3850c6dda288f

[ ... ]

kernel/trace/rv/rv_reactors_kunit.c line 24

Quoted 3 lines

kernel/trace/rv/rv_reactors_kunit.c new file +3 -0
+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?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

Gabriele Monaco

maintainer Applied

2026-09-17 11:44 UTC #7 lore raw

In reply to [email protected]

Quoted 11 lines · On Tue, 15 Sep 2026 03:13:33 +0800, [email protected] wrote:

From: Wen Yang <[email protected]>
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,

Signature · 1 line

Gabriele Monaco <[email protected]>