Re: [PATCH 3/3] rv/reactors: add KUnit tests for reactor_panic
From: XIAO WU
Date: Sat Jun 20 2026 - 19:31:47 EST
Hi Wen,
I came across a Sashiko AI code review [1] that flagged a potential NULL
pointer dereference in the `test_panic_register_unregister()` test case
added by this patch (commit 8655782285e2). The review's analysis seemed
plausible, so I spun up a QEMU environment to see whether it could be
reproduced in practice.
The short version: yes, it triggers a real kernel BUG + Oops. See below
for the crash log and the reproduction approach.
On Tue, 16 Jun 2026 at 00:44, Wen Yang wrote:
> Add KUnit tests for the panic reactor covering:
> - Reactor registration and unregistration lifecycle
> - Panic notifier chain reachability
...
> +static void test_panic_register_unregister(struct kunit *test)
> +{
> + int ret;
> +
> + ret = rv_register_reactor(&mock_panic_reactor);
> + KUNIT_EXPECT_EQ(test, ret, 0);
> + KUNIT_EXPECT_STREQ(test, mock_panic_reactor.name, "test_panic");
> +
> + rv_unregister_reactor(&mock_panic_reactor);
This is the function the review highlighted. The issue is:
- `KUNIT_EXPECT_EQ()` does *not* abort the test on failure.
- If `rv_register_reactor()` fails (e.g. because another reactor
named "test_panic" was already registered), the .list node of the
statically-allocated `mock_panic_reactor` is never added to any
list — it remains zero-initialized (prev = NULL, next = NULL).
- `rv_unregister_reactor()` then unconditionally calls `list_del()`
on this uninitialized list_head, which hits the NULL pointers.
I was able to reproduce this reliably. The trigger condition is
surprisingly simple: if any code path registers a reactor named
"test_panic" before the KUnit suite runs, the test crashes the kernel.
[Reproduction approach]
I rebuilt the kernel with a small late_initcall in rv_reactors.c that
pre-registers "test_panic" (simulating what would happen if, say, a
kernel module or another subsystem registered a reactor with the same
name before the KUnit tests execute):
static int __init prereg_test_panic(void)
{
static struct rv_reactor prereg = {
.name = "test_panic",
.description = "pre-registered to simulate name collision",
};
return rv_register_reactor(&prereg);
}
late_initcall(prereg_test_panic);
The KUnit tests then auto-run at boot (kunit_run_all_tests). The
test_panic_register_unregister case fails registration with -EINVAL due
to the duplicate name, the KUNIT_EXPECT_EQ does not abort, and
rv_unregister_reactor() crashes on the uninitialized list.
[Crash log — kernel 7.1.0-next-20260615, CONFIG_DEBUG_LIST=y]
Reactor test_panic is already registered
# test_panic_register_unregister: EXPECTATION FAILED at kernel/trace/rv/reactor_panic_kunit.c:68
Expected ret == 0, but
ret == -22 (0xffffffffffffffea)
list_del corruption, ffffffff8ecce2f8->next is NULL
------------[ cut here ]------------
kernel BUG at lib/list_debug.c:52!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
CPU: 1 UID: 0 PID: 5028 Comm: kunit_try_catch Tainted: G N
RIP: 0010:__list_del_entry_valid_or_report+0xf2/0x200
Call Trace:
<TASK>
rv_unregister_reactor+0x37/0x190
test_panic_register_unregister+0x1de/0x2e0
kunit_try_run_case+0x1d2/0x520
kunit_generic_run_threadfn_adapter+0x89/0x100
kthread+0x387/0x4a0
ret_from_fork+0xb2c/0xdd0
</TASK>
Kernel panic - not syncing: Fatal exception
The crash is in `rv_unregister_reactor()`, called from
`test_panic_register_unregister()`. The `list_del()` in
`rv_unregister_reactor()` has no guard against a list node that was
never added to any list. With CONFIG_DEBUG_LIST=y the corruption is
caught explicitly; without it this would be a silent NULL dereference.
[Suggested fix]
The most straightforward fix is to use `KUNIT_ASSERT_EQ()` instead of
`KUNIT_EXPECT_EQ()` for the registration result, so the test aborts
before reaching `rv_unregister_reactor()` on a failed registration:
static void test_panic_register_unregister(struct kunit *test)
{
int ret;
ret = rv_register_reactor(&mock_panic_reactor);
- KUNIT_EXPECT_EQ(test, ret, 0);
+ KUNIT_ASSERT_EQ(test, ret, 0);
KUNIT_EXPECT_STREQ(test, mock_panic_reactor.name, "test_panic");
rv_unregister_reactor(&mock_panic_reactor);
}
An alternative (or complementary) approach would be to add a guard in
`rv_unregister_reactor()` itself — e.g. checking whether the reactor is
actually on the list before calling `list_del()`. That would make the
API more robust against future callers making the same mistake.
The same pattern likely applies to the printk reactor tests in patch
2/3, though I haven't tested those.
Full PoC code follows.
[PoC part 1 — Kernel-space: late_initcall to create the name collision]
This is what was added to kernel/trace/rv/rv_reactors.c (or could be
built as a standalone kernel module — see preregister.c below). It
pre-registers "test_panic" before KUnit auto-runs, so the test's own
rv_register_reactor() fails with -EINVAL:
static int __init prereg_test_panic(void)
{
static struct rv_reactor prereg = {
.name = "test_panic",
.description = "pre-registered to simulate name collision",
};
return rv_register_reactor(&prereg);
}
late_initcall(prereg_test_panic);
[PoC part 2 — Userspace: trigger the KUnit test via debugfs]
poc.c:
---8<----------------------------------------------------------------
/*
* POC: NULL pointer dereference in rv_unregister_reactor()
*
* Bug location: kernel/trace/rv/reactor_panic_kunit.c
* test_panic_register_unregister()
*
* Bug: When rv_register_reactor() fails (because "test_panic" is already
* registered), the test calls rv_unregister_reactor() unconditionally.
* This performs list_del() on a zero-initialized list_head (never added
* to any list), causing a NULL pointer dereference crash.
*
* Trigger: With a kernel that has pre-registered "test_panic" reactor,
* simply trigger the KUnit test via debugfs "run" file. The test's
* rv_register_reactor() fails with -EINVAL (duplicate), and the subsequent
* rv_unregister_reactor() crashes on the uninitialized list.
*/
#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#define KUNIT_RUN_PATH "/sys/kernel/debug/kunit/rv_reactor_panic/run"
int main(int argc, char **argv)
{
int fd, ret;
setbuf(stdout, NULL);
printf("[+] POC: Triggering NULL deref in rv_unregister_reactor\n");
printf("[+] Target: %s\n\n", KUNIT_RUN_PATH);
/* Mount debugfs if needed */
if (access("/sys/kernel/debug", F_OK) != 0) {
printf("[*] Mounting debugfs...\n");
ret = system("mount -t debugfs none /sys/kernel/debug/ 2>/dev/null");
(void)ret;
}
/* Verify KUnit path exists */
if (access(KUNIT_RUN_PATH, W_OK) != 0) {
printf("[-] Cannot access %s: %m\n", KUNIT_RUN_PATH);
printf("[*] Available KUnit suites:\n");
fflush(stdout);
system("ls -la /sys/kernel/debug/kunit/ 2>&1");
return 1;
}
printf("[*] Test_panic reactor should be pre-registered at boot\n");
printf("[*] Triggering KUnit test suite...\n\n");
/*
* Write to the KUnit run file. This executes
* __kunit_test_suites_init() -> kunit_run_tests() which
* runs the reactor_panic_kunit test cases including
* test_panic_register_unregister.
*
* With "test_panic" pre-registered:
* 1. rv_register_reactor() returns -EINVAL (duplicate)
* 2. KUNIT_EXPECT_EQ doesn't abort
* 3. rv_unregister_reactor() calls list_del() on NULL list
* 4. BOOM: list corruption / NULL deref / kernel crash
*/
fd = open(KUNIT_RUN_PATH, O_WRONLY);
if (fd < 0) {
printf("[-] open failed: %m\n");
return 1;
}
printf("[!] Writing to %s - triggering the crash now...\n", KUNIT_RUN_PATH);
fflush(stdout);
ret = write(fd, "1", 1);
if (ret < 0) {
printf("[-] write failed: %m\n");
} else {
printf("[+] Write succeeded (ret=%d)\n", ret);
}
close(fd);
/*
* If we reach here without crashing, let the user know
*/
printf("\n[*] If the system is still alive, check dmesg:\n");
printf(" dmesg | grep -i -E 'list_del|list_add|list_corrupt|NULL|BUG|oops\n");
printf("\n[*] dmesg output:\n");
fflush(stdout);
system("dmesg | tail -60");
printf("\n[+] POC completed.\n");
return 0;
}
---8<----------------------------------------------------------------
Compile with:
gcc -o poc poc.c -static
[PoC part 3 — Kernel module alternative (standalone)]
If you prefer not to modify rv_reactors.c directly, the same name
collision can be created by loading this module before running the
KUnit test. Note: this requires rv_register_reactor() to be exported
(or resolved via kallsyms), which it may not be in the current tree.
In that case the late_initcall approach above is the way to go.
preregister.c:
---8<----------------------------------------------------------------
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/rv.h>
static struct rv_reactor prereg_reactor = {
.name = "test_panic",
.description = "pre-registered to trigger KUnit bug",
};
static int __init prereg_init(void)
{
int ret;
ret = rv_register_reactor(&prereg_reactor);
if (ret < 0) {
pr_err("preregister: rv_register_reactor failed: %d\n", ret);
return ret;
}
pr_info("preregister: registered 'test_panic' reactor\n");
return 0;
}
static void __exit prereg_exit(void)
{
rv_unregister_reactor(&prereg_reactor);
pr_info("preregister: unregistered 'test_panic' reactor\n");
}
module_init(prereg_init);
module_exit(prereg_exit);
MODULE_LICENSE("GPL");
---8<----------------------------------------------------------------
[1] https://sashiko.dev/#/patchset/cover.1781541556.git.wen.yang%40linux.dev
(Sashiko AI code review — "Null Pointer Dereference", Severity: High)
Thanks,
XIAO