Re: [PATCHv4 bpf-next 10/12] bpf: Disable preemption in __bpf_get_stack

On Wed, Aug 5, 2026 at 2:30 AM Jiri Olsa <[email protected]> wrote:
>
> From: Daniel Borkmann <[email protected]>
>
> get_perf_callchain() returns a per-CPU perf_callchain_entry buffer and
> releases its recursion slot via put_callchain_entry() before returning,
> so nothing keeps the entry reserved while __bpf_get_stack() consumes
> it below.
>
> A preemptible BPF program (e.g. a non-sleepable raw tracepoint program
> on a PREEMPT kernel, which runs under migrate_disable() but not
> preempt_disable()) can be scheduled out between obtaining the entry
> and the copy. Another task scheduled on the same CPU then reuses the
> same per-CPU buffer and overwrites trace->nr with a larger value.
> copy_len is then computed from the inflated trace->nr and can exceed
> the caller's buffer, causing an out-of-bounds write in the memcpy()
> and in the build_id path.
>
> The rcu_read_lock() taken here alone does not prevent this. It is
> only taken on the may_fault path, and under CONFIG_PREEMPT_RCU it does
> not disable preemption; it merely keeps perf's callchain buffer array
> alive (freed via call_rcu()) and does nothing to stop another task
> from reusing the entry.
>
> Disable preemption around obtaining the callchain entry and copying
> it into the caller's buffer, so the entry cannot be reused underneath
> us and trace->nr stays bounded by max_depth. Build ID resolution may
> fault and is therefore deferred until after preemption is re-enabled;
> by then the instruction pointers have already been copied into buf,
> so it operates only on that private copy. Note, preempt_disable() also
> subsumes the buffer-lifetime guarantee the rcu_read_lock() provided,
> since a preempt-disabled section is an RCU read-side critical section
> for the callchain buffers' call_rcu() reclaim.
>
> Cc: [email protected]
> Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
> Reported-by: Tao Chen <[email protected]>
> Closes: https://lore.kernel.org/bpf/[email protected]/
> Reported-by: STAR Labs SG <[email protected]>
> Signed-off-by: Daniel Borkmann <[email protected]>
> [ changed Fixes: commit ]
> Signed-off-by: Jiri Olsa <[email protected]>
> ---
>  kernel/bpf/stackmap.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index eabeaef31b63..789fe35b893a 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -817,6 +817,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>
>         if (may_fault)
>                 rcu_read_lock(); /* need RCU for perf's callchain below */
> +       preempt_disable();

nit: asymmetrical to preempt_enable, I'll move it to before
rcu_read_lock, so we can have proper nesting



>
>         if (kernel && task) {
>                 trace = get_callchain_entry_for_task(task, max_depth);
> @@ -828,6 +829,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>         if (unlikely(!trace) || trace->nr < skip) {
>                 if (may_fault)
>                         rcu_read_unlock();
> +               preempt_enable();
>                 goto err_fault;
>         }
>
> @@ -836,6 +838,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>         /* trace should not be dereferenced after this point */
>         if (may_fault)
>                 rcu_read_unlock();
> +       preempt_enable();
>
>         return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);
>
> --
> 2.54.0
>