Re: [PATCH RFC 04/13] kprobes: Let Tasks RCU recognise tasks preempted in an optprobe jump window

[sashiko-bot@kernel.org]

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 <josef@toxicpanda.com>

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-d4469f4cc101@toxicpanda.com?part=4