From 007fd460798be9af7d4874734f934afa47742f29 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Sun, 1 Nov 2026 19:05:09 +0000 Subject: 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 Signed-off-by: Alexei Starovoitov Link: https://patch.msgid.link/20261101190535.1454146-2-alexei.starovoitov@gmail.com --- kernel/bpf/stackmap.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'kernel/bpf/stackmap.c') 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: -- cgit v1.2.3 From a08ba2ab79dffd92c32d3f2dadfc7eba03625a2b Mon Sep 17 00:00:00 2001 From: Alexei Starovoitov Date: Sun, 1 Nov 2026 19:05:10 +0000 Subject: bpf: Clamp the trace length before the skip test in stackid_init() stackid_init() gives a bucket of the stack map min(trace_nr - skip, max_depth - skip) trace entries. Here max_depth is the size of a bucket plus skip, but at most perf_event_max_stack. Context markers can make trace_nr larger than that, so the skip test can pass when max_depth < skip. Then 'max_depth - skip' wraps, and the kernel can write after the bucket. Clamp trace_nr to max_depth before the skip test, as __bpf_get_stack_pe() does. The result changes only when skip >= max_depth. Then it is -EFAULT. Until now skip == max_depth could give the id of an empty trace. get_perf_callchain() releases the trace, so an NMI can change trace->nr. Use READ_ONCE() in bpf_get_stackid(), so the compiler reads it one time. Fixes: 23f852daa4ba ("bpf: Fix stackmap overflow check in __bpf_get_stackid()") Signed-off-by: Alexei Starovoitov Link: https://patch.msgid.link/20261101190535.1454146-3-alexei.starovoitov@gmail.com --- kernel/bpf/stackmap.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'kernel/bpf/stackmap.c') diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c index 7d90891cce0a..a19126f5db02 100644 --- a/kernel/bpf/stackmap.c +++ b/kernel/bpf/stackmap.c @@ -522,12 +522,14 @@ static int stackid_init(struct stackid *stackid, struct bpf_map *map, u32 skip = flags & BPF_F_SKIP_FIELD_MASK; u32 max_depth; + max_depth = stack_map_calculate_max_depth(map->value_size, stack_map_data_size(map), flags); + /* trace_nr can be above max_depth and max_depth below skip: clamp first */ + trace_nr = min_t(u32, trace_nr, max_depth); if (trace_nr <= skip) /* skipping more than usable stack trace */ return -EFAULT; - max_depth = stack_map_calculate_max_depth(map->value_size, stack_map_data_size(map), flags); - stackid->nr = min_t(u32, trace_nr - skip, max_depth - skip); + stackid->nr = trace_nr - skip; stackid->len = stackid->nr * sizeof(u64); stackid->ips = trace->ip + skip; stackid->hash = jhash2((const u32 *)stackid->ips, stackid->len / sizeof(u32), 0); @@ -642,7 +644,8 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map, /* couldn't fetch the stack trace */ return -EFAULT; - err = stackid_fastpath(&stackid, map, trace, trace->nr, flags); + /* the entry is released already, an NMI can change trace->nr */ + err = stackid_fastpath(&stackid, map, trace, READ_ONCE(trace->nr), flags); if (err != -ENOENT) return err; -- cgit v1.2.3