[PATCH] livepatch: Fix races between stack check and module load/unload

[PATCH] livepatch: Fix races between stack check and module load/unload

[Andrea Parri]

Unloading a patched module while a livepatch transition is in progress
can crash the kernel in klp_find_ops(), called from the scheduler.
KCSAN reports

  BUG: KCSAN: data-race in klp_find_ops / klp_unpatch_func

  write (marked) to 0xffff9094c09ece10 of 8 bytes by task 381 on cpu 2:
   klp_unpatch_func+0x1bd/0x280
   klp_unpatch_object+0x6a/0x90
   klp_cleanup_module_patches_limited.cold+0x4a/0xd4
   klp_module_going+0x54/0x80
   __do_sys_delete_module+0x247/0x380
   __x64_sys_delete_module+0x39/0x50
   ...

  read to 0xffff9094c09ece10 of 8 bytes by task 146 on cpu 1:
   klp_find_ops+0x55/0x90
   klp_check_and_switch_task+0x228/0x320
   klp_try_switch_task+0xb4/0xc0
   __klp_sched_try_switch+0x9a/0xb0
   __schedule+0x60a/0x1450
   schedule+0x3b/0x100
   schedule_timeout+0xc0/0x160
   ra_target_a_loop+0x97/0xa0 [klp_race_target_a]
   ...

followed by

  Oops: general protection fault, probably for non-canonical address
  0xdead000000000110: 0000 [#2] SMP NOPTI
  RIP: 0010:klp_find_ops+0x55/0x90
   ...
   klp_check_and_switch_task+0x228/0x320
   klp_try_switch_task+0xb4/0xc0
   __klp_sched_try_switch+0x9a/0xb0
   __schedule+0x60a/0x1450

0xdead000000000110 is LIST_POISON1 (0xdead000000000100) plus the offset
of func_stack in struct klp_ops.

klp_check_stack_func() walks the global klp_ops list in klp_find_ops().
It is reached from the scheduler through __klp_sched_try_switch() ->
klp_try_switch_task() -> klp_check_stack() without klp_mutex, while
klp_patch_func() and klp_unpatch_func() add, remove and free klp_ops
entries under klp_mutex when a to-be-patched module is loaded or
unloaded:

  CPU 1                                 CPU 2
  __schedule()                          delete_module()
    klp_check_stack_func()                klp_module_going()
      klp_find_ops()                        klp_unpatch_func()
        pos = ops
                                              list_del(&ops->node)
                                              kfree(ops)
        pos = ops->node.next  /* LIST_POISON1 */
        pos->func_stack.next  /* oops */

Fix it by RCU protecting the klp_ops list.  klp_find_ops() uses
list_for_each_entry_rcu() and list_first_or_null_rcu(), klp_patch_func()
publishes a new ops with list_add_rcu() only after its func_stack has
been initialized, and klp_unpatch_func() and the klp_patch_func() error
path free it with kfree_rcu() after list_del_rcu().

klp_check_stack() takes the RCU read lock around the walk to make the
dependency explicit.  It already runs with preemption disabled and with
RCU watching, so the grace period used by kfree_rcu() covers the reader.

If klp_find_ops() no longer finds the ops, the function is not
redirected anymore and klp_check_stack_func() has nothing to check.

Unload also unpatches the transition patch, so a found func_stack can
shrink, even unlinking func itself, and list_is_singular() and
list_next_entry() then pick a bogus klp_func.  Use
list_next_or_null_rcu() and treat NULL as the original function.

klp_free_object_loaded() then clears func->old_func, and new_func of NOP
functions, without waiting for readers.  Mark these accesses with
READ_ONCE() and WRITE_ONCE().  A NULL new_func matches no stack entry,
and a NULL old_func is skipped, as the lookup could otherwise match
another removed entry.

A reload during the transition needs ordering too.
func->stack_node.next is read through func, not through the ops found
under RCU, so a task that sees obj->patched set can read a stale value:
func's own node from INIT_LIST_HEAD() on the first load, or what
list_del_rcu() left at the last unload, such as a freed ops head.

The first makes klp_check_stack_func() check func->new_func instead of
the original function, so a task sleeping in the original function
could be switched.  The second can read new_func and new_size from
freed memory.  Publish obj->patched with smp_store_release() in
klp_patch_object() and read it with smp_load_acquire() in
klp_check_stack().

Clearing obj->patched needs no ordering, since a task that still sees
it set is handled by the RCU lookup, and the synchronize_rcu() in the
unload path waits for the walk before the module can be loaded again.
patched_show() reads obj->patched for sysfs output only, so mark that
read with data_race().

The load-side ordering was found by code inspection and has not been
reproduced: the reproducer runs on x86, where neither the CPU nor, given
the barriers in between, the compiler reorders these loads.

Fixes: e3ff7c609f39 ("livepatch,sched: Add livepatch task switching to cond_resched()")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
---
Tested on x86_64 in a 4-CPU vng guest with CONFIG_KCSAN_STRICT=y, with
and without CONFIG_PROVE_LOCKING=y and CONFIG_PROVE_RCU_LIST=y.  A
livepatch transition is kept in progress while a patched module is
unloaded and reloaded for 30 minutes (about 370 reloads).  No KCSAN
report involves livepatch code, and lockdep and RCU stay silent.  On a
kernel without this patch, the same reproducer faults in klp_find_ops()
as shown above.

 kernel/livepatch/core.c       |  8 +++++---
 kernel/livepatch/patch.c      | 33 ++++++++++++++++++++------------
 kernel/livepatch/patch.h      |  2 ++
 kernel/livepatch/transition.c | 36 +++++++++++++++++++++++++++--------
 4 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index a240d1144e89c..ccae5cc5169b6 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -499,7 +499,8 @@ static ssize_t patched_show(struct kobject *kobj,
 	struct klp_object *obj;
 
 	obj = container_of(kobj, struct klp_object, kobj);
-	return sysfs_emit(buf, "%d\n", obj->patched);
+	/* Racy without klp_mutex, but only reported to user space. */
+	return sysfs_emit(buf, "%d\n", data_race(obj->patched));
 }
 
 static struct kobj_attribute patched_kobj_attr = __ATTR_RO(patched);
@@ -696,10 +697,11 @@ static void klp_free_object_loaded(struct klp_object *obj)
 	obj->mod = NULL;
 
 	klp_for_each_func(obj, func) {
-		func->old_func = NULL;
+		/* Read without klp_mutex, see klp_check_stack_func(). */
+		WRITE_ONCE(func->old_func, NULL);
 
 		if (func->nop)
-			func->new_func = NULL;
+			WRITE_ONCE(func->new_func, NULL);
 	}
 }
 
diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
index 3f54a017bbf69..d01ec7a6256ab 100644
--- a/kernel/livepatch/patch.c
+++ b/kernel/livepatch/patch.c
@@ -22,15 +22,19 @@
 
 static LIST_HEAD(klp_ops);
 
+/*
+ * Caller must hold klp_mutex or rcu_read_lock(): klp_check_stack_func() walks
+ * klp_ops from the scheduler without klp_mutex.
+ */
 struct klp_ops *klp_find_ops(void *old_func)
 {
 	struct klp_ops *ops;
 	struct klp_func *func;
 
-	list_for_each_entry(ops, &klp_ops, node) {
-		func = list_first_entry(&ops->func_stack, struct klp_func,
-					stack_node);
-		if (func->old_func == old_func)
+	list_for_each_entry_rcu(ops, &klp_ops, node, lockdep_is_held(&klp_mutex)) {
+		func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
+					      stack_node);
+		if (func && READ_ONCE(func->old_func) == old_func)
 			return ops;
 	}
 
@@ -148,8 +152,8 @@ static void klp_unpatch_func(struct klp_func *func)
 		WARN_ON(ftrace_set_filter_ip(&ops->fops, ftrace_loc, 1, 0));
 
 		list_del_rcu(&func->stack_node);
-		list_del(&ops->node);
-		kfree(ops);
+		list_del_rcu(&ops->node);
+		kfree_rcu(ops, rcu_head);
 	} else {
 		list_del_rcu(&func->stack_node);
 	}
@@ -191,10 +195,9 @@ static int klp_patch_func(struct klp_func *func)
 				  FTRACE_OPS_FL_IPMODIFY |
 				  FTRACE_OPS_FL_PERMANENT;
 
-		list_add(&ops->node, &klp_ops);
-
 		INIT_LIST_HEAD(&ops->func_stack);
 		list_add_rcu(&func->stack_node, &ops->func_stack);
+		list_add_rcu(&ops->node, &klp_ops);
 
 		ret = ftrace_set_filter_ip(&ops->fops, ftrace_loc, 0, 0);
 		if (ret) {
@@ -222,8 +225,8 @@ static int klp_patch_func(struct klp_func *func)
 
 err:
 	list_del_rcu(&func->stack_node);
-	list_del(&ops->node);
-	kfree(ops);
+	list_del_rcu(&ops->node);
+	kfree_rcu(ops, rcu_head);
 	return ret;
 }
 
@@ -240,7 +243,7 @@ static void __klp_unpatch_object(struct klp_object *obj, bool nops_only)
 	}
 
 	if (obj->dynamic || !nops_only)
-		obj->patched = false;
+		WRITE_ONCE(obj->patched, false);
 }
 
 
@@ -264,7 +267,13 @@ int klp_patch_object(struct klp_object *obj)
 			return ret;
 		}
 	}
-	obj->patched = true;
+
+	/*
+	 * Pairs with smp_load_acquire() in klp_check_stack(): a task that sees
+	 * the object patched also sees the func_stack linkage and the
+	 * old_func/new_func addresses and sizes of all its funcs.
+	 */
+	smp_store_release(&obj->patched, true);
 
 	return 0;
 }
diff --git a/kernel/livepatch/patch.h b/kernel/livepatch/patch.h
index d5f2fbe373e05..20e8704daca53 100644
--- a/kernel/livepatch/patch.h
+++ b/kernel/livepatch/patch.h
@@ -17,11 +17,13 @@
  *
  * @node:	node for the global klp_ops list
  * @func_stack:	list head for the stack of klp_func's (active func is on top)
+ * @rcu_head:	used to free this structure with kfree_rcu()
  * @fops:	registered ftrace ops struct
  */
 struct klp_ops {
 	struct list_head node;
 	struct list_head func_stack;
+	struct rcu_head rcu_head;
 	struct ftrace_ops fops;
 };
 
diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
index 2351a19ac2a9e..e9aa16baf1743 100644
--- a/kernel/livepatch/transition.c
+++ b/kernel/livepatch/transition.c
@@ -8,6 +8,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/cpu.h>
+#include <linux/rculist.h>
 #include <linux/stacktrace.h>
 #include <linux/static_call.h>
 #include "core.h"
@@ -201,12 +202,16 @@ void klp_update_patch_state(struct task_struct *task)
 /*
  * Determine whether the given stack trace includes any references to a
  * to-be-patched or to-be-unpatched function.
+ *
+ * Must be called under rcu_read_lock(), see klp_check_stack().
  */
 static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
 				unsigned int nr_entries)
 {
 	unsigned long func_addr, func_size, address;
+	struct klp_func *prev;
 	struct klp_ops *ops;
+	void *old_func;
 	int i;
 
 	if (klp_target_state == KLP_TRANSITION_UNPATCHED) {
@@ -214,24 +219,35 @@ static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
 		  * Check for the to-be-unpatched function
 		  * (the func itself).
 		  */
-		func_addr = (unsigned long)func->new_func;
+		func_addr = (unsigned long)READ_ONCE(func->new_func);
 		func_size = func->new_size;
 	} else {
 		/*
 		 * Check for the to-be-patched function
 		 * (the previous func).
 		 */
-		ops = klp_find_ops(func->old_func);
+		old_func = READ_ONCE(func->old_func);
+		if (!old_func)
+			return 0;
+
+		ops = klp_find_ops(old_func);
 
-		if (list_is_singular(&ops->func_stack)) {
+		/*
+		 * The ops might have just been removed by a concurrent module
+		 * unload.  The function is not redirected anymore in that case.
+		 */
+		if (!ops)
+			return 0;
+
+		/* func_stack may shrink under us, even unlinking func itself. */
+		prev = list_next_or_null_rcu(&ops->func_stack, &func->stack_node,
+					     struct klp_func, stack_node);
+		if (!prev) {
 			/* original function */
-			func_addr = (unsigned long)func->old_func;
+			func_addr = (unsigned long)old_func;
 			func_size = func->old_size;
 		} else {
 			/* previously patched function */
-			struct klp_func *prev;
-
-			prev = list_next_entry(func, stack_node);
 			func_addr = (unsigned long)prev->new_func;
 			func_size = prev->new_size;
 		}
@@ -266,8 +282,12 @@ static int klp_check_stack(struct task_struct *task, const char **oldname)
 		return -EINVAL;
 	nr_entries = ret;
 
+	/* A concurrent module unload may free klp_ops, see klp_find_ops(). */
+	guard(rcu)();
+
 	klp_for_each_object(klp_transition_patch, obj) {
-		if (!obj->patched)
+		/* Pairs with smp_store_release() in klp_patch_object(). */
+		if (!smp_load_acquire(&obj->patched))
 			continue;
 		klp_for_each_func(obj, func) {
 			ret = klp_check_stack_func(func, entries, nr_entries);

base-commit: 24a19b953e2a3d0ea5a8c4951a0ab8f32720c907
-- 
2.53.0

Re: [PATCH] livepatch: Fix races between stack check and module load/unload

[Peter Zijlstra]

On Mon, Oct 12, 2026 at 04:05:45PM +0200, Andrea Parri wrote:
> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> index 3f54a017bbf69..d01ec7a6256ab 100644
> --- a/kernel/livepatch/patch.c
> +++ b/kernel/livepatch/patch.c
> @@ -22,15 +22,19 @@
>  
>  static LIST_HEAD(klp_ops);
>  
> +/*
> + * Caller must hold klp_mutex or rcu_read_lock(): klp_check_stack_func() walks
> + * klp_ops from the scheduler without klp_mutex.
> + */
>  struct klp_ops *klp_find_ops(void *old_func)
>  {
>  	struct klp_ops *ops;
>  	struct klp_func *func;
>  
> -	list_for_each_entry(ops, &klp_ops, node) {
> -		func = list_first_entry(&ops->func_stack, struct klp_func,
> -					stack_node);
> -		if (func->old_func == old_func)
> +	list_for_each_entry_rcu(ops, &klp_ops, node, lockdep_is_held(&klp_mutex)) {
> +		func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
> +					      stack_node);
> +		if (func && READ_ONCE(func->old_func) == old_func)
>  			return ops;
>  	}
>  

> diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> index 2351a19ac2a9e..e9aa16baf1743 100644
> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -8,6 +8,7 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/cpu.h>
> +#include <linux/rculist.h>
>  #include <linux/stacktrace.h>
>  #include <linux/static_call.h>
>  #include "core.h"
> @@ -201,12 +202,16 @@ void klp_update_patch_state(struct task_struct *task)
>  /*
>   * Determine whether the given stack trace includes any references to a
>   * to-be-patched or to-be-unpatched function.
> + *
> + * Must be called under rcu_read_lock(), see klp_check_stack().
>   */
>  static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
>  				unsigned int nr_entries)
>  {
>  	unsigned long func_addr, func_size, address;
> +	struct klp_func *prev;
>  	struct klp_ops *ops;
> +	void *old_func;
>  	int i;
>  
>  	if (klp_target_state == KLP_TRANSITION_UNPATCHED) {

As per always, I much prefer things like lockdep_assert_held() and
__must_hold() over comments.

Re: [PATCH] livepatch: Fix races between stack check and module load/unload

[Andrea Parri]

On Mon, Oct 12, 2026 at 04:49:33PM +0200, Peter Zijlstra wrote:
> On Mon, Oct 12, 2026 at 04:05:45PM +0200, Andrea Parri wrote:
> > @@ -201,12 +202,16 @@ void klp_update_patch_state(struct task_struct *task)
> >  /*
> >   * Determine whether the given stack trace includes any references to a
> >   * to-be-patched or to-be-unpatched function.
> > + *
> > + * Must be called under rcu_read_lock(), see klp_check_stack().
> >   */
> >  static int klp_check_stack_func(struct klp_func *func, unsigned long *entries,
> >  				unsigned int nr_entries)
>
> As per always, I much prefer things like lockdep_assert_held() and
> __must_hold() over comments.

Makes sense.  For v2 I'll annotate klp_check_stack_func() with
__must_hold_shared(RCU) and add lockdep_assert_in_rcu_read_lock() there,
dropping the comment.  For klp_find_ops() the requirement is "klp_mutex
or RCU", which list_for_each_entry_rcu()'s lockdep condition already
checks under PROVE_RCU_LIST, so I'll just drop the comment there.

I'll wait a bit for comments from the livepatch folks before sending v2.

Thanks,
  Andrea

Re: [PATCH] livepatch: Fix races between stack check and module load/unload

[Miroslav Benes]

Hi Andrea,

Cc+ Petr Pavlu (you can find the whole thread at 
https://lore.kernel.org/live-patching/20261012150758.207983-1-parri.andrea@gmail.com/T/#t)

On Mon, 12 Oct 2026, Andrea Parri wrote:

> Unloading a patched module while a livepatch transition is in progress
> can crash the kernel in klp_find_ops(), called from the scheduler.
> KCSAN reports
> 
>   BUG: KCSAN: data-race in klp_find_ops / klp_unpatch_func
> 
>   write (marked) to 0xffff9094c09ece10 of 8 bytes by task 381 on cpu 2:
>    klp_unpatch_func+0x1bd/0x280
>    klp_unpatch_object+0x6a/0x90
>    klp_cleanup_module_patches_limited.cold+0x4a/0xd4
>    klp_module_going+0x54/0x80
>    __do_sys_delete_module+0x247/0x380
>    __x64_sys_delete_module+0x39/0x50
>    ...
> 
>   read to 0xffff9094c09ece10 of 8 bytes by task 146 on cpu 1:
>    klp_find_ops+0x55/0x90
>    klp_check_and_switch_task+0x228/0x320
>    klp_try_switch_task+0xb4/0xc0
>    __klp_sched_try_switch+0x9a/0xb0
>    __schedule+0x60a/0x1450
>    schedule+0x3b/0x100
>    schedule_timeout+0xc0/0x160
>    ra_target_a_loop+0x97/0xa0 [klp_race_target_a]
>    ...
> 
> followed by
> 
>   Oops: general protection fault, probably for non-canonical address
>   0xdead000000000110: 0000 [#2] SMP NOPTI
>   RIP: 0010:klp_find_ops+0x55/0x90
>    ...
>    klp_check_and_switch_task+0x228/0x320
>    klp_try_switch_task+0xb4/0xc0
>    __klp_sched_try_switch+0x9a/0xb0
>    __schedule+0x60a/0x1450

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 (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).

How did you find the issue? Is it a real problem in production?

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. Petr P., what do you think?

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

Regards
Miroslav

Re: [PATCH] livepatch: Fix races between stack check and module load/unload

[Andrea Parri]

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");

Re: [PATCH] livepatch: Fix races between stack check and module load/unload

[Petr Mladek]

On Wed 2026-10-14 14:23:50, Miroslav Benes wrote:
> Hi Andrea,
> 
> Cc+ Petr Pavlu (you can find the whole thread at 
> https://lore.kernel.org/live-patching/20261012150758.207983-1-parri.andrea@gmail.com/T/#t)
> 
> On Mon, 12 Oct 2026, Andrea Parri wrote:
> 
> > Unloading a patched module while a livepatch transition is in progress
> > can crash the kernel in klp_find_ops(), called from the scheduler.
> > KCSAN reports
> > 
> >   BUG: KCSAN: data-race in klp_find_ops / klp_unpatch_func
> > 
> >   write (marked) to 0xffff9094c09ece10 of 8 bytes by task 381 on cpu 2:
> >    klp_unpatch_func+0x1bd/0x280
> >    klp_unpatch_object+0x6a/0x90
> >    klp_cleanup_module_patches_limited.cold+0x4a/0xd4
> >    klp_module_going+0x54/0x80
> >    __do_sys_delete_module+0x247/0x380
> >    __x64_sys_delete_module+0x39/0x50
> >    ...
> > 
> >   read to 0xffff9094c09ece10 of 8 bytes by task 146 on cpu 1:
> >    klp_find_ops+0x55/0x90
> >    klp_check_and_switch_task+0x228/0x320
> >    klp_try_switch_task+0xb4/0xc0
> >    __klp_sched_try_switch+0x9a/0xb0
> >    __schedule+0x60a/0x1450
> >    schedule+0x3b/0x100
> >    schedule_timeout+0xc0/0x160
> >    ra_target_a_loop+0x97/0xa0 [klp_race_target_a]> >    ...
> > 
> > followed by
> > 
> >   Oops: general protection fault, probably for non-canonical address
> >   0xdead000000000110: 0000 [#2] SMP NOPTI
> >   RIP: 0010:klp_find_ops+0x55/0x90
> >    ...
> >    klp_check_and_switch_task+0x228/0x320
> >    klp_try_switch_task+0xb4/0xc0
> >    __klp_sched_try_switch+0x9a/0xb0
> >    __schedule+0x60a/0x1450
> 
> 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 (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).
> 
> How did you find the issue? Is it a real problem in production?
> 
> 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. Petr P., what do you think?
> 
> 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 agree that it is a can of worms. This patch is not super complicated.
It seems that it can't break anything. The bigger problem is whether
it solves all possible racy accesses. And how to prevent regressions
in the future.

Preventing the module unload during a livepatch transition might
be one solution. But I am afraid that people might want a better solution
sooner or later because the transition might never finish, ...

I wonder if we could use some more strict locking for the two
operations, e.g. (raw_)spin_lock. And then use

		if (!raw_spin_trylock(&klp_unpatch_object_lock))
			return -EBUSY;

in klp_check_stack(). Plus take the lock in __klp_unpatch_object().

Hmm, we could not take the spin_lock around klp_patch_object()
because it does allocations and might be running for a long time
when there are many patches objects. But this might be avoided by the
smp_store_release(&obj->patched, true) trick.

It still might be conceptually easier than making all the
accesses RCU-safe.

What do you think, please?

Best Regards,
Petr