History

bpf: Disable preemption in __bpf_get_stack

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. Fixes: c195651e565a ("bpf: add bpf_get_stack helper") Reported-by: Tao Chen <chen.dylane@linux.dev> Reported-by: STAR Labs SG <info@starlabs.sg> Signed-off-by: Daniel Borkmann <borkmann@iogearbox.net> Signed-off-by: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/bpf/20260803210149.296496-11-jolsa@kernel.org

Daniel Borkmann committed 3 months ago

Commit b1a47b21 parent f5d24281 file · +3 −0Browse files

Showing the changes to kernel (1 of 1 file). Show all

Unified · Split

kernel/bpf/stackmap.c+3−0
@@ -815,6 +815,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task, · 814 unmodified lines
815 815
816 max_depth = stack_map_calculate_max_depth(size, elem_size, flags);816 max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
817 817
818 preempt_disable();
818 if (may_fault)819 if (may_fault)
819 rcu_read_lock(); /* need RCU for perf's callchain below */820 rcu_read_lock(); /* need RCU for perf's callchain below */
820 821
@@ -828,6 +829,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task, · 7 unmodified lines
828 if (unlikely(!trace) || trace->nr < skip) {829 if (unlikely(!trace) || trace->nr < skip) {
829 if (may_fault)830 if (may_fault)
830 rcu_read_unlock();831 rcu_read_unlock();
832 preempt_enable();
831 goto err_fault;833 goto err_fault;
832 }834 }
833 835
@@ -836,6 +838,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task, · 2 unmodified lines
836 /* trace should not be dereferenced after this point */838 /* trace should not be dereferenced after this point */
837 if (may_fault)839 if (may_fault)
838 rcu_read_unlock();840 rcu_read_unlock();
841 preempt_enable();
839 842
840 return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);843 return callchain_finalize(buf, size, trace_nr, elem_size, flags, may_fault);
841 844

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t