Re: [PATCH RFC v2 04/15] 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 1 potential issue(s) to consider:
- [Medium] Lockless traversal of the RCU-protected `kprobe_table` in `kprobe_in_optimized_region()` lacks `rcu_read_lock()`, leading to lockdep splats on PREEMPT_RCU kernels.
--

commit dc16333e12fedda231a4417b135bef649425955b
Author: Josef Bacik <josef@toxicpanda.com>

kprobes: Let Tasks RCU recognise tasks preempted in an optprobe jump window

This commit adds a lockless check kprobe_in_optimized_region() to allow Tasks
RCU to recognize tasks preempted within an instruction boundary about to be
overwritten by an optimized jump kprobe. It evaluates registered kprobes 
inside the optimizer's Tasks RCU grace period.

> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 6337da5cab9e7..cf2ea278fdf5b 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c

[ ... ]

> @@ -511,6 +511,48 @@ static struct kprobe *get_optimized_kprobe(kprobe_opcode_t *addr)
>  	return NULL;
>  }
>  
> +/*
> + * True while kprobe_optimizer() is waiting for its Tasks RCU grace period.
> + * Only in that window can a preemption inside an optprobe's jump region
> + * matter to it, so kprobe_in_optimized_region() does no work otherwise.
> + */
> +static bool kprobe_optimizer_waiting;
> +
> +/**
> + * kprobe_in_optimized_region - Could @addr be inside bytes a jump-optimized
> + *	kprobe replaces?
> + * @addr: kernel text address, typically an interrupted instruction pointer
> + *
> + * kprobe_optimizer() relies on synchronize_rcu_tasks() to wait for tasks that
> + * were preempted on an instruction boundary inside the region about to be
> + * overwritten by the optimized jump; such a task must not report a Tasks RCU
> + * quiescent state when it is preempted (see rcu_tasks_ip_in_trampoline()).
> + * This is the lockless, conservative form of get_optimized_kprobe(): it does
> + * not care whether the kprobe found is, or ever will be, optimized.  May be
> + * called from any context with preemption disabled; the kprobe hash is
> + * RCU-protected and every free path waits for a grace period after unhashing.
> + *
> + * The hash walk only runs while the optimizer is actually waiting.  A
> + * preemption that does not observe kprobe_optimizer_waiting predates the
> + * grace period (its leading synchronize_rcu() publishes the store to every
> + * interrupts-disabled reader before any task is sampled as a holdout); such a
> + * task is then an ordinary preempted holdout, and the jump is not written
> + * until it has run again and left the region.
> + */
> +bool kprobe_in_optimized_region(unsigned long addr)
> +{
> +	int i;
> +
> +	if (!READ_ONCE(kprobe_optimizer_waiting))
> +		return false;
> +
> +	for (i = 1; i < MAX_OPTIMIZED_LENGTH / sizeof(kprobe_opcode_t); i++)
> +		if (get_kprobe((kprobe_opcode_t *)addr - i))

[Severity: Medium]
When kprobe_in_optimized_region() calls get_kprobe(), neither rcu_read_lock()
nor kprobe_mutex is held (for example, when called from the Tasks RCU GP
kthread).

get_kprobe() iterates the hash list using:

    hlist_for_each_entry_rcu(p, head, hlist,
                             lockdep_is_held(&kprobe_mutex)) {

Without explicitly acquiring rcu_read_lock() around get_kprobe(), lockdep
will complain about suspicious RCU usage, because preempt_disable() (or
running from a kthread) does not satisfy rcu_read_lock_held() on
CONFIG_PREEMPT_RCU.

Could this lockless traversal of kprobe_table trigger a lockdep warning on
PREEMPT_RCU kernels?

> +			return true;
> +	return false;
> +}
> +NOKPROBE_SYMBOL(kprobe_in_optimized_region);
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-b4-rcu-tasks-preempt-qs-v2-0-eaaa61ed2da4@toxicpanda.com?part=4