[PATCH 0/3] kprobes: Fix optprobe state after forced unoptimization

[PATCH 0/3] kprobes: Fix optprobe state after forced unoptimization

[Mani Tofigh via B4 Relay]

Since commit 06e0d3b89da1 ("kprobes: Make optprobe optimizer
multi-generational and asynchronous") a forced unoptimization can leave
a probe's int3 in the wrong state.  A disabled probe can be freed with
its int3 still in the text, an unhashed probe gets disarmed twice and a
reused probe never gets armed.

Patch 2 relies on patch 1 since it assumes every disabled probe on a
cooling list is already disarmed.

---
Mani Tofigh (3):
      kprobes: Disarm disabled optprobe on forced unoptimization
      kprobes: Do not disarm unhashed optprobe on dispatch
      kprobes: Arm reused optprobe on cooling list

 kernel/kprobes.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)
---
base-commit: d4d028a7ae6401ce70825a3eb663f0a2dfc49db6
change-id: 20261103-kprobes-optprobe-state-90dbf55b246c

Best regards,
--  
Mani Tofigh <mt4022@columbia.edu>

[PATCH 1/3] kprobes: Disarm disabled optprobe on forced unoptimization

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

A disabled optimized probe can be freed with its int3 still in the
text.  It waits on the unoptimizing list for the optimizer to remove its
jump.  If another probe is armed on one of the bytes that jump replaced,
unoptimize_kprobe() forces the unoptimization and moves the disabled
probe straight to a cooling list without removing its int3:

  Task A (tracefs)                       Task B (optimizer)
  disable k1
    /* k1 queued to unoptimize */
                                         /* still on an earlier batch */
  enable k2 on a byte k1's jump replaced
    unoptimize_kprobe(k1, true)
      /* int3 back, k1 on a cooling list */
  remove k1
    /* k1 freed, int3 left in the text */

The next call into the function then oopses:

  [    9.924313] Oops: int3: 0000 [#1] SMP KASAN NOPTI
  [    9.925627] CPU: 0 UID: 0 PID: 68 Comm: kprobeopt Tainted: G        W            7.3.0-17087-gd865f2677c44 #1 PREEMPT
  [    9.930933] RIP: 0010:__do_sys_sysinfo+0x8/0xe0
  ...
  [    9.949348] Call Trace:
  [    9.949890]  <TASK>
  [    9.950404]  do_syscall_64+0xfb/0x590
  [    9.952027]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
  ...
  [    9.966436]  </TASK>
  ...
  [    9.968009] Kernel panic - not syncing: Fatal exception in interrupt

Disarm a disabled probe on that path like do_unoptimize_kprobes() does.
With the fix, the int3 is gone by the time the probe is freed.

This bug was identified and confirmed with the help of a deterministic
hypervisor currently under development.  The C reproducer below however
hits it within about 10s.

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
Reproducer: https://github.com/manitofigh/files/blob/main/linux-repros/kprobeopt.c
---
 kernel/kprobes.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 11add929ea16..4c35cc24aadc 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1006,6 +1006,9 @@ static void unoptimize_kprobe(struct kprobe *p, bool force)
 					hlist_del_init_rcu(&op->kp.hlist);
 					list_move(&op->list, &cur->freeing_list);
 				} else {
+					/* Disarm probes if marked disabled and not gone */
+					if (kprobe_disabled(&op->kp) && !kprobe_gone(&op->kp))
+						arch_disarm_kprobe(&op->kp);
 					list_move(&op->list, &cur->cooling_list);
 				}
 				kick_kprobe_optimizer();

-- 
2.53.0

[PATCH 2/3] kprobes: Do not disarm unhashed optprobe on dispatch

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

A new probe at the address of a force-unoptimized probe can stop
firing.  When unoptimize_kprobe() force-unoptimizes an unused probe it
disarms and unhashes it, but the next dispatch disarms it again.  If a
new probe was registered at the same address in the meantime, the
second disarm clears the new probe's int3.  For a module address the
same disarm would write to freed module text after rmmod.

Skip the disarm for unhashed probes since they're always disarmed before
they're unhashed.  With the fix, the new probe fires.

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
 kernel/kprobes.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 4c35cc24aadc..84a5f68476eb 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -631,8 +631,12 @@ static void do_unoptimize_kprobes(struct list_head *unopt_list,
 	list_for_each_entry_safe(op, tmp, free_list, list) {
 		/* Switching from detour code to origin */
 		op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
-		/* Disarm probes if marked disabled and not gone */
-		if (kprobe_disabled(&op->kp) && !kprobe_gone(&op->kp))
+		/*
+		 * Disarm probes if marked disabled and not gone. Unhashed probes
+		 * were disarmed before they were unhashed.
+		 */
+		if (kprobe_disabled(&op->kp) && !kprobe_gone(&op->kp) &&
+		    !hlist_unhashed(&op->kp.hlist))
 			arch_disarm_kprobe(&op->kp);
 		if (kprobe_unused(&op->kp)) {
 			/*

-- 
2.53.0

[PATCH 3/3] kprobes: Arm reused optprobe on cooling list

[Mani Tofigh via B4 Relay]

From: Mani Tofigh <mt4022@columbia.edu>

A new probe that reuses an unused probe still on a cooling list doesn't
fire.  reuse_unused_kprobe() clears DISABLED without arming it, so the
probe has no int3 until it's optimized again, or ever if it can't be
(e.g., it has a post_handler).

Arm a reused cooling probe.  With the fix, it fires.

Fixes: 06e0d3b89da1 ("kprobes: Make optprobe optimizer multi-generational and asynchronous")
Signed-off-by: Mani Tofigh <mt4022@columbia.edu>
---
 kernel/kprobes.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 84a5f68476eb..59c5689299ce 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -536,6 +536,7 @@ static struct optprobe_generation optprobe_gens[OPTPROBE_GEN_MAX];
 static int optprobe_cur_gen;
 
 static void optimize_kprobe(struct kprobe *p);
+static void __arm_kprobe(struct kprobe *p);
 static struct task_struct *kprobe_optimizer_task;
 static wait_queue_head_t kprobe_optimizer_wait;
 static atomic_t optimizer_state;
@@ -1067,6 +1068,13 @@ static int reuse_unused_kprobe(struct kprobe *ap)
 	if (!kprobe_optready(ap))
 		return -EINVAL;
 
+	if (optprobe_queued_cooling(op)) {
+		/* Already disarmed. It will be optimized again upon finalize */
+		if (!kprobes_all_disarmed)
+			__arm_kprobe(ap);
+		return 0;
+	}
+
 	optimize_kprobe(ap);
 	return 0;
 }

-- 
2.53.0

Re: [PATCH 3/3] kprobes: Arm reused optprobe on cooling list

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kprobes: reuse_unused_kprobe() arms cooling probe when new probe is disabled
--

commit c03ba118d217fb856e6a911233d52037e4e98250
Author: Mani Tofigh <mt4022@columbia.edu>

kprobes: Arm reused optprobe on cooling list

This commit arms an unused aggregator kprobe in reuse_unused_kprobe() when
it is still on an optprobe cooling list so that a newly registered probe
at the same address will fire before optimization finishes.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 84a5f68476eb1..59c5689299ce3 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

[ ... ]

> @@ -1067,6 +1068,13 @@ static int reuse_unused_kprobe(struct kprobe *ap)
>  	if (!kprobe_optready(ap))
>  		return -EINVAL;
>  
> +	if (optprobe_queued_cooling(op)) {
> +		/* Already disarmed. It will be optimized again upon finalize */
> +		if (!kprobes_all_disarmed)
> +			__arm_kprobe(ap);
> +		return 0;
> +	}

[Severity: High]
What happens here if the newly registered kprobe has KPROBE_FLAG_DISABLED
set, such as when __register_trace_kprobe() registers a new tracefs
kprobe_events entry before enabling it?

In __register_kprobe() -> register_aggr_kprobe(), reuse_unused_kprobe(ap)
is called before checking whether the new probe p is disabled:

kernel/kprobes.c:register_aggr_kprobe() {
	...
		} else if (kprobe_unused(ap)) {
			/* This probe is going to die. Rescue it */
			ret = reuse_unused_kprobe(ap);
			if (ret)
				return ret;
		}
	...
	if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) {
		ap->flags &= ~KPROBE_FLAG_DISABLED;
		if (!kprobes_all_disarmed) {
			/* Arm the breakpoint again. */
			ret = arm_kprobe(ap);
	...
}

Because reuse_unused_kprobe(ap) unconditionally clears KPROBE_FLAG_DISABLED
on ap and calls __arm_kprobe(ap) when op is on a cooling_list, it bypasses
the !kprobe_disabled(p) check in register_aggr_kprobe(). This arms a
breakpoint in kernel text (and later jump-optimizes ap in
optprobe_finalize_generation()) even though all child probes on ap->list
are disabled.

Furthermore, if the disabled child probe p is later unregistered without
ever being enabled, __unregister_kprobe_top() calls __disable_kprobe(p),
which sees kprobe_disabled(p) and returns early without setting
KPROBE_FLAG_DISABLED on ap or disarming it:

kernel/kprobes.c:__disable_kprobe() {
	...
	if (kprobe_disabled(p))
		return orig_p;
	...
}

Then in __unregister_kprobe_top(), kprobe_disarmed(ap) is false because
KPROBE_FLAG_DISABLED is not set on ap:

kernel/kprobes.c:__unregister_kprobe_top() {
	...
	if (ap == p ||
		(list_is_singular(&ap->list) && kprobe_disarmed(ap))) {
		/*
		 * !disarmed could be happen if the probe is under delayed
		 * unoptimizing.
		 */
		hlist_del_rcu(&ap->hlist);
		return 0;
	}
	...
}

Does this leave the aggregator struct optimized_kprobe permanently hashed
and leaked in kprobe_table with an empty ap->list and an active breakpoint
or jump instruction patched into kernel text?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-kprobes-optprobe-state-v1-0-6aaac612abe6@columbia.edu?part=3