Hi Miroslav,
On Wed, Oct 14, 2026 at 02:23:50PM +0200, Miroslav Benes wrote:
> Interesting. So to sum it up... you load a live patch which contains a fix
> for a module and while the transition is on, you remove the module and it
> crashes
Yes. Loading such a module during the transition races too:
klp_patch_func() adds to klp_ops while the scheduler walks it.
> (btw, I do not think that those traces are necessary here. More
> btw, it would be nice to have a human readable changelog instead of what
> looks like AI generated description of the patch itself).
Fair enough, I'll rewrite it and drop the traces.
> How did you find the issue? Is it a real problem in production?
An LLM found it while auditing kernel/livepatch/. It is not a
production issue. The reproducer was written by an LLM as well; it is
below, apply it with "git apply -p0". It keeps a transition stuck on
a few kthreads and reloads a patched module in a loop. On v7.3-rc7 it
faults in klp_find_ops() within a few minutes with KCSAN_STRICT=y.
> I wonder whether we could just decline delete_module() in this case and
> return -EBUSY. It should be a rare operation anyway. Alternatively we
> could increase refcounts of all patched modules during a transition and
> decrease them after.
I don't think either covers all the paths. klp_module_going() is also
called when a module's init fails, or when its load is aborted after
klp_module_coming(), and neither -EBUSY nor a module reference prevents
that. Loading a patched module also adds to klp_ops under the walk.
And klp_find_ops() walks the global klp_ops list, so it's the modules of
all patches that matter, not just the transition patch.
> I am not saying we should not do the RCU protection but it feels like a
> can of worms a bit so if there is an easier way...
I could split it: one patch RCU-protecting klp_ops, which fixes the
crash, and a second one for the ordering on module reload, which was found
by inspection and has not been reproduced. Would that be easier to review?
Thanks,
Andrea
diff --git klp-race-repro/README klp-race-repro/README
new file mode 100644
--- /dev/null
+++ klp-race-repro/README
@@ -0,0 +1,108 @@
+Livepatch klp_ops race reproducer
+=================================
+
+This directory contains a reproducer for a use-after-free in
+klp_find_ops(): the function walks the global klp_ops list from the
+scheduler (__klp_sched_try_switch() -> klp_try_switch_task() ->
+klp_check_stack() -> klp_check_stack_func()) without klp_mutex, while
+klp_patch_func()/klp_unpatch_func() add, remove and free klp_ops entries
+under klp_mutex when a to-be-patched module is loaded or unloaded.
+
+The fix ("livepatch: Fix races between stack check and module load/unload")
+RCU-protects the klp_ops list.
+
+Files
+-----
+ klp_race_target_a.c target module A: four kthreads looping in
+ ra_target_a_loop(), which sleeps with
+ TASK_FREEZABLE. The livepatch transition can not
+ switch them, so it stays in progress and the
+ scheduler keeps calling klp_find_ops().
+ klp_race_target_b.c target module B: 32 patchable no-op functions.
+ klp_race_patch.c livepatch patching ra_target_a_loop() (A) and all
+ ra_target_b_func*() (B).
+ run.sh sets up the stuck transition and then insmod/rmmods
+ module B in a loop.
+ Makefile builds the three modules against a kernel tree.
+
+Requirements
+------------
+ CONFIG_LIVEPATCH=y, CONFIG_MODULES=y, CONFIG_MODULE_UNLOAD=y,
+ CONFIG_FUNCTION_TRACER=y, CONFIG_DYNAMIC_FTRACE_WITH_REGS=y,
+ CONFIG_KALLSYMS_ALL=y, CONFIG_KCSAN=y and CONFIG_KCSAN_STRICT=y.
+
+ CONFIG_KCSAN_STRICT is important: the default KCSAN configuration
+ (CONFIG_KCSAN_REPORT_VALUE_CHANGE_ONLY=y) does not report this race.
+
+ Developed on a WSL2 host; vng needs "--force-9p" there (virtiofsd is
+ not available).
+
+Building
+--------
+ KDIR=/path/to/kernel/tree
+
+ cd $KDIR
+ # enable the options above, then:
+ make -j4
+
+ cd $KDIR/repro
+ make -j4 KERNELDIR=$KDIR
+
+Settings
+--------
+ run.sh reads these environment variables:
+
+ ITER maximum number of unload/reload cycles of module B
+ (default 20000)
+ DURATION maximum length of the loop in seconds (default 1800);
+ the loop stops at whichever of ITER and DURATION is
+ reached first, and run.sh prints both numbers
+ OUT if set, run.sh saves the full dmesg to $OUT/dmesg.txt
+ at the end; the grep it prints can miss lockdep and RCU
+ reports. Make the directory writable in the guest,
+ e.g. with vng --rwdir=$OUT
+
+Run unpatched (expect a crash)
+------------------------------
+ Revert/invert the fix first and rebuild the kernel, then:
+
+ vng -r $KDIR --cpus 4 --memory 4G --force-9p --verbose \
+ --exec "ITER=100000 sh $KDIR/repro/run.sh $KDIR/repro"
+
+ Within a few minutes the guest reports a data race between
+ klp_find_ops() and klp_unpatch_func(), and then a general protection
+ fault in klp_find_ops():
+
+ BUG: KCSAN: data-race in klp_find_ops / klp_unpatch_func
+ ...
+ Oops: general protection fault, probably for non-canonical address
+ 0xdead000000000110
+ RIP: 0010:klp_find_ops+0x55/0x90
+ ...
+ __klp_sched_try_switch
+ __schedule
+
+ 0xdead000000000110 is LIST_POISON1 (0xdead000000000100) plus the offset
+ of func_stack in struct klp_ops: the reader walked a list node that
+ klp_unpatch_func() had just list_del()'d.
+
+Run patched (expect no klp report)
+----------------------------------
+ With the fix applied and the kernel rebuilt, run the same command.
+ Unrelated KCSAN races (9p, kernfs, ...) may still be reported, but
+ there must be no klp_* report and no fault. The transition also
+ completes normally once the kthreads are stopped:
+
+ livepatch: 'klp_race_patch': patching complete
+
+Notes
+-----
+ - The transition is kept incomplete on purpose: the kthreads sleep
+ inside a function the livepatch wants to patch, so the consistency
+ model refuses to switch them and klp_resched_enable() keeps the
+ scheduler hook active.
+ - Module B is unloaded/reloaded to race klp_unpatch_func() and
+ klp_patch_func() against the scheduler walk; the 32 functions widen
+ the walk.
+ - Reproduces probabilistically; run it for a few minutes.
+ KCSAN_STRICT usually reports the race well before the GPF.
diff --git klp-race-repro/Makefile klp-race-repro/Makefile
new file mode 100644
--- /dev/null
+++ klp-race-repro/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+KERNELDIR ?= /lib/modules/$(shell uname -r)/build
+PWD := $(shell pwd)
+
+obj-m += klp_race_target_a.o
+obj-m += klp_race_target_b.o
+obj-m += klp_race_patch.o
+
+all:
+ $(MAKE) -C $(KERNELDIR) M=$(PWD) modules
+
+clean:
+ $(MAKE) -C $(KERNELDIR) M=$(PWD) clean
diff --git klp-race-repro/run.sh klp-race-repro/run.sh
new file mode 100644
--- /dev/null
+++ klp-race-repro/run.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Reproducer for the livepatch klp_ops race.
+#
+# klp_check_stack_func() runs from the scheduler (__klp_sched_try_switch() ->
+# klp_try_switch_task() -> klp_check_stack() -> klp_check_stack_func()) without
+# klp_mutex and calls klp_find_ops(), which walks the global klp_ops list.
+# klp_patch_func()/klp_unpatch_func() add/remove/free those entries under
+# klp_mutex when a to-be-patched module is loaded/unloaded.
+#
+# The target A kthread sleeps freezably inside a patched function, so the
+# transition never completes and the scheduler keeps walking klp_ops. The
+# target B module is then loaded and unloaded in a loop to free/reallocate
+# klp_ops entries concurrently.
+
+set -u
+
+DIR=${1:-$(pwd)}
+ITER=${ITER:-20000}
+DURATION=${DURATION:-1800}
+OUT=${OUT:-}
+
+echo "== livepatch klp_ops race reproducer =="
+
+insmod "$DIR/klp_race_target_a.ko" || exit 1
+insmod "$DIR/klp_race_target_b.ko" || exit 1
+insmod "$DIR/klp_race_patch.ko" || exit 1
+
+# Let the transition get stuck on the target A kthread.
+sleep 1
+echo -n "transition: "
+cat /sys/kernel/livepatch/klp_race_patch/transition
+
+i=0
+start=$(date +%s)
+end=$((start + DURATION))
+while [ "$i" -lt "$ITER" ] && [ "$(date +%s)" -lt "$end" ]; do
+ rmmod klp_race_target_b || break
+ insmod "$DIR/klp_race_target_b.ko" || break
+ i=$((i + 1))
+done
+
+echo "iterations: $i in $(( $(date +%s) - start ))s"
+
+# Unblock and tear down.
+rmmod klp_race_target_a 2>/dev/null
+sleep 1
+echo 0 > /sys/kernel/livepatch/klp_race_patch/enabled 2>/dev/null
+rmmod klp_race_patch 2>/dev/null
+rmmod klp_race_target_b 2>/dev/null
+
+echo "== result =="
+dmesg | grep -iE
"KCSAN|BUG:|use-after-free|list_(add|del)|klp_(find_ops|unpatch_func|patch_func)|ra_target"
| tail -n 120
+
+[ -n "$OUT" ] && dmesg > "$OUT/dmesg.txt"
diff --git klp-race-repro/klp_race_patch.c klp-race-repro/klp_race_patch.c
new file mode 100644
--- /dev/null
+++ klp-race-repro/klp_race_patch.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Livepatch for the klp_ops race reproducer.
+ *
+ * It patches ra_target_a_loop() (target A) and the ra_target_b_func*()
+ * family (target B). Loading it starts a transition which can not complete
+ * because the target A kthread sleeps inside ra_target_a_loop().
+ */
+
+#include <linux/module.h>
+#include <linux/livepatch.h>
+
+static void ra_patch_a(void)
+{
+}
+
+static void ra_patch_b(void)
+{
+}
+
+static struct klp_func funcs_a[] = {
+ { .old_name = "ra_target_a_loop", .new_func = ra_patch_a, },
+ { }
+};
+
+#define RA_PATCH_B_ENTRY(n) \
+ { .old_name = "ra_target_b_func" #n, .new_func = ra_patch_b, },
+
+static struct klp_func funcs_b[] = {
+ RA_PATCH_B_ENTRY(0)
+ RA_PATCH_B_ENTRY(1)
+ RA_PATCH_B_ENTRY(2)
+ RA_PATCH_B_ENTRY(3)
+ RA_PATCH_B_ENTRY(4)
+ RA_PATCH_B_ENTRY(5)
+ RA_PATCH_B_ENTRY(6)
+ RA_PATCH_B_ENTRY(7)
+ RA_PATCH_B_ENTRY(8)
+ RA_PATCH_B_ENTRY(9)
+ RA_PATCH_B_ENTRY(10)
+ RA_PATCH_B_ENTRY(11)
+ RA_PATCH_B_ENTRY(12)
+ RA_PATCH_B_ENTRY(13)
+ RA_PATCH_B_ENTRY(14)
+ RA_PATCH_B_ENTRY(15)
+ RA_PATCH_B_ENTRY(16)
+ RA_PATCH_B_ENTRY(17)
+ RA_PATCH_B_ENTRY(18)
+ RA_PATCH_B_ENTRY(19)
+ RA_PATCH_B_ENTRY(20)
+ RA_PATCH_B_ENTRY(21)
+ RA_PATCH_B_ENTRY(22)
+ RA_PATCH_B_ENTRY(23)
+ RA_PATCH_B_ENTRY(24)
+ RA_PATCH_B_ENTRY(25)
+ RA_PATCH_B_ENTRY(26)
+ RA_PATCH_B_ENTRY(27)
+ RA_PATCH_B_ENTRY(28)
+ RA_PATCH_B_ENTRY(29)
+ RA_PATCH_B_ENTRY(30)
+ RA_PATCH_B_ENTRY(31)
+ { }
+};
+
+static struct klp_object objs[] = {
+ { .name = "klp_race_target_a", .funcs = funcs_a, },
+ { .name = "klp_race_target_b", .funcs = funcs_b, },
+ { }
+};
+
+static struct klp_patch patch = {
+ .mod = THIS_MODULE,
+ .objs = objs,
+};
+
+static int __init ra_patch_init(void)
+{
+ return klp_enable_patch(&patch);
+}
+
+static void __exit ra_patch_exit(void)
+{
+}
+
+module_init(ra_patch_init);
+module_exit(ra_patch_exit);
+MODULE_LICENSE("GPL");
+MODULE_INFO(livepatch, "Y");
+MODULE_DESCRIPTION("livepatch klp_ops race reproducer: patch");
diff --git klp-race-repro/klp_race_target_a.c klp-race-repro/klp_race_target_a.c
new file mode 100644
--- /dev/null
+++ klp-race-repro/klp_race_target_a.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Target module A for the livepatch klp_ops race reproducer.
+ *
+ * A kthread repeatedly calls ra_target_a_loop(), which goes to sleep with
+ * TASK_FREEZABLE. While the livepatch transition is in progress the task
+ * can not be switched (it sleeps inside the to-be-patched function), so the
+ * transition never completes and __klp_sched_try_switch() keeps walking the
+ * global klp_ops list from the scheduler.
+ */
+
+#include <linux/module.h>
+#include <linux/kthread.h>
+#include <linux/sched.h>
+#include <linux/delay.h>
+
+void ra_target_a_loop(void);
+
+noinline void ra_target_a_loop(void)
+{
+ set_current_state(TASK_INTERRUPTIBLE | TASK_FREEZABLE);
+ if (!kthread_should_stop())
+ schedule_timeout(msecs_to_jiffies(1));
+ __set_current_state(TASK_RUNNING);
+}
+
+#define RA_NR_THREADS 4
+
+static struct task_struct *ra_threads[RA_NR_THREADS];
+
+static int ra_thread_fn(void *unused)
+{
+ while (!kthread_should_stop())
+ ra_target_a_loop();
+
+ return 0;
+}
+
+static int __init ra_a_init(void)
+{
+ int i;
+
+ for (i = 0; i < RA_NR_THREADS; i++) {
+ int err;
+
+ ra_threads[i] = kthread_run(ra_thread_fn, NULL,
"ra_target_a/%d", i);
+ if (IS_ERR(ra_threads[i])) {
+ err = PTR_ERR(ra_threads[i]);
+ while (--i >= 0)
+ kthread_stop(ra_threads[i]);
+ return err;
+ }
+ }
+
+ return 0;
+}
+
+static void __exit ra_a_exit(void)
+{
+ int i;
+
+ for (i = 0; i < RA_NR_THREADS; i++)
+ kthread_stop(ra_threads[i]);
+}
+
+module_init(ra_a_init);
+module_exit(ra_a_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("livepatch klp_ops race reproducer: target A");
diff --git klp-race-repro/klp_race_target_b.c klp-race-repro/klp_race_target_b.c
new file mode 100644
--- /dev/null
+++ klp-race-repro/klp_race_target_b.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Target module B for the livepatch klp_ops race reproducer.
+ *
+ * It provides a set of functions which are patched by klp_race_patch. The
+ * module is unloaded and loaded in a loop while the livepatch transition is
+ * stuck on target A, so that klp_unpatch_func()/klp_patch_func() free and
+ * allocate klp_ops entries while the scheduler walks the list.
+ */
+
+#include <linux/module.h>
+#include <linux/compiler.h>
+
+#define RA_TARGET_B_FUNCS(X) \
+ X(0) X(1) X(2) X(3) X(4) X(5) X(6) X(7) \
+ X(8) X(9) X(10) X(11) X(12) X(13) X(14) X(15) \
+ X(16) X(17) X(18) X(19) X(20) X(21) X(22) X(23) \
+ X(24) X(25) X(26) X(27) X(28) X(29) X(30) X(31)
+
+#define RA_PROTO_B(n) void ra_target_b_func##n(void);
+RA_TARGET_B_FUNCS(RA_PROTO_B)
+
+#define RA_DEF_B(n) noinline void ra_target_b_func##n(void) { barrier(); }
+RA_TARGET_B_FUNCS(RA_DEF_B)
+
+static int __init ra_b_init(void)
+{
+ return 0;
+}
+
+static void __exit ra_b_exit(void)
+{
+}
+
+module_init(ra_b_init);
+module_exit(ra_b_exit);
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("livepatch klp_ops race reproducer: target B");
