diff options
| author | Josef Bacik <josef@toxicpanda.com> | 2026-11-01 19:05:09 +0000 |
|---|---|---|
| committer | Alexei Starovoitov <ast@kernel.org> | 2026-11-02 00:10:39 +0000 |
| commit | 007fd460798be9af7d4874734f934afa47742f29 (patch) | |
| tree | 01b2436ab16883d1058faff525fa7dfd3dcbb540 /kernel/bpf/stackmap.c | |
| parent | 1d55411e8b677c67de7a20618d95dc114796de26 (diff) | |
bpf: Clamp the trace length in __bpf_get_stack() again
get_perf_callchain() and get_callchain_entry_for_task() store a trace in a
per-CPU entry, one per context. They release the entry before they return
it. Thus a second user in the same context can write to the entry while the
first user reads it. preempt_disable() stops only other tasks.
__bpf_get_stack() requests at most max_depth frames. This is skip plus the
frames that fit in the buffer, but not more than perf_event_max_stack.
Since the commit below, it copies 'trace->nr - skip' frames with no limit.
But a second user can store a longer trace. The unwinders of PowerPC add
context markers that max_depth does not limit. In both cases the copy
writes after the end of the buffer.
Read trace->nr one time. Clamp the value to max_depth before the skip test,
as __bpf_get_stack_pe() does. Thus when max_depth is less than skip, the
helper returns -EFAULT, and 'trace_nr - skip' does not underflow. The
copied frames can still be those of the second user.
Fixes: e17d62fedd10 ("bpf: Refactor stack map trace depth calculation into helper function")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261101190535.1454146-2-alexei.starovoitov@gmail.com
Diffstat (limited to 'kernel/bpf/stackmap.c')
| -rw-r--r-- | kernel/bpf/stackmap.c | 24 |
1 files changed, 17 insertions, 7 deletions
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c index f7e8d766d128..7d90891cce0a 100644 --- a/kernel/bpf/stackmap.c +++ b/kernel/bpf/stackmap.c @@ -822,14 +822,20 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task, crosstask, false, 0); } - if (unlikely(!trace) || trace->nr < skip) { - if (may_fault) - rcu_read_unlock(); - preempt_enable(); - goto err_fault; - } + if (unlikely(!trace)) + goto err_unlock; + + /* + * trace->nr can be above max_depth. The per-CPU entry is released + * already: a tracing program or an NMI which takes a callchain in the + * same context before the copy below reuses it. And the unwinders of + * PowerPC store context markers on top of max_depth. + */ + trace_nr = min_t(u32, READ_ONCE(trace->nr), max_depth); + if (trace_nr < skip) + goto err_unlock; - trace_nr = callchain_store(trace, trace->nr, buf, elem_size, flags); + trace_nr = callchain_store(trace, trace_nr, buf, elem_size, flags); /* trace should not be dereferenced after this point */ if (may_fault) @@ -838,6 +844,10 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task, return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault); +err_unlock: + if (may_fault) + rcu_read_unlock(); + preempt_enable(); err_fault: err = -EFAULT; clear: |