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

[sashiko-bot@kernel.org]

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