Re: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Sleepable BPF programs calling bpf_get_stack() or bpf_get_task_stack() can corrupt per-CPU perf callchain buffers and trigger preemption warnings.
--

--- Patch [7]: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack ---
commit a181c39a42a0e8c840b9a2a09b02e1efa512170b
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor callchain_finalize function from __bpf_get_stack
    
The new callchain_finalize function calls the build-id retrieval
(if needed) and zeroes the buffer.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index b990643f25cd8..eea7b781300ca 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -827,18 +840,12 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>  	}
>  
>  	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
> -	copy_len = trace_nr * elem_size;
>  
>  	/* trace should not be dereferenced after this point */
>  	if (may_fault)
>  		rcu_read_unlock();

[Severity: High]
This is a pre-existing issue, but can sleepable BPF programs calling this
function corrupt per-CPU perf callchain buffers and trigger preemption
warnings?

When may_fault is true (such as from bpf_get_stack_sleepable or
bpf_get_task_stack_sleepable), this code relies on rcu_read_lock() earlier
in the function. However, on PREEMPT_RCU kernels, rcu_read_lock() does not
disable preemption.

The earlier call to get_perf_callchain() eventually reaches
get_callchain_entry(), which relies on smp_processor_id() and requires
preemption to be explicitly disabled:

kernel/events/callchain.c:get_callchain_entry() {
    ...
    cpu = smp_processor_id();
    return (((void *)entries->cpu_entries[cpu]) +
            (*rctx * perf_callchain_entry__sizeof()));
}

Since preemption is not disabled during this sequence, can the task be
preempted during or after fetching the per-CPU buffer, allowing another BPF
program on the same CPU to overwrite the buffer before it is copied by
callchain_store()?

> -	if (user_build_id)
> -		stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
> -
> -	if (size > copy_len)
> -		memset(buf + copy_len, 0, size - copy_len);
> -	return copy_len;
> +	return callchain_finalize(buf, size, trace_nr, elem_size, user_build_id, user, may_fault);
>  
>  err_fault:

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