Re: [PATCH bpf v2] bpf: guard uprobes against private-stack corruption

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for private stack corruption: BPF programs attached to both uprobes and kprobes/tracepoints bypass recursion protection.
--

--- Patch [1]: [PATCH bpf v2] bpf: guard uprobes against private-stack corruption ---
commit 91e05311e6d3a25ea1742394d64f625138f2c862
Author: Christian Simon <simon@swine.de>

bpf: guard uprobes against private-stack corruption

Eligible BPF programs use one private stack per program and CPU. This
commit adds per-program recursion protection to the uprobe execution
paths to prevent a second invocation from overwriting the first invocation's
private stack.

Closes: https://github.com/open-telemetry/opentelemetry-ebpf-instrumentation/issues/3056

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 7719f65284456..a94fc9898ecef 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h

[ ... ]

> @@ -2635,15 +2643,30 @@ bpf_prog_run_array_uprobe(const struct bpf_prog_array *array,
>  	old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
>  	item = &array->items[0];
>  	while ((prog = READ_ONCE(item->prog))) {
> +		/* dummy_bpf_prog has no recursion state. */
> +		if (unlikely(!prog->len)) {
> +			item++;
> +			continue;
> +		}
> +
> +		if (unlikely(!bpf_prog_get_recursion_context(prog))) {

[Severity: High]
This is a pre-existing issue, but does this leave the private stack vulnerable
to corruption if a program is attached to both an uprobe and a kprobe?

The uprobe path increments the per-program recursion counter here via
bpf_prog_get_recursion_context(), but it does not increment the global
bpf_prog_active counter.

If an interrupt triggers a kprobe while the uprobe is executing on the same
CPU, the kprobe dispatcher only checks the global counter:

kernel/trace/bpf_trace.c:trace_call_bpf() {
    if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
        ...
        ret = bpf_prog_run_array(..., bpf_prog_run);
    ...
}

Because the uprobe didn't increment the global bpf_prog_active counter,
trace_call_bpf() will proceed and execute the same BPF program again,
bypassing the per-program recursion protection and potentially overwriting
the first invocation's stack frames.

Does kprobe_multi_link_prog_run() also have this same vulnerability since
it relies solely on bpf_prog_active before executing bpf_prog_run()?

> +			bpf_prog_inc_misses_counter(prog);
> +			bpf_prog_put_recursion_context(prog);
> +			item++;
> +			continue;
> +		}
> +
>  		if (!prog->sleepable)
>  			rcu_read_lock();
>  
>  		run_ctx.bpf_cookie = item->bpf_cookie;
>  		ret &= run_prog(prog, ctx);
> -		item++;
>  
>  		if (!prog->sleepable)
>  			rcu_read_unlock();
> +
> +		bpf_prog_put_recursion_context(prog);
> +		item++;
>  	}
>  	bpf_reset_run_ctx(old_run_ctx);
>  	migrate_enable();

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260818203234.1142913-1-simon@swine.de?part=1