As Alexei noted, get_perf_callchain() return values may be reused
if a task is preempted after the BPF program enters migrate disable
mode. Drawing on the per-cpu design of bpf_bprintf_buffers,
per-cpu BPF callchain entry is used here.
Signed-off-by: Tao Chen <chen.dylane@linux.dev>
---
kernel/bpf/stackmap.c | 98 ++++++++++++++++++++++++++++++++-----------
1 file changed, 74 insertions(+), 24 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 94e46b7f340..3513077c57d 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -31,6 +31,52 @@ struct bpf_stack_map {
struct stack_map_bucket *buckets[] __counted_by(n_buckets);
};
+struct bpf_perf_callchain_entry {
+ u64 nr;
+ u64 ip[PERF_MAX_STACK_DEPTH];
+};
+
+#define MAX_PERF_CALLCHAIN_PREEMPT 3
+static DEFINE_PER_CPU(struct bpf_perf_callchain_entry[MAX_PERF_CALLCHAIN_PREEMPT],
+ bpf_perf_callchain_entries);
+static DEFINE_PER_CPU(int, bpf_perf_callchain_preempt_cnt);
+
+static int bpf_get_perf_callchain_or_entry(struct perf_callchain_entry **entry,
+ struct pt_regs *regs, bool kernel,
+ bool user, u32 max_stack, bool crosstack,
+ bool add_mark, bool get_callchain)
+{
+ struct bpf_perf_callchain_entry *bpf_entry;
+ struct perf_callchain_entry *perf_entry;
+ int preempt_cnt;
+
+ preempt_cnt = this_cpu_inc_return(bpf_perf_callchain_preempt_cnt);
+ if (WARN_ON_ONCE(preempt_cnt > MAX_PERF_CALLCHAIN_PREEMPT)) {
+ this_cpu_dec(bpf_perf_callchain_preempt_cnt);
+ return -EBUSY;
+ }
+
+ bpf_entry = this_cpu_ptr(&bpf_perf_callchain_entries[preempt_cnt - 1]);
+ if (!get_callchain) {
+ *entry = (struct perf_callchain_entry *)bpf_entry;
+ return 0;
+ }
+
+ perf_entry = get_perf_callchain(regs, (struct perf_callchain_entry *)bpf_entry,
+ kernel, user, max_stack,
+ crosstack, add_mark);
+ *entry = perf_entry;
+
+ return 0;
+}
+
+static void bpf_put_perf_callchain(void)
+{
+ if (WARN_ON_ONCE(this_cpu_read(bpf_perf_callchain_preempt_cnt) == 0))
+ return;
+ this_cpu_dec(bpf_perf_callchain_preempt_cnt);
+}
+
static inline bool stack_map_use_build_id(struct bpf_map *map)
{
return (map->map_flags & BPF_F_STACK_BUILD_ID);
@@ -192,11 +238,11 @@ get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
{
#ifdef CONFIG_STACKTRACE
struct perf_callchain_entry *entry;
- int rctx;
-
- entry = get_callchain_entry(&rctx);
+ int ret;
- if (!entry)
+ ret = bpf_get_perf_callchain_or_entry(&entry, NULL, false, false, 0, false, false,
+ false);
+ if (ret)
return NULL;
entry->nr = stack_trace_save_tsk(task, (unsigned long *)entry->ip,
@@ -216,7 +262,7 @@ get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
to[i] = (u64)(from[i]);
}
- put_callchain_entry(rctx);
+ bpf_put_perf_callchain();
return entry;
#else /* CONFIG_STACKTRACE */
@@ -305,6 +351,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
bool user = flags & BPF_F_USER_STACK;
struct perf_callchain_entry *trace;
bool kernel = !user;
+ int err;
if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
@@ -314,14 +361,15 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
if (max_depth > sysctl_perf_event_max_stack)
max_depth = sysctl_perf_event_max_stack;
- trace = get_perf_callchain(regs, NULL, kernel, user, max_depth,
- false, false);
+ err = bpf_get_perf_callchain_or_entry(&trace, regs, kernel, user, max_depth,
+ false, false, true);
+ if (err)
+ return err;
- if (unlikely(!trace))
- /* couldn't fetch the stack trace */
- return -EFAULT;
+ err = __bpf_get_stackid(map, trace, flags);
+ bpf_put_perf_callchain();
- return __bpf_get_stackid(map, trace, flags);
+ return err;
}
const struct bpf_func_proto bpf_get_stackid_proto = {
@@ -443,20 +491,23 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
if (sysctl_perf_event_max_stack < max_depth)
max_depth = sysctl_perf_event_max_stack;
- if (may_fault)
- rcu_read_lock(); /* need RCU for perf's callchain below */
-
if (trace_in)
trace = trace_in;
- else if (kernel && task)
+ else if (kernel && task) {
trace = get_callchain_entry_for_task(task, max_depth);
- else
- trace = get_perf_callchain(regs, NULL, kernel, user, max_depth,
- crosstask, false);
+ } else {
+ err = bpf_get_perf_callchain_or_entry(&trace, regs, kernel, user, max_depth,
+ false, false, true);
+ if (err)
+ return err;
+ }
+
+ if (unlikely(!trace))
+ goto err_fault;
- if (unlikely(!trace) || trace->nr < skip) {
- if (may_fault)
- rcu_read_unlock();
+ if (trace->nr < skip) {
+ if (!trace_in)
+ bpf_put_perf_callchain();
goto err_fault;
}
@@ -475,9 +526,8 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
memcpy(buf, ips, copy_len);
}
- /* trace/ips should not be dereferenced after this point */
- if (may_fault)
- rcu_read_unlock();
+ if (!trace_in)
+ bpf_put_perf_callchain();
if (user_build_id)
stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
--
2.48.1On Mon, Oct 20, 2025 at 01:01:18AM +0800, Tao Chen wrote: > As Alexei noted, get_perf_callchain() return values may be reused > if a task is preempted after the BPF program enters migrate disable > mode. Drawing on the per-cpu design of bpf_bprintf_buffers, > per-cpu BPF callchain entry is used here. And now you can only unwind 3 tasks, and then start failing. This is acceptable, why? > - if (may_fault) > - rcu_read_lock(); /* need RCU for perf's callchain below */ > - I know you propose to remove this code; but how was that correct? The perf callchain code hard relies on non-preemptible context, RCU does not imply such a thing. > if (trace_in) > trace = trace_in; > - else if (kernel && task) > trace = get_callchain_entry_for_task(task, max_depth); > - else > - trace = get_perf_callchain(regs, NULL, kernel, user, max_depth, > - crosstask, false);
在 2025/10/20 19:03, Peter Zijlstra 写道: > On Mon, Oct 20, 2025 at 01:01:18AM +0800, Tao Chen wrote: >> As Alexei noted, get_perf_callchain() return values may be reused >> if a task is preempted after the BPF program enters migrate disable >> mode. Drawing on the per-cpu design of bpf_bprintf_buffers, >> per-cpu BPF callchain entry is used here. > > And now you can only unwind 3 tasks, and then start failing. This is > acceptable, why? Yes it is, if we use per-cpu-bpf-callchain-entry like bpf_bprintf_buffers, this is a proposal from Andrii and Alexei, In my understanding, is it a low-probability event to be preempted three times in a row in the same cpu? > >> - if (may_fault) >> - rcu_read_lock(); /* need RCU for perf's callchain below */ >> - > > I know you propose to remove this code; but how was that correct? The > perf callchain code hard relies on non-preemptible context, RCU does not > imply such a thing. > Alexei mentioned this rcu-lock issue before， It seems we need preemption protection. https://lore.kernel.org/bpf/CAADnVQ+s8B7-fvR1TNO-bniSyKv57cH_ihRszmZV7pQDyV=VDQ@mail.gmail.com >> if (trace_in) >> trace = trace_in; >> - else if (kernel && task) >> trace = get_callchain_entry_for_task(task, max_depth); >> - else >> - trace = get_perf_callchain(regs, NULL, kernel, user, max_depth, >> - crosstask, false); > > -- Best Regards Tao Chen
在 2025/10/20 01:01, Tao Chen 写道:
> As Alexei noted, get_perf_callchain() return values may be reused
> if a task is preempted after the BPF program enters migrate disable
> mode. Drawing on the per-cpu design of bpf_bprintf_buffers,
> per-cpu BPF callchain entry is used here.
>
> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> ---
> kernel/bpf/stackmap.c | 98 ++++++++++++++++++++++++++++++++-----------
> 1 file changed, 74 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 94e46b7f340..3513077c57d 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -31,6 +31,52 @@ struct bpf_stack_map {
> struct stack_map_bucket *buckets[] __counted_by(n_buckets);
> };
>
> +struct bpf_perf_callchain_entry {
> + u64 nr;
> + u64 ip[PERF_MAX_STACK_DEPTH];
> +};
> +
> +#define MAX_PERF_CALLCHAIN_PREEMPT 3
> +static DEFINE_PER_CPU(struct bpf_perf_callchain_entry[MAX_PERF_CALLCHAIN_PREEMPT],
> + bpf_perf_callchain_entries);
> +static DEFINE_PER_CPU(int, bpf_perf_callchain_preempt_cnt);
> +
> +static int bpf_get_perf_callchain_or_entry(struct perf_callchain_entry **entry,
> + struct pt_regs *regs, bool kernel,
> + bool user, u32 max_stack, bool crosstack,
> + bool add_mark, bool get_callchain)
> +{
> + struct bpf_perf_callchain_entry *bpf_entry;
> + struct perf_callchain_entry *perf_entry;
> + int preempt_cnt;
> +
> + preempt_cnt = this_cpu_inc_return(bpf_perf_callchain_preempt_cnt);
> + if (WARN_ON_ONCE(preempt_cnt > MAX_PERF_CALLCHAIN_PREEMPT)) {
> + this_cpu_dec(bpf_perf_callchain_preempt_cnt);
> + return -EBUSY;
> + }
> +
> + bpf_entry = this_cpu_ptr(&bpf_perf_callchain_entries[preempt_cnt - 1]);
> + if (!get_callchain) {
> + *entry = (struct perf_callchain_entry *)bpf_entry;
> + return 0;
> + }
> +
> + perf_entry = get_perf_callchain(regs, (struct perf_callchain_entry *)bpf_entry,
> + kernel, user, max_stack,
> + crosstack, add_mark);
> + *entry = perf_entry;
> +
> + return 0;
> +}
> +
> +static void bpf_put_perf_callchain(void)
> +{
> + if (WARN_ON_ONCE(this_cpu_read(bpf_perf_callchain_preempt_cnt) == 0))
> + return;
> + this_cpu_dec(bpf_perf_callchain_preempt_cnt);
> +}
> +
> static inline bool stack_map_use_build_id(struct bpf_map *map)
> {
> return (map->map_flags & BPF_F_STACK_BUILD_ID);
> @@ -192,11 +238,11 @@ get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
> {
> #ifdef CONFIG_STACKTRACE
> struct perf_callchain_entry *entry;
> - int rctx;
> -
> - entry = get_callchain_entry(&rctx);
> + int ret;
>
> - if (!entry)
> + ret = bpf_get_perf_callchain_or_entry(&entry, NULL, false, false, 0, false, false,
> + false);
> + if (ret)
> return NULL;
>
> entry->nr = stack_trace_save_tsk(task, (unsigned long *)entry->ip,
> @@ -216,7 +262,7 @@ get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
> to[i] = (u64)(from[i]);
> }
>
> - put_callchain_entry(rctx);
> + bpf_put_perf_callchain();
double-put issue here reported by AI, i will fix it.
>
> return entry;
> #else /* CONFIG_STACKTRACE */
> @@ -305,6 +351,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
> bool user = flags & BPF_F_USER_STACK;
> struct perf_callchain_entry *trace;
> bool kernel = !user;
> + int err;
>
> if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
> BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
> @@ -314,14 +361,15 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
> if (max_depth > sysctl_perf_event_max_stack)
> max_depth = sysctl_perf_event_max_stack;
>
> - trace = get_perf_callchain(regs, NULL, kernel, user, max_depth,
> - false, false);
> + err = bpf_get_perf_callchain_or_entry(&trace, regs, kernel, user, max_depth,
> + false, false, true);
> + if (err)
> + return err;
>
> - if (unlikely(!trace))
> - /* couldn't fetch the stack trace */
> - return -EFAULT;
> + err = __bpf_get_stackid(map, trace, flags);
> + bpf_put_perf_callchain();
>
> - return __bpf_get_stackid(map, trace, flags);
> + return err;
> }
>
> const struct bpf_func_proto bpf_get_stackid_proto = {
> @@ -443,20 +491,23 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
> if (sysctl_perf_event_max_stack < max_depth)
> max_depth = sysctl_perf_event_max_stack;
>
> - if (may_fault)
> - rcu_read_lock(); /* need RCU for perf's callchain below */
> -
> if (trace_in)
> trace = trace_in;
> - else if (kernel && task)
> + else if (kernel && task) {
> trace = get_callchain_entry_for_task(task, max_depth);
> - else
> - trace = get_perf_callchain(regs, NULL, kernel, user, max_depth,
> - crosstask, false);
> + } else {
> + err = bpf_get_perf_callchain_or_entry(&trace, regs, kernel, user, max_depth,
> + false, false, true);
> + if (err)
> + return err;
> + }
> +
> + if (unlikely(!trace))
> + goto err_fault;
>
> - if (unlikely(!trace) || trace->nr < skip) {
> - if (may_fault)
> - rcu_read_unlock();
> + if (trace->nr < skip) {
> + if (!trace_in)
> + bpf_put_perf_callchain();
> goto err_fault;
> }
>
> @@ -475,9 +526,8 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
> memcpy(buf, ips, copy_len);
> }
>
> - /* trace/ips should not be dereferenced after this point */
> - if (may_fault)
> - rcu_read_unlock();
> + if (!trace_in)
> + bpf_put_perf_callchain();
>
> if (user_build_id)
> stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
--
Best Regards
Tao Chen
© 2016 - 2026 Red Hat, Inc.