Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Massive performance overhead in the kernel preemption fast-path due to unbounded hash table lookups. - [High] kprobe_optimizer() frees kprobes without synchronize_rcu(), causing a Use-After-Free when get_kprobe() is called from irqentry_preempt() on idle CPUs. --
commit c6c38cd5a975e6f9c2037bf63fa6d732d6cd1fab Author: Josef Bacik <[email protected]> kprobes: Let Tasks RCU recognise tasks preempted in an optprobe jump window This patch adds kprobe_in_optimized_region() to determine if an instruction pointer falls within the bytes of an optimized kprobe. It then updates rcu_tasks_ip_in_trampoline() to use this check, allowing Tasks RCU to recognize and hold off grace periods for tasks preempted within these jump windows. > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c [ ... ] > +bool kprobe_in_optimized_region(unsigned long addr) > +{ > + int i; > + > + for (i = 1; i < MAX_OPTIMIZED_LENGTH / sizeof(kprobe_opcode_t); i++) > + if (get_kprobe((kprobe_opcode_t *)addr - i)) [Severity: High] Could this loop introduce significant overhead in the preemption fast-path? On architectures like x86, this loop executes up to 18 times, querying the kprobe hash table via get_kprobe() on each iteration. Since the hash table can have long chains on systems with many active kprobes, this could result in thousands of pointer dereferences during a preemption event. Also, does this introduce a use-after-free risk for interrupted idle tasks? kprobe_optimizer() unlinks kprobes and frees them after waiting only for synchronize_rcu_tasks(). Because synchronize_rcu_tasks() explicitly ignores idle tasks, an idle CPU that is interrupted could end up traversing the kprobe_table here via get_kprobe() while the kprobe is concurrently freed, as Tasks RCU will not wait for the idle task's traversal to finish. > + return true; > + return false; > +} [ ... ] > diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h > --- a/kernel/rcu/tasks.h > +++ b/kernel/rcu/tasks.h [ ... ] > bool rcu_tasks_ip_in_trampoline(unsigned long ip) > { > + if (kprobe_in_optimized_region(ip)) [Severity: High] Is it safe to place this expensive check unconditionally at the start of this function? When CONFIG_RCU_TASKS_PREEMPT_QS is enabled, rcu_tasks_ip_in_trampoline() is called on every kernel preemption via irqentry_preempt(). By placing kprobe_in_optimized_region() before cheaper checks like core_kernel_text(ip), every preemption event incurs the cost of the loop and hash table lookups, which could severely degrade system performance. > + return true; > + > if (core_kernel_text(ip)) > return arch_rcu_tasks_ip_in_trampoline(ip); > return !is_module_text_address(ip); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260910-b4-rcu-tasks-preempt-qs-v1-0-d4469f4cc...@toxicpanda.com?part=4
