Re: [RESEND PATCH v2] kprobes: Make optprobe optimizer multi-generational and asynchronous

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kprobes: 100% CPU busy-loop livelock when flushing optimizer
- [High] kprobes: memory leak of optinsn slots during async unoptimization
--

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

kprobes: Make optprobe optimizer multi-generational and asynchronous

This patch decouples kprobe jump optimization from synchronous waiting by
moving to an asynchronous, multi-generational model using call_rcu_tasks().

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 4edd8ca5c6578..377fa9bfa0291 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
[ ... ]
> @@ -588,17 +653,25 @@ static void do_unoptimize_kprobes(void)
[ ... ]
> -/* Reclaim all kprobes on the 'freeing_list' */
> -static void do_free_cleaned_kprobes(void)
> +/* Reclaim all kprobes on the 'free_list' */
> +static void do_free_cleaned_kprobes(struct list_head *free_list)
>  {
>  	struct optimized_kprobe *op, *tmp;
>  
> -	list_for_each_entry_safe(op, tmp, &freeing_list, list) {
> +	list_for_each_entry_safe(op, tmp, free_list, list) {
> +		struct kprobe *_p;
> +
>  		list_del_init(&op->list);
[ ... ]
> -		struct kprobe *_p = get_optimized_kprobe(op->kp.addr);
> +		_p = get_optimized_kprobe(op->kp.addr);
>  		if (unlikely(_p))
>  			optimize_kprobe(_p);
>  
> -		free_aggr_kprobe(&op->kp);
> +		free_aggr_kprobe(&op->kp);
>  	}
>  }

[Severity: High]
Does this leak the dynamically allocated optinsn memory slots?

Later in the patch, kill_optimized_kprobe() is modified to intentionally
defer calling arch_remove_optimized_kprobe(op) if the probe is queued,
leaving the optimizer to reclaim it after the quiescence period.

However, when do_free_cleaned_kprobes() runs to reclaim the probe after the
grace period, free_aggr_kprobe() only frees the kprobe struct. Without a
call to arch_remove_optimized_kprobe(op) here, the executable memory slots
allocated for the optimized kprobes appear to be permanently leaked.

[ ... ]
> @@ -624,67 +696,138 @@ static void do_free_cleaned_kprobes(void)
[ ... ]
> +/* Kprobe jump optimizer */
> +static void kprobe_optimizer(void)
> +{
[ ... ]
> +	/* Step 3: Wake up flushers and kick optimizer again if needed */
>  	optimizer_passes++;
>  	wake_up_var_locked(&optimizer_passes, &kprobe_mutex);

[Severity: High]
Does unconditionally incrementing optimizer_passes here cause a 100% CPU
busy-loop livelock?

When wait_for_kprobe_optimizer_locked() is called while a Tasks RCU grace
period is in flight, it sees optprobe_optimizer_busy() is true and wakes the
optimizer thread.

The optimizer thread wakes up, clears its flushing state, and calls
kprobe_optimizer(). Since the RCU callback hasn't fired yet, it makes no
progress but unconditionally increments optimizer_passes here.

This wakes wait_for_kprobe_optimizer_locked(), which loops, wakes the
optimizer again, and waits for another optimizer_passes change. Both threads
could spin in this tight loop until the Tasks RCU grace period completes.

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