[PATCH 0/8] kprobes: Fixes for registration errors, ftrace ops and optprobe state machine

[PATCH 0/8] kprobes: Fixes for registration errors, ftrace ops and optprobe state machine

[Masami Hiramatsu (Google)]

Hi,

Here is a series of bug fixes for kprobes and optprobes, addressing error
recovery, ftrace ops management, and optprobe asynchronous state machine
edge cases:

1. Registration & Unregistration Error Handling (Patches 1-3):
   - In __register_kprobe(), when arm_kprobe() fails, the error code was
     ignored and the instruction slot was leaked.
   - In unregister_kretprobes(), failing __unregister_kprobe_top() still
     invoked rethook/rph freeing, causing a double free or NULL deref.
     Clearing rp->rh is also deferred until after synchronize_rcu() to
     prevent racing with in-flight pre_handler_kretprobe() invocations.
   - In register_kretprobe(), re-registration validation was skipped when
     registered by symbol_name without an explicit addr, leading to
     destruction of the live probe's rethook instance on failure.

2. Ftrace Ops Switching for Post-Handlers (Patch 4):
   - When adding a second probe with a post_handler to an existing ftrace
     kprobe, the aggregate probe's post_handler was set but remained armed
     on kprobe_ftrace_ops. Subsequent disarming attempted to disarm from
     kprobe_ipmodify_ops, failing with -ENOENT and leaking the armed probe.
     Switch ftrace ops safely in register_aggr_kprobe() outside of
     text_mutex and cpus_read_lock.

3. Optprobe State Machine & Tasks RCU Quiescence (Patches 5-7):
   - Forcibly unoptimized probes (unoptimize_kprobe(force = true)) were
     not queued to cooling_list, causing kprobe_disarmed() to return true
     prematurely and risking use-after-free in the optinsn detour buffer
     before Tasks RCU completed.
   - Re-arming an unoptimizing probe before the unoptimization pass ran
     overwrote the first byte of the relative jump with a breakpoint,
     corrupting the jump opcode in text.
   - Probes killed at module unload while child probes were still active
     were prematurely freed or improperly unhashed before Tasks RCU
     quiescence elapsed.

In addition to the bugfixes, the last patch adds KUnit test cases for
these bugs. (Patch 8)

Summary of patches in this series:

[01/08] kprobes: Fix error return and slot leak in __register_kprobe()
 - Release allocated instruction slot via arch_remove_kprobe(p) and
   return the arm_kprobe() error code upon failure.

[02/08] kprobes: Fix double free and NULL deref in unregister_kretprobes()
 - Skip rethook_free() and rph cleanup if __unregister_kprobe_top()
   fails, and clear rp->rh after synchronize_rcu().

[03/08] kprobes: Validate re-registration earlier in register_kretprobe()
 - Resolve target address via _kprobe_addr() up-front and check whether
   the probe is already registered via check_kprobe_rereg_at() before
   allocating rethook resources, dropping the WARN_ON_ONCE().

[04/08] kprobes: Switch ftrace ops when adding probe with post_handler
 - Switch from kprobe_ftrace_ops to kprobe_ipmodify_ops in
   register_aggr_kprobe() outside of text_mutex and cpus_read_lock.
 - Restore ftrace ops counter and registration if ftrace_set_filter_ip()
   fails during disarm.

[05/08] kprobes: Fix UAF and queue cooling probes on forced unoptimization
 - Queue forcibly unoptimized probes onto cur->cooling_list (or
   cur->freeing_list if unused), include cooling_list in
   optprobe_has_queued_probes(), and isolate newly unoptimized probes
   into a local list in do_unoptimize_kprobes() to prevent double disarm.

[06/08] kprobes: Do not overwrite jump instruction when re-arming
        unoptimizing probe
 - Skip arch_arm_kprobe() in __arm_kprobe() when kprobe_queued_unopt()
   is true, keeping the relative jump intact while unoptimization is
   cancelled.
 - Force unoptimization immediately if optimize_kprobe() cannot optimize
   the re-armed probe.
 - Avoid premature flag clearing or dequeuing in reuse_unused_kprobe().

[07/08] kprobes: Fix slot reclaim and sibling re-opt in optprobe state machine
 - In kill_optimized_kprobe(), queue in-use probes on unoptimizing_list
   to cooling_list rather than freeing_list to preserve kprobe_table
   lookup and ensure Tasks RCU elapses before slot reclamation.
 - Reclaim optinsn slots for kprobe_gone() probes upon cooling completion
   in optprobe_finalize_generation().
 - Re-optimize blocked sibling probes when a cooling probe finishes
   cooling to safely disarmed.

[08/08] kprobes: Add KUnit test cases for kprobes regressions
 - Add test cases to lib/tests/test_kprobes.c covering unregistered
   kretprobe cleanup, kretprobe re-registration validation by symbol,
   ftrace ops dynamic switching on post_handler addition, and optprobe
   re-arming while unoptimizing.

Thank you,

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Masami Hiramatsu (Google) (8):
      kprobes: Fix error return and slot leak in __register_kprobe()
      kprobes: Fix double free and NULL deref in unregister_kretprobes()
      kprobes: Validate re-registration earlier in register_kretprobe()
      kprobes: Switch ftrace ops when adding probe with post_handler
      kprobes: Fix UAF and queue cooling probes on forced unoptimization
      kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe
      kprobes: Fix slot reclaim and sibling re-opt in optprobe state machine
      kprobes: Add KUnit test cases for kprobes regressions


 kernel/kprobes.c         |  226 ++++++++++++++++++++++++++++++----------------
 lib/tests/test_kprobes.c |  123 +++++++++++++++++++++++++
 2 files changed, 271 insertions(+), 78 deletions(-)

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>

[PATCH 1/8] kprobes: Fix error return and slot leak in __register_kprobe()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

When arm_kprobe() fails in __register_kprobe(), the probe is removed
from kprobe_table and synchronize_rcu() is executed, but the error code
ret was ignored and try_to_optimize_kprobe(p) was executed, returning 0.
Additionally, the instruction slot allocated by prepare_kprobe() was
leaked.

Fix this by releasing the instruction slot via arch_remove_kprobe(p)
and returning ret when arm_kprobe() fails.

Fixes: 587e8e6d640b ("kprobes: Adopt guard() and scoped_guard()")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 2441b9e39343..10946658bb35 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1946,6 +1946,8 @@ static int __register_kprobe(struct kprobe *p)
 		if (ret) {
 			hlist_del_rcu(&p->hlist);
 			synchronize_rcu();
+			arch_remove_kprobe(p);
+			return ret;
 		}
 	}
 

[PATCH 2/8] kprobes: Fix double free and NULL deref in unregister_kretprobes()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

In unregister_kretprobes(), if __unregister_kprobe_top() returns an error
(e.g., if the kretprobe was not registered or was already unregistered),
rethook_free(rps[i]->rh) or rcu_assign_pointer(rps[i]->rph->rp, NULL) is
still executed. For rethook-based kretprobes, calling rethook_free() on an
already-freed rethook causes a double-free / list corruption in RCU. For
legacy kretprobes, rps[i]->rph is NULL after previous cleanup, resulting
in a NULL pointer dereference.

Skip rethook_free() and rph cleanup if __unregister_kprobe_top() fails,
and clear rps[i]->rh after synchronize_rcu() has waited for in-flight
pre_handler_kretprobe() invocations to complete.

Fixes: 73f9b911faa7 ("kprobes: Use rethook for kretprobe if possible")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 10946658bb35..7a3da809bac7 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -2605,8 +2605,10 @@ void unregister_kretprobes(struct kretprobe **rps, int num)
 	for (i = 0; i < num; i++) {
 		guard(mutex)(&kprobe_mutex);
 
-		if (__unregister_kprobe_top(&rps[i]->kp) < 0)
+		if (__unregister_kprobe_top(&rps[i]->kp) < 0) {
 			rps[i]->kp.addr = NULL;
+			continue;
+		}
 #ifdef CONFIG_KRETPROBE_ON_RETHOOK
 		rethook_free(rps[i]->rh);
 #else
@@ -2618,7 +2620,9 @@ void unregister_kretprobes(struct kretprobe **rps, int num)
 	for (i = 0; i < num; i++) {
 		if (rps[i]->kp.addr) {
 			__unregister_kprobe_bottom(&rps[i]->kp);
-#ifndef CONFIG_KRETPROBE_ON_RETHOOK
+#ifdef CONFIG_KRETPROBE_ON_RETHOOK
+			rps[i]->rh = NULL;
+#else
 			free_rp_inst(rps[i]);
 #endif
 		}

[PATCH 3/8] kprobes: Validate re-registration earlier in register_kretprobe()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

register_kretprobe() only checked for re-registration if rp->kp.addr was
specified, skipping the check when rp was registered with symbol_name
and rp->kp.addr was NULL. In that scenario, register_kretprobe()
proceeded to allocate a new rethook (or kretprobe holder), and when
register_kprobe(&rp->kp) subsequently failed due to re-registration, the
error path freed and NULL-ed rp->rh (or rp->rph) on the still-active
kretprobe, leading to NULL pointer dereferences when the active probe
was hit.

Resolve the target address via _kprobe_addr() up-front and check whether
the probe is already registered at that address via
check_kprobe_rereg_at() before allocating any resources, and drop the
WARN_ON_ONCE() in the re-registration check now that re-registration is
gracefully detected and rejected.

Fixes: 0188b87899ff ("kretprobe: Avoid re-registration of the same kretprobe earlier")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |   72 ++++++++++++++++++++++++++++++------------------------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 7a3da809bac7..0b41d69540ae 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1793,52 +1793,61 @@ _kprobe_addr(kprobe_opcode_t *addr, const char *symbol_name,
 	return addr;
 }
 
-static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
-{
-	bool on_func_entry;
-
-	return _kprobe_addr(p->addr, p->symbol_name, p->offset, &on_func_entry);
-}
-
 /*
  * Check the 'p' is valid and return the aggregator kprobe
  * at the same address.
  */
-static struct kprobe *__get_valid_kprobe(struct kprobe *p)
+static struct kprobe *__get_valid_kprobe_at(struct kprobe *p, kprobe_opcode_t *addr)
 {
+	struct hlist_head *head;
 	struct kprobe *ap, *list_p;
 
 	lockdep_assert_held(&kprobe_mutex);
 
-	ap = get_kprobe(p->addr);
-	if (unlikely(!ap))
+	if (unlikely(!addr))
 		return NULL;
 
-	if (p == ap)
-		return ap;
-
-	list_for_each_entry(list_p, &ap->list, list)
-		if (list_p == p)
-		/* kprobe p is a valid probe */
+	head = &kprobe_table[hash_ptr(addr, KPROBE_HASH_BITS)];
+	hlist_for_each_entry_rcu(ap, head, hlist,
+				 lockdep_is_held(&kprobe_mutex)) {
+		if (ap == p) {
+			p->addr = addr;
 			return ap;
+		}
+		if (ap->addr == addr && kprobe_aggrprobe(ap)) {
+			list_for_each_entry(list_p, &ap->list, list) {
+				if (list_p == p) {
+					p->addr = addr;
+					return ap;
+				}
+			}
+		}
+	}
 
 	return NULL;
 }
 
-/*
- * Warn and return error if the kprobe is being re-registered since
- * there must be a software bug.
- */
-static inline int warn_kprobe_rereg(struct kprobe *p)
+static inline struct kprobe *__get_valid_kprobe(struct kprobe *p)
+{
+	return __get_valid_kprobe_at(p, p->addr);
+}
+
+/* Return error if the kprobe is being re-registered */
+static inline int check_kprobe_rereg_at(struct kprobe *p, kprobe_opcode_t *addr)
 {
 	guard(mutex)(&kprobe_mutex);
 
-	if (WARN_ON_ONCE(__get_valid_kprobe(p)))
+	if (__get_valid_kprobe_at(p, addr))
 		return -EINVAL;
 
 	return 0;
 }
 
+static inline int check_kprobe_rereg(struct kprobe *p)
+{
+	return check_kprobe_rereg_at(p, p->addr);
+}
+
 static int check_ftrace_location(struct kprobe *p)
 {
 	unsigned long addr = (unsigned long)p->addr;
@@ -1969,7 +1978,7 @@ int register_kprobe(struct kprobe *p)
 		return PTR_ERR(addr);
 	p->addr = addr;
 
-	ret = warn_kprobe_rereg(p);
+	ret = check_kprobe_rereg(p);
 	if (ret)
 		return ret;
 
@@ -2505,20 +2514,19 @@ int register_kretprobe(struct kretprobe *rp)
 	int ret;
 	int i;
 	void *addr;
+	bool on_func_entry;
 
-	ret = kprobe_on_func_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset);
-	if (ret)
-		return ret;
+	addr = _kprobe_addr(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset, &on_func_entry);
+	if (IS_ERR(addr))
+		return PTR_ERR(addr);
 
-	/* If only 'rp->kp.addr' is specified, check reregistering kprobes */
-	if (rp->kp.addr && warn_kprobe_rereg(&rp->kp))
+	if (!on_func_entry)
 		return -EINVAL;
 
-	if (kretprobe_blacklist_size) {
-		addr = kprobe_addr(&rp->kp);
-		if (IS_ERR(addr))
-			return PTR_ERR(addr);
+	if (check_kprobe_rereg_at(&rp->kp, addr))
+		return -EINVAL;
 
+	if (kretprobe_blacklist_size) {
 		for (i = 0; kretprobe_blacklist[i].name != NULL; i++) {
 			if (kretprobe_blacklist[i].addr == addr)
 				return -EINVAL;

[PATCH 4/8] kprobes: Switch ftrace ops when adding probe with post_handler

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

When an ftrace-based kprobe is registered without a post_handler, it is
armed using kprobe_ftrace_ops. If a second probe with a post_handler is
subsequently registered at the same address, register_aggr_kprobe() sets
ap->post_handler to aggr_post_handler, but leaves the probe armed using
kprobe_ftrace_ops.

When the probe is later disarmed or disabled, disarm_kprobe_ftrace()
checks whether p->post_handler is non-NULL and attempts to disarm from
kprobe_ipmodify_ops. This fails with -ENOENT, decrements
kprobe_ipmodify_enabled below zero, leaves the probe armed on
kprobe_ftrace_ops, and aborts unregistration, leading to a use-after-free
if the probe module is unloaded.

Switch the armed probe from kprobe_ftrace_ops to kprobe_ipmodify_ops in
register_aggr_kprobe() outside of text_mutex and cpus_read_lock to avoid
lockdep and deadlock issues when modifying ftrace code. Also restore
ftrace ops registration and counter if ftrace_set_filter_ip() fails in
__disarm_kprobe_ftrace().

Fixes: 5dd7caf0bdc5 ("kprobes: Skip clearing aggrprobe's post_handler in kprobe-on-ftrace case")
Cc: stable@vger.kernel.org
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |   56 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 46 insertions(+), 10 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0b41d69540ae..135fec5d49cf 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1435,9 +1435,14 @@ static int __disarm_kprobe_ftrace(struct kprobe *p, struct ftrace_ops *ops,
 	(*cnt)--;
 
 	ret = ftrace_set_filter_ip(ops, (unsigned long)p->addr, 1, 0);
-	WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (error %d)\n",
-		  p->addr, ret);
-	return ret;
+	if (WARN_ONCE(ret < 0, "Failed to disarm kprobe-ftrace at %pS (error %d)\n",
+		      p->addr, ret)) {
+		if (*cnt == 0)
+			register_ftrace_function(ops);
+		(*cnt)++;
+		return ret;
+	}
+	return 0;
 }
 
 static int disarm_kprobe_ftrace(struct kprobe *p)
@@ -1566,16 +1571,12 @@ void kprobe_busy_end(void)
 }
 
 /* Add the new probe to 'ap->list'. */
-static int add_new_kprobe(struct kprobe *ap, struct kprobe *p)
+static void add_new_kprobe(struct kprobe *ap, struct kprobe *p)
 {
 	if (p->post_handler)
 		unoptimize_kprobe(ap, true);	/* Fall back to normal kprobe */
 
 	list_add_rcu(&p->list, &ap->list);
-	if (p->post_handler && !ap->post_handler)
-		ap->post_handler = aggr_post_handler;
-
-	return 0;
 }
 
 /*
@@ -1608,6 +1609,8 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
 {
 	int ret = 0;
 	struct kprobe *ap = orig_p;
+	bool reset_post_handler = false;
+	bool ftrace_switch_ops = false;
 
 	scoped_guard(cpus_read_lock) {
 		/* For preparing optimization, jump_label_text_reserved() is called */
@@ -1654,18 +1657,51 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
 					| KPROBE_FLAG_DISABLED;
 		}
 
+		if (p->post_handler && !ap->post_handler) {
+			reset_post_handler = true;
+			if (kprobe_ftrace(ap) && !kprobes_all_disarmed &&
+			    !kprobe_disabled(ap))
+				ftrace_switch_ops = true;
+			else
+				ap->post_handler = aggr_post_handler;
+		}
+
 		/* Copy the insn slot of 'p' to 'ap'. */
 		copy_kprobe(ap, p);
-		ret = add_new_kprobe(ap, p);
+		add_new_kprobe(ap, p);
+	}
+
+	if (ftrace_switch_ops) {
+		/*
+		 * Must switch ftrace ops outside of text_mutex and cpus_read_lock
+		 * to prevent deadlocks when ftrace code modification acquires text_mutex.
+		 */
+		ret = disarm_kprobe_ftrace(ap);
+		if (ret) {
+			list_del_rcu(&p->list);
+			synchronize_rcu();
+			return ret;
+		}
+		ap->post_handler = aggr_post_handler;
+		ret = arm_kprobe_ftrace(ap);
+		if (ret) {
+			ap->post_handler = NULL;
+			arm_kprobe_ftrace(ap);
+			list_del_rcu(&p->list);
+			synchronize_rcu();
+			return ret;
+		}
 	}
 
-	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
+	if (kprobe_disabled(ap) && !kprobe_disabled(p)) {
 		ap->flags &= ~KPROBE_FLAG_DISABLED;
 		if (!kprobes_all_disarmed) {
 			/* Arm the breakpoint again. */
 			ret = arm_kprobe(ap);
 			if (ret) {
 				ap->flags |= KPROBE_FLAG_DISABLED;
+				if (reset_post_handler)
+					ap->post_handler = NULL;
 				list_del_rcu(&p->list);
 				synchronize_rcu();
 			}

Re: [PATCH 4/8] kprobes: Switch ftrace ops when adding probe with post_handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kprobes: premature kprobe publication onto ap->list in register_aggr_kprobe()
- [High] kprobes: unchecked rollback arm_kprobe_ftrace() in register_aggr_kprobe()
--

commit 90db6f90ad3cb02409cec15b4dabeb1b192665e6
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Switch ftrace ops when adding probe with post_handler

This commit switches an armed ftrace-based kprobe from kprobe_ftrace_ops to
kprobe_ipmodify_ops in register_aggr_kprobe() when a new kprobe with a
post_handler is added to an aggregated kprobe that previously had no
post_handler. It also restores the ftrace ops registration and counter if
ftrace_set_filter_ip() fails in __disarm_kprobe_ftrace().

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 0b41d69540ae4..135fec5d49cf5 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

[ ... ]

> @@ -1654,18 +1657,51 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p)
>  					| KPROBE_FLAG_DISABLED;
>  		}
>  
> +		if (p->post_handler && !ap->post_handler) {
> +			reset_post_handler = true;
> +			if (kprobe_ftrace(ap) && !kprobes_all_disarmed &&
> +			    !kprobe_disabled(ap))
> +				ftrace_switch_ops = true;
> +			else
> +				ap->post_handler = aggr_post_handler;
> +		}
> +
>  		/* Copy the insn slot of 'p' to 'ap'. */
>  		copy_kprobe(ap, p);
> -		ret = add_new_kprobe(ap, p);
> +		add_new_kprobe(ap, p);
> +	}

[Severity: High]
When ftrace_switch_ops is true in register_aggr_kprobe(), add_new_kprobe(ap, p)
publishes p onto ap->list via list_add_rcu() while ap is still armed on
kprobe_ftrace_ops and ap->post_handler is still NULL.

If another CPU hits the probed function before disarm_kprobe_ftrace(ap)
finishes updating the ftrace callsite, kprobe_ftrace_handler() will call
aggr_pre_handler() -> p->pre_handler(), and then skip aggr_post_handler()
because ap->post_handler is NULL:

arch/x86/kernel/kprobes/ftrace.c:kprobe_ftrace_handler() {
	...
	if (!p->pre_handler || !p->pre_handler(p, regs)) {
		if (unlikely(p->post_handler)) {
			instruction_pointer_set(regs, ip + MCOUNT_INSN_SIZE);
			kcb->kprobe_status = KPROBE_HIT_SSDONE;
			p->post_handler(p, regs, 0);
		}
	...
}

In addition, p->pre_handler() executes under kprobe_ftrace_ops before
arm_kprobe_ftrace(ap) has verified that kprobe_ipmodify_ops
(FTRACE_OPS_FL_IPMODIFY) does not conflict with an active livepatch. A similar
window also occurs in the error path below if arm_kprobe_ftrace(ap) fails,
where ap->post_handler is reset to NULL and ap is re-armed on
kprobe_ftrace_ops before list_del_rcu(&p->list) removes p.

Could add_new_kprobe(ap, p) be deferred until after ap has been switched to
kprobe_ipmodify_ops, or could p be removed from ap->list before re-arming ap
on rollback?

> +
> +	if (ftrace_switch_ops) {
> +		/*
> +		 * Must switch ftrace ops outside of text_mutex and cpus_read_lock
> +		 * to prevent deadlocks when ftrace code modification acquires text_mutex.
> +		 */
> +		ret = disarm_kprobe_ftrace(ap);
> +		if (ret) {
> +			list_del_rcu(&p->list);
> +			synchronize_rcu();
> +			return ret;
> +		}
> +		ap->post_handler = aggr_post_handler;
> +		ret = arm_kprobe_ftrace(ap);
> +		if (ret) {
> +			ap->post_handler = NULL;
> +			arm_kprobe_ftrace(ap);
> +			list_del_rcu(&p->list);
> +			synchronize_rcu();
> +			return ret;
> +		}
>  	}

[Severity: High]
What happens if the rollback call to arm_kprobe_ftrace(ap) in
register_aggr_kprobe() also fails (for example, with -ENOMEM in
ftrace_set_filter_ip() -> alloc_and_copy_ftrace_hash())?

Because the return value of the rollback arm_kprobe_ftrace(ap) is ignored and
KPROBE_FLAG_DISABLED is not set on ap->flags, ap remains marked as enabled
even though it is no longer registered in kprobe_ftrace_ops's filter hash.

When the remaining probe on ap is later disabled or unregistered via
unregister_kprobe() -> __unregister_kprobe_top() -> __disable_kprobe(),
__disable_kprobe() sees !kprobe_disabled(orig_p) and calls
disarm_kprobe_ftrace(), which fails with -ENOENT in ftrace_set_filter_ip(),
triggers the WARN_ONCE() in __disarm_kprobe_ftrace(), and aborts
unregistration while leaving the probe hashed in kprobe_table:

kernel/kprobes.c:__disable_kprobe() {
	...
	if (!kprobes_all_disarmed && !kprobe_disabled(orig_p)) {
		ret = disarm_kprobe(orig_p, true);
		if (ret) {
			p->flags &= ~KPROBE_FLAG_DISABLED;
			return ERR_PTR(ret);
		}
	}
	...
}

Should the rollback path check the return value of arm_kprobe_ftrace(ap) and
set KPROBE_FLAG_DISABLED on ap->flags if re-arming fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179375029807.247640.13024754173032838618.stgit@devnote2?part=4

[PATCH 5/8] kprobes: Fix UAF and queue cooling probes on forced unoptimization

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

When unoptimize_kprobe() is called with force = true on an actively
optimized probe (whose op->list is empty), force_unoptimize_kprobe()
replaces the relative jump with a breakpoint, but leaves op->list empty
without queueing op to cur->cooling_list or cur->freeing_list.

Because op->list remains empty, kprobe_disarmed() returns true
prematurely as soon as the probe is disabled. When unregister_kprobes()
runs, __unregister_kprobe_top() unhashes the probe immediately, and
unregister_kprobes() frees op and the optinsn slot after synchronize_rcu()
without waiting for Tasks RCU. Any task preempted inside the optinsn slot
then encounters a use-after-free when resuming execution.

Queue forcibly unoptimized probes onto cur->freeing_list (if unused) or
cur->cooling_list (if still in use), and include gen->cooling_list in
optprobe_has_queued_probes() so that the generation is dispatched and
flushed properly. In do_unoptimize_kprobes(), collect newly unoptimized
probes into a separate local list so that probes already placed on
freeing_list are not disarmed a second time.

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 135fec5d49cf..ceacd56245c2 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -552,17 +552,13 @@ static unsigned long optimizer_passes;
 
 #define OPTIMIZE_DELAY 5
 
-/*
- * Note: gen->cooling_list is not checked here because it is strictly
- * an in-flight holding list for dispatched generations, so it is always
- * empty in optprobe_cur_gen.
- */
 static bool optprobe_has_queued_probes(void)
 {
 	struct optprobe_generation *gen = &optprobe_gens[optprobe_cur_gen];
 
 	return !list_empty(&gen->optimizing_list) ||
 	       !list_empty(&gen->unoptimizing_list) ||
+	       !list_empty(&gen->cooling_list) ||
 	       !list_empty(&gen->freeing_list);
 }
 
@@ -612,6 +608,7 @@ static void do_unoptimize_kprobes(struct list_head *unopt_list,
 				  struct list_head *cooling_list)
 {
 	struct optimized_kprobe *op, *tmp;
+	LIST_HEAD(unoptimized);
 
 	lockdep_assert_held(&text_mutex);
 	/*
@@ -626,11 +623,13 @@ static void do_unoptimize_kprobes(struct list_head *unopt_list,
 	 */
 	lockdep_assert_cpus_held();
 
-	if (!list_empty(unopt_list))
-		arch_unoptimize_kprobes(unopt_list, free_list);
+	if (list_empty(unopt_list))
+		return;
 
-	/* Loop on 'free_list' for disarming and removing from kprobe hash list */
-	list_for_each_entry_safe(op, tmp, free_list, list) {
+	arch_unoptimize_kprobes(unopt_list, &unoptimized);
+
+	/* Loop on newly unoptimized probes for disarming and hash list removal */
+	list_for_each_entry_safe(op, tmp, &unoptimized, list) {
 		/* Switching from detour code to origin */
 		op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
 		/* Disarm probes if marked disabled and not gone */
@@ -643,6 +642,7 @@ static void do_unoptimize_kprobes(struct list_head *unopt_list,
 			 * (reclaiming is done by do_free_cleaned_kprobes().)
 			 */
 			hlist_del_init_rcu(&op->kp.hlist);
+			list_move(&op->list, free_list);
 		} else {
 			/*
 			 * Keep on cooling_list until the quiescence period
@@ -1039,6 +1039,15 @@ static void unoptimize_kprobe(struct kprobe *p, bool force)
 	if (force) {
 		/* Forcibly update the code: this is a special case */
 		force_unoptimize_kprobe(op);
+		if (kprobe_unused(&op->kp)) {
+			if (!kprobe_gone(&op->kp))
+				arch_disarm_kprobe(&op->kp);
+			hlist_del_init_rcu(&op->kp.hlist);
+			list_add(&op->list, &cur->freeing_list);
+		} else {
+			list_add(&op->list, &cur->cooling_list);
+		}
+		kick_kprobe_optimizer();
 	} else {
 		list_add(&op->list, &cur->unoptimizing_list);
 		kick_kprobe_optimizer();

Re: [PATCH 5/8] kprobes: Fix UAF and queue cooling probes on forced unoptimization

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] kprobes: unreachable kprobe_unused() branch in unoptimize_kprobe()
--

commit c6680d6b1b284ca1b2cdf402fac410e5adfc1db4
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Fix UAF and queue cooling probes on forced unoptimization

This patch fixes a use-after-free on forced unoptimization of an active
optimized kprobe by queueing the probe onto the current generation's list
and waiting for a Tasks RCU grace period before freeing it. It also separates
newly unoptimized probes into a local list in do_unoptimize_kprobes() so
probes already on freeing_list are not disarmed twice.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 135fec5d49cf5..ceacd56245c24 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

[ ... ]

> @@ -1039,6 +1039,15 @@ static void unoptimize_kprobe(struct kprobe *p, bool force)
>  	if (force) {
>  		/* Forcibly update the code: this is a special case */
>  		force_unoptimize_kprobe(op);
> +		if (kprobe_unused(&op->kp)) {
> +			if (!kprobe_gone(&op->kp))
> +				arch_disarm_kprobe(&op->kp);
> +			hlist_del_init_rcu(&op->kp.hlist);
> +			list_add(&op->list, &cur->freeing_list);
> +		} else {
> +			list_add(&op->list, &cur->cooling_list);
> +		}
> +		kick_kprobe_optimizer();

[Severity: Low]
Can kprobe_unused(&op->kp) ever evaluate to true here?

At the start of unoptimize_kprobe(), the function checks:

	if (!kprobe_aggrprobe(p) || kprobe_disarmed(p))
		return; /* This is not an optprobe nor optimized */

where kprobe_disarmed(p) for an aggregator probe is:

	return kprobe_disabled(p) && list_empty(&op->list);

And earlier in unoptimize_kprobe(), the !list_empty(&op->list) block handles
all probes already on a list and returns.

When execution reaches this final if (force) block, list_empty(&op->list) is
guaranteed to be true, which means kprobe_disabled(&op->kp) must be false
(otherwise kprobe_disarmed(p) at the top of unoptimize_kprobe() would have
returned early). Since force_unoptimize_kprobe(op) only clears
KPROBE_FLAG_OPTIMIZED and kprobe_unused(&op->kp) requires
kprobe_disabled(&op->kp) to be true, isn't the kprobe_unused(&op->kp) branch
unreachable here, leaving the probe to always be added to cur->cooling_list?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179375029807.247640.13024754173032838618.stgit@devnote2?part=5

[PATCH 6/8] kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

When an optimized probe is temporarily disarmed or disabled,
unoptimize_kprobe() places it on the unoptimizing list, but leaves
the relative jump in text until the optimizer thread runs.
__disarm_kprobe() explicitly skips arch_disarm_kprobe() when
kprobe_queued_unopt(p) is true.

However, if __arm_kprobe() is called before the unoptimization pass
executes, it unconditionally called arch_arm_kprobe(p), writing a
breakpoint instruction over the first byte of the jump while leaving the
trailing 4 bytes intact. optimize_kprobe(p) then dequeued the probe from
the unoptimizing queue, leaving the jump opcode overwritten and corrupted
in text.

Skip calling arch_arm_kprobe() in __arm_kprobe() when
kprobe_queued_unopt(p) is true, keeping the existing relative jump intact
while unoptimization is cancelled. If optimize_kprobe() cannot optimize
the probe (e.g. due to a post_handler or disabled optimization), force
unoptimization immediately. Also, in reuse_unused_kprobe(), do not
prematurely clear KPROBE_FLAG_DISABLED or call optimize_kprobe(), leaving
arming and optimization to register_aggr_kprobe().

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |   39 +++++++++++++++++----------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ceacd56245c2..0ef3a8701eda 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1059,19 +1059,11 @@ static int reuse_unused_kprobe(struct kprobe *ap)
 {
 	struct optimized_kprobe *op;
 
-	/*
-	 * Unused kprobe MUST be on the way of delayed unoptimizing (means
-	 * there is still a relative jump) and disabled.
-	 */
 	op = container_of(ap, struct optimized_kprobe, kp);
 	WARN_ON_ONCE(list_empty(&op->list));
-	/* Enable the probe again */
-	ap->flags &= ~KPROBE_FLAG_DISABLED;
-	/* Optimize it again. (remove from 'op->list') */
 	if (!kprobe_optready(ap))
 		return -EINVAL;
 
-	optimize_kprobe(ap);
 	return 0;
 }
 
@@ -1262,6 +1254,19 @@ static void __init kprobe_sysctls_init(void)
 }
 #endif /* CONFIG_SYSCTL */
 
+/* Return true if the probe is queued on unoptimizing lists */
+static bool kprobe_queued_unopt(struct kprobe *p)
+{
+	struct optimized_kprobe *op;
+
+	if (kprobe_aggrprobe(p)) {
+		op = container_of(p, struct optimized_kprobe, kp);
+		if (optprobe_queued_unopt(op))
+			return true;
+	}
+	return false;
+}
+
 /* Put a breakpoint for a probe. */
 static void __arm_kprobe(struct kprobe *p)
 {
@@ -1275,21 +1280,11 @@ static void __arm_kprobe(struct kprobe *p)
 		/* Fallback to unoptimized kprobe */
 		unoptimize_kprobe(_p, true);
 
-	arch_arm_kprobe(p);
+	if (!kprobe_queued_unopt(p))
+		arch_arm_kprobe(p);
 	optimize_kprobe(p);	/* Try to optimize (add kprobe to a list) */
-}
-
-/* Return true if the probe is queued on unoptimizing lists */
-static bool kprobe_queued_unopt(struct kprobe *p)
-{
-	struct optimized_kprobe *op;
-
-	if (kprobe_aggrprobe(p)) {
-		op = container_of(p, struct optimized_kprobe, kp);
-		if (optprobe_queued_unopt(op))
-			return true;
-	}
-	return false;
+	if (unlikely(kprobe_queued_unopt(p)))
+		unoptimize_kprobe(p, true);
 }
 
 /* Remove the breakpoint of a probe. */

Re: [PATCH 6/8] kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kprobes: unoptimize_kprobe() unhashes reused aggrprobe with post_handler
--

commit 9db5fa1da3716f288258348f174e01fe00a6f383
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe

This commit avoids calling arch_arm_kprobe() in __arm_kprobe() when a
probe is queued for unoptimization so that the existing jump instruction
is not overwritten with a software breakpoint. It also stops
reuse_unused_kprobe() from clearing KPROBE_FLAG_DISABLED or calling
optimize_kprobe() early, deferring arming and optimization to
register_aggr_kprobe().

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ceacd56245c24..0ef3a8701edad 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1059,19 +1059,11 @@ static int reuse_unused_kprobe(struct kprobe *ap)
>  {
>  	struct optimized_kprobe *op;
>  
> -	/*
> -	 * Unused kprobe MUST be on the way of delayed unoptimizing (means
> -	 * there is still a relative jump) and disabled.
> -	 */
>  	op = container_of(ap, struct optimized_kprobe, kp);
>  	WARN_ON_ONCE(list_empty(&op->list));
> -	/* Enable the probe again */
> -	ap->flags &= ~KPROBE_FLAG_DISABLED;
> -	/* Optimize it again. (remove from 'op->list') */
>  	if (!kprobe_optready(ap))
>  		return -EINVAL;
>  
> -	optimize_kprobe(ap);
>  	return 0;
>  }

[Severity: High]
Can leaving KPROBE_FLAG_DISABLED set and op->list queued here cause
unoptimize_kprobe() to unhash ap from kprobe_table when reusing an unused
aggrprobe for a new kprobe with a post_handler?

In kernel/kprobes.c, register_aggr_kprobe() calls reuse_unused_kprobe(ap)
and then calls add_new_kprobe(ap, p) before clearing KPROBE_FLAG_DISABLED:

register_aggr_kprobe(ap, p)
  if (kprobe_unused(ap)) {
      ret = reuse_unused_kprobe(ap);
      ...
  }
  ...
  add_new_kprobe(ap, p);
  if (kprobe_disabled(ap) && !kprobe_disabled(p)) {
      ap->flags &= ~KPROBE_FLAG_DISABLED;
      if (!kprobes_all_disarmed)
          ret = arm_kprobe(ap);
  }

Inside add_new_kprobe(), if p->post_handler is set, it calls
unoptimize_kprobe(ap, true) before adding p to ap->list:

static void add_new_kprobe(struct kprobe *ap, struct kprobe *p)
{
	if (p->post_handler)
		unoptimize_kprobe(ap, true);	/* Fall back to normal kprobe */

	list_add_rcu(&p->list, &ap->list);
}

Because KPROBE_FLAG_DISABLED is still set on ap, ap->list is still empty,
and op->list is still on unoptimizing_list (or cooling_list),
kprobe_unused(&op->kp) evaluates to true inside unoptimize_kprobe(ap, true):

	if (!list_empty(&op->list)) {
		if (optprobe_queued_unopt(op)) {
			if (force) {
				force_unoptimize_kprobe(op);
				if (kprobe_unused(&op->kp)) {
					if (!kprobe_gone(&op->kp))
						arch_disarm_kprobe(&op->kp);
					hlist_del_init_rcu(&op->kp.hlist);
					list_move(&op->list, &cur->freeing_list);
				} else {
					list_move(&op->list, &cur->cooling_list);
				}
				kick_kprobe_optimizer();
			}
		} else if (optprobe_queued_cooling(op)) {
			if (force && kprobe_unused(&op->kp)) {
				if (!kprobe_gone(&op->kp))
					arch_disarm_kprobe(&op->kp);
				hlist_del_init_rcu(&op->kp.hlist);
				list_move(&op->list, &cur->freeing_list);
				kick_kprobe_optimizer();
			}
		}

It looks like unoptimize_kprobe() will delete ap from kprobe_table via
hlist_del_init_rcu(&op->kp.hlist) and move op->list to cur->freeing_list.
Immediately after, add_new_kprobe() links p onto ap->list and
register_aggr_kprobe() clears KPROBE_FLAG_DISABLED and calls arm_kprobe(ap),
which writes a software breakpoint at ap->addr for a probe that is no longer
in kprobe_table, leading to an unhandled breakpoint trap when ap->addr is
executed and a warning in do_free_cleaned_kprobes().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179375029807.247640.13024754173032838618.stgit@devnote2?part=6

[PATCH 7/8] kprobes: Fix slot reclaim and sibling re-opt in optprobe state machine

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Several edge cases in the optprobe state machine can prematurely release
instruction slots or stall blocked sibling probes:

1. In kill_optimized_kprobe(), when a probed module is unloaded while child
probes still exist on p->list, the probe is not unused (!kprobe_unused(p)).
If op was queued for unoptimization, move it to the cooling list rather
than immediately calling arch_remove_optimized_kprobe(), ensuring Tasks
RCU elapses before slot reclamation. In optprobe_finalize_generation(),
reclaim the optinsn slot once cooling finishes for kprobe_gone() probes.

2. In optprobe_finalize_generation(), when a disabled cooling probe
finishes cooling and transitions to safely disarmed, re-optimize any
blocked sibling probe at the same address.

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/kprobes.c |   22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0ef3a8701eda..53725c09ef9b 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -725,10 +725,19 @@ static void optprobe_finalize_generation(struct optprobe_generation *gen)
 			list_move(&op->list, &optprobe_gens[optprobe_cur_gen].freeing_list);
 			kick_kprobe_optimizer();
 		} else {
+			struct kprobe *_p;
+
 			/* Still in use; now safely disarmed */
 			list_del_init(&op->list);
-			if (!kprobe_disabled(&op->kp))
+			if (kprobe_gone(&op->kp)) {
+				arch_remove_optimized_kprobe(op);
+			} else if (!kprobe_disabled(&op->kp)) {
 				optimize_kprobe(&op->kp);
+			} else {
+				_p = get_optimized_kprobe(op->kp.addr);
+				if (unlikely(_p))
+					optimize_kprobe(_p);
+			}
 		}
 	}
 
@@ -1084,13 +1093,20 @@ static void kill_optimized_kprobe(struct kprobe *p)
 				list_del_init(&op->list);
 			}
 		} else {
-			list_del_init(&op->list);
+			if (optprobe_queued_unopt(op)) {
+				/* Move to cooling list so optinsn slot is cooled down */
+				list_move(&op->list,
+					  &optprobe_gens[optprobe_cur_gen].cooling_list);
+				kick_kprobe_optimizer();
+			} else if (!optprobe_queued_cooling(op) && !optprobe_queued_freeing(op)) {
+				list_del_init(&op->list);
+			}
 		}
 	}
 	op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
 
 	/*
-	 * Don't remove the slot if it is queued for freeing or unoptimization;
+	 * Don't remove the slot if it is queued for freeing, unoptimization, or cooling;
 	 * the optimizer will reclaim it after the quiescence period.
 	 */
 	if (!optprobe_queued_freeing(op) && !optprobe_queued_unopt(op) &&

Re: [PATCH 7/8] kprobes: Fix slot reclaim and sibling re-opt in optprobe state machine

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] kprobes: slot leak and stale optinsn reuse via register_aggr_kprobe()
- [High] kprobes: missing arch_disarm_kprobe() in unoptimize_kprobe() on force
- [High] kprobes: premature unhash of rescued aggrprobe in add_new_kprobe()
--

commit 1b0eca43598590c99ad643a8f10e407a46a7cbcb
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Fix slot reclaim and sibling re-opt in optprobe state machine

This commit moves gone optimized probes with remaining child probes onto the
cooling list in kill_optimized_kprobe() and reclaims their instruction slots
in optprobe_finalize_generation() after Tasks RCU completes. It also
re-optimizes blocked sibling probes in optprobe_finalize_generation() once a
disabled probe finishes cooling.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 0ef3a8701edad..53725c09ef9b0 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -725,10 +725,19 @@ static void optprobe_finalize_generation(struct optprobe_generation *gen)
>  			list_move(&op->list, &optprobe_gens[optprobe_cur_gen].freeing_list);
>  			kick_kprobe_optimizer();
>  		} else {
> +			struct kprobe *_p;
> +
>  			/* Still in use; now safely disarmed */
>  			list_del_init(&op->list);
> -			if (!kprobe_disabled(&op->kp))
> +			if (kprobe_gone(&op->kp)) {
> +				arch_remove_optimized_kprobe(op);
> +			} else if (!kprobe_disabled(&op->kp)) {
>  				optimize_kprobe(&op->kp);
> +			} else {
> +				_p = get_optimized_kprobe(op->kp.addr);
> +				if (unlikely(_p))
> +					optimize_kprobe(_p);
> +			}

[Severity: High]
Can this re-optimize a sibling probe _p while a software breakpoint is still
armed at op->kp.addr?

When an optimized probe is disabled via disable_kprobe(), __disable_kprobe()
calls __disarm_kprobe(), which calls unoptimize_kprobe(p, false) and skips
arch_disarm_kprobe(p) because kprobe_queued_unopt(p) is true. Because the
disabled child probe remains on op->kp.list, kprobe_disabled(&op->kp) is true
while kprobe_unused(&op->kp) is false.

If unoptimize_kprobe(&op->kp, true) is then called with force = true before
do_unoptimize_kprobes() runs (for example, when an overlapping probe is armed
via __arm_kprobe() -> get_optimized_kprobe() -> unoptimize_kprobe(_p, true),
or when a child probe with a post_handler is added), force_unoptimize_kprobe()
replaces the jump at op->kp.addr with a software breakpoint. However, unlike
do_unoptimize_kprobes():

kernel/kprobes.c:do_unoptimize_kprobes() {
	...
		if (kprobe_disabled(&op->kp) && !kprobe_gone(&op->kp))
			arch_disarm_kprobe(&op->kp);
	...
}

unoptimize_kprobe() only calls arch_disarm_kprobe(&op->kp) when
kprobe_unused(&op->kp) is true:

kernel/kprobes.c:unoptimize_kprobe() {
	...
				force_unoptimize_kprobe(op);
				if (kprobe_unused(&op->kp)) {
					if (!kprobe_gone(&op->kp))
						arch_disarm_kprobe(&op->kp);
					hlist_del_init_rcu(&op->kp.hlist);
					list_move(&op->list, &cur->freeing_list);
				} else {
					list_move(&op->list, &cur->cooling_list);
				}
	...
}

When optprobe_finalize_generation() finishes cooling op and runs this else
branch, op->list is emptied (making kprobe_disarmed(&op->kp) true) and
optimize_kprobe(_p) is called on the overlapping probe while the breakpoint at
op->kp.addr is still armed in kernel text. Wouldn't arch_optimize_kprobes()
then copy the undisarmed breakpoint instruction into _p->optinsn.copied_insn,
and leave an orphaned breakpoint in kernel text if op is later unregistered?

[Severity: High]
In the same optprobe state machine, what happens when an unused aggregator
probe on unoptimizing_list or cooling_list is rescued by
register_aggr_kprobe() for a new kprobe p that has a post_handler?

In commit 9db5fa1da371 ("kprobes: Do not overwrite jump instruction when
re-arming unoptimizing probe"), reuse_unused_kprobe() was changed to leave
KPROBE_FLAG_DISABLED set on ap and leave op on unoptimizing_list or
cooling_list:

kernel/kprobes.c:reuse_unused_kprobe() {
	struct optimized_kprobe *op;

	op = container_of(ap, struct optimized_kprobe, kp);
	WARN_ON_ONCE(list_empty(&op->list));
	if (!kprobe_optready(ap))
		return -EINVAL;

	return 0;
}

register_aggr_kprobe() then calls add_new_kprobe(ap, p), which calls
unoptimize_kprobe(ap, true) before adding p->list to ap->list:

kernel/kprobes.c:add_new_kprobe() {
	if (p->post_handler)
		unoptimize_kprobe(ap, true);	/* Fall back to normal kprobe */

	list_add_rcu(&p->list, &ap->list);
}

Because ap->list is still empty and KPROBE_FLAG_DISABLED is still set when
unoptimize_kprobe(ap, true) runs, kprobe_unused(&op->kp) evaluates to true in
both the optprobe_queued_unopt(op) and optprobe_queued_cooling(op) branches of
unoptimize_kprobe(). Doesn't unoptimize_kprobe(ap, true) then unhash
op->kp.hlist from kprobe_table and move op->list to cur->freeing_list right
before register_aggr_kprobe() arms a breakpoint at ap->addr for the unhashed
probe?

>  		}
>  	}
>  
> @@ -1084,13 +1093,20 @@ static void kill_optimized_kprobe(struct kprobe *p)
>  				list_del_init(&op->list);
>  			}
>  		} else {
> -			list_del_init(&op->list);
> +			if (optprobe_queued_unopt(op)) {
> +				/* Move to cooling list so optinsn slot is cooled down */
> +				list_move(&op->list,
> +					  &optprobe_gens[optprobe_cur_gen].cooling_list);
> +				kick_kprobe_optimizer();
> +			} else if (!optprobe_queued_cooling(op) && !optprobe_queued_freeing(op)) {
> +				list_del_init(&op->list);
> +			}
>  		}
>  	}
>  	op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
>  
>  	/*
> -	 * Don't remove the slot if it is queued for freeing or unoptimization;
> +	 * Don't remove the slot if it is queued for freeing, unoptimization, or cooling;
>  	 * the optimizer will reclaim it after the quiescence period.
>  	 */
>  	if (!optprobe_queued_freeing(op) && !optprobe_queued_unopt(op) &&

[Severity: High]
Does deferring arch_remove_optimized_kprobe(op) here leak op->optinsn.insn or
reuse a stale optinsn trampoline if a new kprobe is registered at the same
virtual address before cooling completes?

When kill_kprobe() runs on a probe that still has child probes on p->list
(!kprobe_unused(p)) while op is on unoptimizing_list or cooling_list,
kill_optimized_kprobe() now leaves op on cooling_list with op->optinsn.insn
allocated, while ap (&op->kp) remains hashed in kprobe_table with
KPROBE_FLAG_GONE set.

If a module is loaded at the same virtual address and a new kprobe is
registered at ap->addr before optprobe_finalize_generation() runs,
__register_kprobe() finds ap in kprobe_table and calls
register_aggr_kprobe():

kernel/kprobes.c:register_aggr_kprobe() {
	...
		if (kprobe_gone(ap)) {
			ret = arch_prepare_kprobe(ap);
			if (ret)
				return ret;

			/* Prepare optimized instructions if possible. */
			prepare_optimized_kprobe(ap);

			ap->flags = (ap->flags & ~KPROBE_FLAG_GONE)
					| KPROBE_FLAG_DISABLED;
		}
	...
}

Because register_aggr_kprobe() assumes the old instruction slots were already
freed by kill_kprobe(), it calls prepare_optimized_kprobe(ap) ->
arch_prepare_optimized_kprobe(op, ap) and clears KPROBE_FLAG_GONE:

1. If arch_prepare_optimized_kprobe() allocates a new slot, it overwrites
   op->optinsn.insn without freeing the in-flight cooling slot, leaking the
   old optinsn slot (and since KPROBE_FLAG_GONE is cleared,
   optprobe_finalize_generation() will not reclaim it either).

2. If arch_prepare_optimized_kprobe() fails before writing op->optinsn.insn
   (for example, when can_optimize() rejects the new function at ap->addr),
   op->optinsn.insn still points to the old unloaded module's optinsn slot.
   Once register_aggr_kprobe() clears KPROBE_FLAG_GONE and arms ap, won't
   optprobe_finalize_generation() see !kprobe_gone(&op->kp) and
   !kprobe_disabled(&op->kp), and call optimize_kprobe(&op->kp) using the
   stale optinsn trampoline from the unloaded module?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179375029807.247640.13024754173032838618.stgit@devnote2?part=7

[PATCH 8/8] kprobes: Add KUnit test cases for kprobes regressions

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Add KUnit test cases to lib/tests/test_kprobes.c to verify that the
recent regressions in kprobes registration, ftrace ops switching, and
optimization state transitions are resolved:

1. test_kretprobe_unregistered_cleanup:
   Verifies that unregistering an uninitialized or already-unregistered
   kretprobe does not cause a NULL pointer dereference or double free.

2. test_kretprobe_rereg_by_symbol:
   Verifies that attempting to re-register an active kretprobe by symbol
   name fails with -EINVAL and does not destroy the active probe's
   underlying rethook state.

3. test_kprobe_ftrace_post_handler_switch:
   Verifies that adding a second kprobe with a post_handler to a probe
   point armed with kprobe_ftrace_ops correctly switches to
   kprobe_ipmodify_ops and both probes cleanly unregister without
   disarming errors (-ENOENT).

4. test_optprobe_rearm_unoptimizing:
   Verifies that disabling an optimized kprobe and immediately
   re-enabling it before the optimizer thread runs does not overwrite the
   jump instruction opcode or crash on execution.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 lib/tests/test_kprobes.c |  123 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 123 insertions(+)

diff --git a/lib/tests/test_kprobes.c b/lib/tests/test_kprobes.c
index 06e729e4de05..c20f70cc63d7 100644
--- a/lib/tests/test_kprobes.c
+++ b/lib/tests/test_kprobes.c
@@ -358,8 +358,123 @@ static void test_stacktrace_on_nested_kretprobe(struct kunit *test)
 }
 #endif /* CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE */
 
+/* Test for: kprobes: Fix double free and NULL deref in unregister_kretprobes() */
+static void test_kretprobe_unregistered_cleanup(struct kunit *test)
+{
+	struct kretprobe unreg_rp = {
+		.kp.symbol_name = "kprobe_target",
+	};
+
+	current_test = test;
+
+	/* Unregistering a probe that was never registered must return cleanly */
+	unregister_kretprobe(&unreg_rp);
+
+	/* Double unregister of a valid registered probe */
+	KUNIT_ASSERT_EQ(test, 0, register_kretprobe(&unreg_rp));
+	unregister_kretprobe(&unreg_rp);
+	/* Second unregister must not cause double-free or NULL pointer dereference */
+	unregister_kretprobe(&unreg_rp);
+}
+
+/* Test for: kprobes: Validate re-registration earlier in register_kretprobe() */
+static void test_kretprobe_rereg_by_symbol(struct kunit *test)
+{
+	struct kretprobe test_rp = {
+		.kp.symbol_name = "kprobe_target",
+		.handler = return_handler,
+		.entry_handler = entry_handler,
+	};
+
+	current_test = test;
+	KUNIT_ASSERT_EQ(test, 0, register_kretprobe(&test_rp));
+
+	/* Verify the probe is live and functional */
+	krph_val = 0;
+	target(rand1);
+	KUNIT_EXPECT_EQ(test, krph_val, rand1);
+
+	/*
+	 * Attempt re-registration with symbol_name (with kp.addr cleared).
+	 * Must fail with -EINVAL and NOT destroy the live probe's rethook.
+	 */
+	test_rp.kp.addr = NULL;
+	KUNIT_EXPECT_EQ(test, -EINVAL, register_kretprobe(&test_rp));
+
+	/* The original kretprobe must still be functional */
+	krph_val = 0;
+	target(rand1);
+	KUNIT_EXPECT_EQ(test, krph_val, rand1);
+
+	unregister_kretprobe(&test_rp);
+}
+
 #endif /* CONFIG_KRETPROBES */
 
+#if defined(CONFIG_KPROBES_ON_FTRACE)
+/* Test for: kprobes: Switch ftrace ops when adding probe with post_handler */
+static void test_kprobe_ftrace_post_handler_switch(struct kunit *test)
+{
+	struct kprobe kp_entry = {
+		.symbol_name = "kprobe_target",
+		.pre_handler = kp_pre_handler,
+	};
+	struct kprobe kp_post = {
+		.symbol_name = "kprobe_target",
+		.post_handler = kp_post_handler,
+	};
+
+	current_test = test;
+
+	/* 1. Register first probe without post_handler (armed with kprobe_ftrace_ops) */
+	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&kp_entry));
+
+	/* 2. Register second probe with post_handler at same IP (must switch ops) */
+	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&kp_post));
+
+	/* 3. Execute target function to trigger both */
+	preh_val = 0;
+	posth_val = 0;
+	target(rand1);
+	KUNIT_EXPECT_NE(test, 0, preh_val);
+	KUNIT_EXPECT_NE(test, 0, posth_val);
+
+	/* 4. Unregistering must successfully disarm without -ENOENT */
+	unregister_kprobe(&kp_post);
+	unregister_kprobe(&kp_entry);
+}
+#endif
+
+#if defined(CONFIG_OPTPROBES)
+/* Test for: kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe */
+static void test_optprobe_rearm_unoptimizing(struct kunit *test)
+{
+	struct kprobe opt_kp = {
+		.symbol_name = "kprobe_target",
+		.pre_handler = kp_pre_handler,
+	};
+
+	current_test = test;
+	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&opt_kp));
+
+	/* Allow background optimizer thread to jump-optimize the probe */
+	wait_for_kprobe_optimizer();
+
+	/* Temporarily disable probe (queues on unoptimizing_list, leaves jump) */
+	KUNIT_ASSERT_EQ(test, 0, disable_kprobe(&opt_kp));
+
+	/* Immediately re-enable (re-arm) before optimizer thread unoptimizes */
+	KUNIT_ASSERT_EQ(test, 0, enable_kprobe(&opt_kp));
+
+	/* Verify jump opcode was not corrupted by an unaligned INT3 overwrite */
+	preh_val = 0;
+	target(rand1);
+	KUNIT_EXPECT_NE(test, 0, preh_val);
+
+	unregister_kprobe(&opt_kp);
+}
+#endif
+
 static int kprobes_test_init(struct kunit *test)
 {
 	KP_CLEAR(kp);
@@ -391,10 +506,18 @@ static struct kunit_case kprobes_testcases[] = {
 #ifdef CONFIG_KRETPROBES
 	KUNIT_CASE(test_kretprobe),
 	KUNIT_CASE(test_kretprobes),
+	KUNIT_CASE(test_kretprobe_unregistered_cleanup),
+	KUNIT_CASE(test_kretprobe_rereg_by_symbol),
 #ifdef CONFIG_ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
 	KUNIT_CASE(test_stacktrace_on_kretprobe),
 	KUNIT_CASE(test_stacktrace_on_nested_kretprobe),
 #endif
+#endif
+#if defined(CONFIG_KPROBES_ON_FTRACE)
+	KUNIT_CASE(test_kprobe_ftrace_post_handler_switch),
+#endif
+#if defined(CONFIG_OPTPROBES)
+	KUNIT_CASE(test_optprobe_rearm_unoptimizing),
 #endif
 	{}
 };

Re: [PATCH 8/8] kprobes: Add KUnit test cases for kprobes regressions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] kprobes: test_optprobe_rearm_unoptimizing() probes ftrace entry
- [Medium] kprobes: stack UAF if KUNIT_ASSERT_EQ() aborts with registered kprobe
- [Low] kprobes: unexported wait_for_kprobe_optimizer() breaks module build
--

commit 879cc55db87246da67610c427cb32cb35c372768
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

kprobes: Add KUnit test cases for kprobes regressions

This commit adds four KUnit test cases to lib/tests/test_kprobes.c to
verify fixes for regressions in kretprobe unregistration, kretprobe
re-registration, ftrace ops switching, and optprobe re-arming.

> diff --git a/lib/tests/test_kprobes.c b/lib/tests/test_kprobes.c
> index 06e729e4de051..c20f70cc63d73 100644
> --- a/lib/tests/test_kprobes.c
> +++ b/lib/tests/test_kprobes.c
[ ... ]
> +#if defined(CONFIG_KPROBES_ON_FTRACE)
> +/* Test for: kprobes: Switch ftrace ops when adding probe with post_handler */
> +static void test_kprobe_ftrace_post_handler_switch(struct kunit *test)
> +{
> +	struct kprobe kp_entry = {
> +		.symbol_name = "kprobe_target",
> +		.pre_handler = kp_pre_handler,
> +	};
> +	struct kprobe kp_post = {
> +		.symbol_name = "kprobe_target",
> +		.post_handler = kp_post_handler,
> +	};
> +
> +	current_test = test;
> +
> +	/* 1. Register first probe without post_handler (armed with kprobe_ftrace_ops) */
> +	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&kp_entry));
> +
> +	/* 2. Register second probe with post_handler at same IP (must switch ops) */
> +	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&kp_post));

[Severity: Medium]
In test_kprobe_ftrace_post_handler_switch(), kp_entry is allocated on the
test kthread stack. If the second register_kprobe(&kp_post) call fails (for
example, if alloc_aggr_kprobe() or ftrace ops switching fails), the assertion
macro aborts the test immediately via:

  __kunit_abort()
    -> kunit_try_catch_throw()
      -> kthread_exit(0)

Would this exit the kthread and free its stack while kp_entry is still
registered in kprobe_table, leaving a dangling stack pointer that causes a
stack use-after-free?

The same question applies to test_optprobe_rearm_unoptimizing() below, where
opt_kp is on the stack and registered before the assertions on
disable_kprobe(&opt_kp) and enable_kprobe(&opt_kp).

> +
> +	/* 3. Execute target function to trigger both */
> +	preh_val = 0;
> +	posth_val = 0;
> +	target(rand1);
> +	KUNIT_EXPECT_NE(test, 0, preh_val);
> +	KUNIT_EXPECT_NE(test, 0, posth_val);
> +
> +	/* 4. Unregistering must successfully disarm without -ENOENT */
> +	unregister_kprobe(&kp_post);
> +	unregister_kprobe(&kp_entry);
> +}
> +#endif
> +
> +#if defined(CONFIG_OPTPROBES)
> +/* Test for: kprobes: Do not overwrite jump instruction when re-arming unoptimizing probe */
> +static void test_optprobe_rearm_unoptimizing(struct kunit *test)
> +{
> +	struct kprobe opt_kp = {
> +		.symbol_name = "kprobe_target",
> +		.pre_handler = kp_pre_handler,
> +	};
> +
> +	current_test = test;
> +	KUNIT_ASSERT_EQ(test, 0, register_kprobe(&opt_kp));

[Severity: Medium]
Does test_optprobe_rearm_unoptimizing() actually exercise jump optimization
when ftrace-based kprobes are enabled?

Because opt_kp is registered at kprobe_target with offset 0, on kernels with
ftrace-based kprobes enabled, register_kprobe() marks the probe as an ftrace
kprobe:

  register_kprobe()
    -> check_kprobe_address_safe()
      -> check_ftrace_location()
         if (ftrace_location(addr) == addr)
             p->flags |= KPROBE_FLAG_FTRACE;

Then __register_kprobe() calls try_to_optimize_kprobe(), which bails out
immediately:

  try_to_optimize_kprobe()
    if (kprobe_ftrace(p))
        return;

As a result, disable_kprobe() and enable_kprobe() take the ftrace disarm/arm
paths instead of queuing opt_kp on unoptimizing_list and re-arming an
optimized probe. Should opt_kp be placed at a non-zero instruction offset
within kprobe_target, or target a notrace function, so that jump optimization
is actually used?

> +
> +	/* Allow background optimizer thread to jump-optimize the probe */
> +	wait_for_kprobe_optimizer();

[Severity: Low]
Can this break the build when the test is compiled as a loadable module?

In lib/Kconfig.debug, this test suite is tristate:

  config KPROBES_SANITY_TEST
  	tristate "Kprobes sanity tests" if !KUNIT_ALL_TESTS

However, wait_for_kprobe_optimizer() in kernel/kprobes.c is not exported to
modules, causing modpost to fail with an undefined symbol when the test is
built as a module with optprobes enabled. Should wait_for_kprobe_optimizer()
be exported, or should this test be guarded like the stack_trace_save_regs()
callers earlier in lib/tests/test_kprobes.c?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179375029807.247640.13024754173032838618.stgit@devnote2?part=8

Re: [PATCH 0/8] kprobes: Fixes for registration errors, ftrace ops and optprobe state machine

[Masami Hiramatsu (Google)]

On Wed,  4 Nov 2026 08:58:18 +0900, Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> Hi,
> 
> Here is a series of bug fixes for kprobes and optprobes, addressing error
> recovery, ftrace ops management, and optprobe asynchronous state machine
> edge cases:
> 
> 1. Registration & Unregistration Error Handling (Patches 1-3):
>    - In __register_kprobe(), when arm_kprobe() fails, the error code was
>      ignored and the instruction slot was leaked.
>    - In unregister_kretprobes(), failing __unregister_kprobe_top() still
>      invoked rethook/rph freeing, causing a double free or NULL deref.
>      Clearing rp->rh is also deferred until after synchronize_rcu() to
>      prevent racing with in-flight pre_handler_kretprobe() invocations.
>    - In register_kretprobe(), re-registration validation was skipped when
>      registered by symbol_name without an explicit addr, leading to
>      destruction of the live probe's rethook instance on failure.

I think these are urgent fixes. So I'll pick those to probes/fixes.

> 
> 2. Ftrace Ops Switching for Post-Handlers (Patch 4):
>    - When adding a second probe with a post_handler to an existing ftrace
>      kprobe, the aggregate probe's post_handler was set but remained armed
>      on kprobe_ftrace_ops. Subsequent disarming attempted to disarm from
>      kprobe_ipmodify_ops, failing with -ENOENT and leaking the armed probe.
>      Switch ftrace ops safely in register_aggr_kprobe() outside of
>      text_mutex and cpus_read_lock.

This should be updated to v2.

> 
> 3. Optprobe State Machine & Tasks RCU Quiescence (Patches 5-7):
>    - Forcibly unoptimized probes (unoptimize_kprobe(force = true)) were
>      not queued to cooling_list, causing kprobe_disarmed() to return true
>      prematurely and risking use-after-free in the optinsn detour buffer
>      before Tasks RCU completed.
>    - Re-arming an unoptimizing probe before the unoptimization pass ran
>      overwrote the first byte of the relative jump with a breakpoint,
>      corrupting the jump opcode in text.
>    - Probes killed at module unload while child probes were still active
>      were prematurely freed or improperly unhashed before Tasks RCU
>      quiescence elapsed.

I need to update these and rebase on Mani's fixes [1] after he fixed
problems, since that will fix the similar problems. These will be
probes/for-next branch.

[1] https://lore.kernel.org/r/20261103-kprobes-optprobe-state-v1-0-6aaac612abe6@columbia.edu

> 
> In addition to the bugfixes, the last patch adds KUnit test cases for
> these bugs. (Patch 8)

This also needs to be updated, but it will also go to probes/for-next.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>