Re: [PATCH 09/13] x86/kprobes: Use kprobe iterator in arch_check_optimized_kprobe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] kprobes: sibling re-optimization failure in __disarm_kprobe()
  [1]
--

commit 8174a2b0aeefac5511bfa58a9428f79328aae112
Author: Jiri Olsa <jolsa@kernel.org>

x86/kprobes: Use kprobe iterator in arch_check_optimized_kprobe

This commit replaces the byte-by-byte get_kprobe() lookups in
arch_check_optimized_kprobe() with for_each_kprobe() over the optimized
instruction range.

> diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
> index 9da41424be777..cdf69c939631d 100644
> --- a/arch/x86/kernel/kprobes/opt.c
> +++ b/arch/x86/kernel/kprobes/opt.c
> @@ -334,12 +334,13 @@ static int can_optimize(unsigned long paddr)
>  /* Check optimized_kprobe can actually be optimized. */
>  int arch_check_optimized_kprobe(struct optimized_kprobe *op)
>  {
> -	int i;
> +	unsigned long start = (unsigned long)op->kp.addr;
> +	unsigned long end = start + op->optinsn.size - 1;
> +	struct kprobe_iter iter;
>  	struct kprobe *p;
>  
> -	for (i = 1; i < op->optinsn.size; i++) {
> -		p = get_kprobe(op->kp.addr + i);
> -		if (p && !kprobe_disarmed(p))
> +	for_each_kprobe(iter, p, start + 1, end) {
> +		if (!kprobe_disarmed(p))
>  			return -EEXIST;
>  	}

[Severity: Medium]
This problem wasn't introduced by this patch, but can kprobe_disarmed(p)
here reject sibling re-optimization during __disable_kprobe() [1]?

In __disable_kprobe(), disarm_kprobe(orig_p, true) runs before
KPROBE_FLAG_DISABLED is set on orig_p->flags:

kernel/kprobes.c:__disable_kprobe() {
	...
		if (!kprobes_all_disarmed && !kprobe_disabled(orig_p)) {
			ret = disarm_kprobe(orig_p, true);
			if (ret) {
				p->flags &= ~KPROBE_FLAG_DISABLED;
				return ERR_PTR(ret);
			}
		}
		orig_p->flags |= KPROBE_FLAG_DISABLED;
	...
}

When __disarm_kprobe() disarms a probe that overlapped the jump range of a
sibling optprobe, it calls optimize_kprobe() -> arch_check_optimized_kprobe()
to re-optimize that sibling.  Because orig_p is still registered and lacks
KPROBE_FLAG_DISABLED at that point, kprobe_disarmed(orig_p) returns false
and arch_check_optimized_kprobe() fails with -EEXIST, leaving the sibling
probe permanently unoptimized.

[1] https://sashiko.dev/#/bug/linux-504cacc5-65e4-4f67-bdd9-fa0c7244bee6

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109194916.1089703-1-jolsa@kernel.org?part=9