Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] kprobes: double free/list corruption via double hlist_del_rcu() on freeing_list --
commit 713397faac31184a877c512a3215f1d8fc48a2f8 Author: Masami Hiramatsu (Google) <[email protected]> Subject: kprobes: Make optprobe optimizer multi-generational and asynchronous This commit replaces synchronize_rcu_tasks() with call_rcu_tasks() by introducing a multi-generational ring buffer. The kprobe optimizer now unoptimizes probes and defers their cleanup asynchronously. > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 4edd8ca5c6578..33523e9beb884 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c [ ... ] > @@ -624,67 +696,145 @@ static void do_free_cleaned_kprobes(void) > > static void kick_kprobe_optimizer(void); > > -/* Kprobe jump optimizer */ > -static void kprobe_optimizer(void) > +static void optprobe_generation_rcu_cb(struct rcu_head *rcu) > { > - guard(mutex)(&kprobe_mutex); > + struct optprobe_generation *gen; > + > + gen = container_of(rcu, struct optprobe_generation, rcu); > + WRITE_ONCE(gen->ready, true); > + wake_up(&kprobe_optimizer_wait); > +} > + > +static void optprobe_finalize_generation(struct optprobe_generation *gen) > +{ > + struct optimized_kprobe *op, *tmp; > + > + lockdep_assert_held(&kprobe_mutex); > > scoped_guard(cpus_read_lock) { > guard(mutex)(&text_mutex); > > - /* > - * Step 1: Unoptimize kprobes and collect cleaned (unused and > disarmed) > - * kprobes before waiting for quiesence period. > - */ > - do_unoptimize_kprobes(); > + /* Optimization never be done when disarmed */ > + if (!kprobes_all_disarmed && kprobes_allow_optimization && > + !list_empty(&gen->optimizing_list)) > + arch_optimize_kprobes(&gen->optimizing_list); > + } > + > + /* Free cleaned kprobes after quiescence period */ > + do_free_cleaned_kprobes(&gen->freeing_list); > + > + /* Finalize unoptimized kprobes whose quiescence period completed */ > + list_for_each_entry_safe(op, tmp, &gen->cooling_list, list) { > + if (kprobe_unused(&op->kp)) { > + /* > + * Unregistered while quiescence period was in flight. > + * Remove from hash list and move to cur_gen's freeing > list. > + */ > + hlist_del_rcu(&op->kp.hlist); > + list_move(&op->list, > &optprobe_gens[optprobe_cur_gen].freeing_list); [Severity: High] Does this code risk list corruption via a double free? If a kprobe in the cooling_list becomes unused and is unhashed here, it is moved to the current generation's freeing_list. When this current generation is later dispatched, do_unoptimize_kprobes() will iterate over its freeing_list (passed as free_list): kernel/kprobes.c:do_unoptimize_kprobes() { ... list_for_each_entry_safe(op, tmp, free_list, list) { ... if (kprobe_unused(&op->kp)) { hlist_del_rcu(&op->kp.hlist); } ... } Will this unconditionally call hlist_del_rcu() a second time on the same probe, causing a regression by dereferencing LIST_POISON2? > + kick_kprobe_optimizer(); > + } else { > + /* Still in use; now safely disarmed */ > + list_del_init(&op->list); > + if (!kprobe_disabled(&op->kp)) > + optimize_kprobe(&op->kp); > + } > + } > + > + gen->in_flight = false; > + WRITE_ONCE(gen->ready, false); > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/179077186716.424323.12884899073547396948.stgit@devnote2?part=1
