[PATCH bpf-next 0/9] bpf: Disable preemption in stack map code

[PATCH bpf-next 0/9] bpf: Disable preemption in stack map code

[Jiri Olsa]

hi,
we need to disable preemption for get_perf_callchain and keep it disabled
as long as we are accessing its returned trace entries buffer.

This patchset refactors both bpf_get_stack and bpf_get_stackid helpers as
suggested by Andrii [1] before applying the actual preemption fix.

Note the initial fix was sent by Tao Chen [2], but there was no follow up
on this since February, hence this post.

thanks,
jirka


[1] https://lore.kernel.org/bpf/CAEf4BzZwvAUgLwz-M0Y_NJLTmedyY9U6s7LrSmn751hQdTP4Uw@mail.gmail.com/
[2] https://lore.kernel.org/bpf/20260206090653.1336687-1-chen.dylane@linux.dev/
---
Daniel Borkmann (1):
      bpf: Disable preemption in __bpf_get_stack

Jiri Olsa (8):
      bpf: Factor stackid_init function from __bpf_get_stackid
      bpf: Factor stackid_fastpath function from __bpf_get_stackid
      bpf: Factor stackid_new_bucket from __bpf_get_stackid
      bpf: Use stack id functions instead of __bpf_get_stackid
      bpf: Disable preemption in bpf_get_stackid
      bpf: Factor callchain_store function from __bpf_get_stack
      bpf: Factor callchain_finalize function from __bpf_get_stack
      bpf: Remove trace_in argument from __bpf_get_stack

 kernel/bpf/stackmap.c | 292 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------
 1 file changed, 198 insertions(+), 94 deletions(-)

[PATCH bpf-next 1/9] bpf: Factor stackid_init function from __bpf_get_stackid

[Jiri Olsa]

The new stackid_init function stores all the necessary bits for stackid
trace and it will be used by other functions in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 95 +++++++++++++++++++++++++++----------------
 1 file changed, 59 insertions(+), 36 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 41fe87d7302f..0eafe55b1828 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -504,33 +504,54 @@ get_callchain_entry_for_task(struct task_struct *task, u32 max_depth)
 #endif
 }
 
-static long __bpf_get_stackid(struct bpf_map *map,
-			      struct perf_callchain_entry *trace, u64 flags)
+struct stackid {
+	struct stack_map_bucket *bucket;
+	u64 *ips;
+	u32  nr;
+	u32  len;
+	u32  hash;
+	u32  id;
+};
+
+static int stackid_init(struct stackid *stackid, struct bpf_map *map,
+			struct perf_callchain_entry *trace, u64 flags)
 {
 	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
-	struct stack_map_bucket *bucket, *new_bucket, *old_bucket;
-	u32 hash, id, trace_nr, trace_len, i, max_depth;
 	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
-	bool user = flags & BPF_F_USER_STACK;
-	u64 *ips;
-	bool hash_matches;
+	u32 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);
-	trace_nr = min_t(u32, trace->nr - skip, max_depth - skip);
-	trace_len = trace_nr * sizeof(u64);
-	ips = trace->ip + skip;
-	hash = jhash2((u32 *)ips, trace_len / sizeof(u32), 0);
-	id = hash & (smap->n_buckets - 1);
-	bucket = READ_ONCE(smap->buckets[id]);
+	stackid->nr = min_t(u32, trace->nr - skip, max_depth - skip);
+	stackid->len = stackid->nr * sizeof(u64);
+	stackid->ips = trace->ip + skip;
+	stackid->hash = jhash2((u32 *)stackid->ips, stackid->len / sizeof(u32), 0);
+	stackid->id = stackid->hash & (smap->n_buckets - 1);
+	stackid->bucket = READ_ONCE(smap->buckets[stackid->id]);
+	return 0;
+}
 
-	hash_matches = bucket && bucket->hash == hash;
+static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
+			      struct perf_callchain_entry *trace, u64 flags)
+{
+	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
+	struct stack_map_bucket *new_bucket, *old_bucket;
+	bool user = flags & BPF_F_USER_STACK;
+	bool hash_matches;
+	u32 trace_len, i;
+	int err;
+
+	err = stackid_init(stackid, map, trace, flags);
+	if (err)
+		return err;
+
+	hash_matches = stackid->bucket && stackid->bucket->hash == stackid->hash;
 	/* fast cmp */
 	if (hash_matches && flags & BPF_F_FAST_STACK_CMP)
-		return id;
+		return stackid->id;
 
 	if (stack_map_use_build_id(map)) {
 		struct bpf_stack_build_id *id_offs;
@@ -540,42 +561,42 @@ static long __bpf_get_stackid(struct bpf_map *map,
 			pcpu_freelist_pop(&smap->freelist);
 		if (unlikely(!new_bucket))
 			return -ENOMEM;
-		new_bucket->nr = trace_nr;
+		new_bucket->nr = stackid->nr;
 		id_offs = (struct bpf_stack_build_id *)new_bucket->data;
-		for (i = 0; i < trace_nr; i++)
-			id_offs[i].ip = ips[i];
-		stack_map_get_build_id_offset(id_offs, trace_nr, user, false /* !may_fault */);
-		trace_len = trace_nr * sizeof(struct bpf_stack_build_id);
-		if (hash_matches && bucket->nr == trace_nr &&
-		    memcmp(bucket->data, new_bucket->data, trace_len) == 0) {
+		for (i = 0; i < stackid->nr; i++)
+			id_offs[i].ip = stackid->ips[i];
+		stack_map_get_build_id_offset(id_offs, stackid->nr, user, false /* !may_fault */);
+		trace_len = stackid->nr * sizeof(struct bpf_stack_build_id);
+		if (hash_matches && stackid->bucket->nr == stackid->nr &&
+		    memcmp(stackid->bucket->data, new_bucket->data, trace_len) == 0) {
 			pcpu_freelist_push(&smap->freelist, &new_bucket->fnode);
-			return id;
+			return stackid->id;
 		}
-		if (bucket && !(flags & BPF_F_REUSE_STACKID)) {
+		if (stackid->bucket && !(flags & BPF_F_REUSE_STACKID)) {
 			pcpu_freelist_push(&smap->freelist, &new_bucket->fnode);
 			return -EEXIST;
 		}
 	} else {
-		if (hash_matches && bucket->nr == trace_nr &&
-		    memcmp(bucket->data, ips, trace_len) == 0)
-			return id;
-		if (bucket && !(flags & BPF_F_REUSE_STACKID))
+		if (hash_matches && stackid->bucket->nr == stackid->nr &&
+		    memcmp(stackid->bucket->data, stackid->ips, stackid->len) == 0)
+			return stackid->id;
+		if (stackid->bucket && !(flags & BPF_F_REUSE_STACKID))
 			return -EEXIST;
 
 		new_bucket = (struct stack_map_bucket *)
 			pcpu_freelist_pop(&smap->freelist);
 		if (unlikely(!new_bucket))
 			return -ENOMEM;
-		memcpy(new_bucket->data, ips, trace_len);
+		memcpy(new_bucket->data, stackid->ips, stackid->len);
 	}
 
-	new_bucket->hash = hash;
-	new_bucket->nr = trace_nr;
+	new_bucket->hash = stackid->hash;
+	new_bucket->nr = stackid->nr;
 
-	old_bucket = xchg(&smap->buckets[id], new_bucket);
+	old_bucket = xchg(&smap->buckets[stackid->id], new_bucket);
 	if (old_bucket)
 		pcpu_freelist_push(&smap->freelist, &old_bucket->fnode);
-	return id;
+	return stackid->id;
 }
 
 BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
@@ -584,6 +605,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 	u32 elem_size = stack_map_data_size(map);
 	bool user = flags & BPF_F_USER_STACK;
 	struct perf_callchain_entry *trace;
+	struct stackid stackid;
 	bool kernel = !user;
 	u32 max_depth;
 
@@ -599,7 +621,7 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 		/* couldn't fetch the stack trace */
 		return -EFAULT;
 
-	return __bpf_get_stackid(map, trace, flags);
+	return __bpf_get_stackid(&stackid, map, trace, flags);
 }
 
 const struct bpf_func_proto bpf_get_stackid_proto = {
@@ -628,6 +650,7 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 {
 	struct perf_event *event = ctx->event;
 	struct perf_callchain_entry *trace;
+	struct stackid stackid;
 	bool kernel, user;
 	__u64 nr_kernel;
 	int ret;
@@ -653,7 +676,7 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 
 	if (kernel) {
 		trace->nr = nr_kernel;
-		ret = __bpf_get_stackid(map, trace, flags);
+		ret = __bpf_get_stackid(&stackid, map, trace, flags);
 	} else { /* user */
 		u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
 
@@ -662,7 +685,7 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 			return -EFAULT;
 
 		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
-		ret = __bpf_get_stackid(map, trace, flags);
+		ret = __bpf_get_stackid(&stackid, map, trace, flags);
 	}
 
 	/* restore nr */
-- 
2.54.0

[PATCH bpf-next 2/9] bpf: Factor stackid_fastpath function from __bpf_get_stackid

[Jiri Olsa]

The new stackid_fastpath does the fast stack hash and trace check, that
does not need new bucket allocation. It covers both just-ip and buildid
code paths.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 0eafe55b1828..7bc2a966e3e8 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -511,6 +511,7 @@ struct stackid {
 	u32  len;
 	u32  hash;
 	u32  id;
+	bool hash_matches;
 };
 
 static int stackid_init(struct stackid *stackid, struct bpf_map *map,
@@ -531,28 +532,46 @@ static int stackid_init(struct stackid *stackid, struct bpf_map *map,
 	stackid->hash = jhash2((u32 *)stackid->ips, stackid->len / sizeof(u32), 0);
 	stackid->id = stackid->hash & (smap->n_buckets - 1);
 	stackid->bucket = READ_ONCE(smap->buckets[stackid->id]);
+	stackid->hash_matches = stackid->bucket && stackid->bucket->hash == stackid->hash;
 	return 0;
 }
 
+static int stackid_fastpath(struct stackid *stackid, struct bpf_map *map,
+			    struct perf_callchain_entry *trace, u64 flags)
+{
+	int err;
+
+	err = stackid_init(stackid, map, trace, flags);
+	if (err)
+		return err;
+
+	/* fast cmp */
+	if (stackid->hash_matches && flags & BPF_F_FAST_STACK_CMP)
+		return stackid->id;
+
+	if (stack_map_use_build_id(map))
+		return -ENOENT;
+	if (stackid->hash_matches && stackid->bucket->nr == stackid->nr &&
+	    memcmp(stackid->bucket->data, stackid->ips, stackid->len) == 0)
+		return stackid->id;
+	if (stackid->bucket && !(flags & BPF_F_REUSE_STACKID))
+		return -EEXIST;
+	return -ENOENT;
+}
+
 static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
 			      struct perf_callchain_entry *trace, u64 flags)
 {
 	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
 	struct stack_map_bucket *new_bucket, *old_bucket;
 	bool user = flags & BPF_F_USER_STACK;
-	bool hash_matches;
 	u32 trace_len, i;
 	int err;
 
-	err = stackid_init(stackid, map, trace, flags);
-	if (err)
+	err = stackid_fastpath(stackid, map, trace, flags);
+	if (err != -ENOENT)
 		return err;
 
-	hash_matches = stackid->bucket && stackid->bucket->hash == stackid->hash;
-	/* fast cmp */
-	if (hash_matches && flags & BPF_F_FAST_STACK_CMP)
-		return stackid->id;
-
 	if (stack_map_use_build_id(map)) {
 		struct bpf_stack_build_id *id_offs;
 
@@ -567,7 +586,7 @@ static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
 			id_offs[i].ip = stackid->ips[i];
 		stack_map_get_build_id_offset(id_offs, stackid->nr, user, false /* !may_fault */);
 		trace_len = stackid->nr * sizeof(struct bpf_stack_build_id);
-		if (hash_matches && stackid->bucket->nr == stackid->nr &&
+		if (stackid->hash_matches && stackid->bucket->nr == stackid->nr &&
 		    memcmp(stackid->bucket->data, new_bucket->data, trace_len) == 0) {
 			pcpu_freelist_push(&smap->freelist, &new_bucket->fnode);
 			return stackid->id;
@@ -577,12 +596,6 @@ static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
 			return -EEXIST;
 		}
 	} else {
-		if (hash_matches && stackid->bucket->nr == stackid->nr &&
-		    memcmp(stackid->bucket->data, stackid->ips, stackid->len) == 0)
-			return stackid->id;
-		if (stackid->bucket && !(flags & BPF_F_REUSE_STACKID))
-			return -EEXIST;
-
 		new_bucket = (struct stack_map_bucket *)
 			pcpu_freelist_pop(&smap->freelist);
 		if (unlikely(!new_bucket))
-- 
2.54.0

[PATCH bpf-next 3/9] bpf: Factor stackid_new_bucket from __bpf_get_stackid

[Jiri Olsa]

The new stackid_new_bucket allocates the new bucket and initializes it
with the trace data.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 49 +++++++++++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 7bc2a966e3e8..48a2e28f0c68 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -559,31 +559,53 @@ static int stackid_fastpath(struct stackid *stackid, struct bpf_map *map,
 	return -ENOENT;
 }
 
+static struct stack_map_bucket *
+stackid_new_bucket(struct stackid *stackid, struct bpf_map *map,
+		   struct perf_callchain_entry *trace, u64 flags)
+{
+	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
+	struct bpf_stack_build_id *id_offs;
+	struct stack_map_bucket *bucket;
+	u32 i;
+
+	bucket = (struct stack_map_bucket *) pcpu_freelist_pop(&smap->freelist);
+	if (unlikely(!bucket))
+		return NULL;
+
+	if (stack_map_use_build_id(map)) {
+		id_offs = (struct bpf_stack_build_id *)bucket->data;
+		for (i = 0; i < stackid->nr; i++)
+			id_offs[i].ip = stackid->ips[i];
+	} else {
+		memcpy(bucket->data, stackid->ips, stackid->len);
+	}
+
+	bucket->hash = stackid->hash;
+	bucket->nr = stackid->nr;
+	return bucket;
+}
+
 static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
 			      struct perf_callchain_entry *trace, u64 flags)
 {
 	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
 	struct stack_map_bucket *new_bucket, *old_bucket;
 	bool user = flags & BPF_F_USER_STACK;
-	u32 trace_len, i;
+	u32 trace_len;
 	int err;
 
 	err = stackid_fastpath(stackid, map, trace, flags);
 	if (err != -ENOENT)
 		return err;
 
+	new_bucket = stackid_new_bucket(stackid, map, trace, flags);
+	if (!new_bucket)
+		return -ENOMEM;
+
 	if (stack_map_use_build_id(map)) {
 		struct bpf_stack_build_id *id_offs;
 
-		/* for build_id+offset, pop a bucket before slow cmp */
-		new_bucket = (struct stack_map_bucket *)
-			pcpu_freelist_pop(&smap->freelist);
-		if (unlikely(!new_bucket))
-			return -ENOMEM;
-		new_bucket->nr = stackid->nr;
 		id_offs = (struct bpf_stack_build_id *)new_bucket->data;
-		for (i = 0; i < stackid->nr; i++)
-			id_offs[i].ip = stackid->ips[i];
 		stack_map_get_build_id_offset(id_offs, stackid->nr, user, false /* !may_fault */);
 		trace_len = stackid->nr * sizeof(struct bpf_stack_build_id);
 		if (stackid->hash_matches && stackid->bucket->nr == stackid->nr &&
@@ -595,17 +617,8 @@ static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
 			pcpu_freelist_push(&smap->freelist, &new_bucket->fnode);
 			return -EEXIST;
 		}
-	} else {
-		new_bucket = (struct stack_map_bucket *)
-			pcpu_freelist_pop(&smap->freelist);
-		if (unlikely(!new_bucket))
-			return -ENOMEM;
-		memcpy(new_bucket->data, stackid->ips, stackid->len);
 	}
 
-	new_bucket->hash = stackid->hash;
-	new_bucket->nr = stackid->nr;
-
 	old_bucket = xchg(&smap->buckets[stackid->id], new_bucket);
 	if (old_bucket)
 		pcpu_freelist_push(&smap->freelist, &old_bucket->fnode);
-- 
2.54.0

[PATCH bpf-next 4/9] bpf: Use stack id functions instead of __bpf_get_stackid

[Jiri Olsa]

Replacing __bpf_get_stackid calls with sequence of following functions:

  stackid_fastpath
  stackid_new_bucket
  stackid_install

This makes code more structured and allows us to easily disable
preemption only in bpf_get_stackid in following changes.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 43 +++++++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 48a2e28f0c68..83efd5892d90 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -585,22 +585,13 @@ stackid_new_bucket(struct stackid *stackid, struct bpf_map *map,
 	return bucket;
 }
 
-static long __bpf_get_stackid(struct stackid *stackid, struct bpf_map *map,
-			      struct perf_callchain_entry *trace, u64 flags)
+static long stackid_install(struct stackid *stackid, struct bpf_map *map,
+			    struct stack_map_bucket *new_bucket, u64 flags)
 {
 	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
-	struct stack_map_bucket *new_bucket, *old_bucket;
 	bool user = flags & BPF_F_USER_STACK;
+	struct stack_map_bucket *old_bucket;
 	u32 trace_len;
-	int err;
-
-	err = stackid_fastpath(stackid, map, trace, flags);
-	if (err != -ENOENT)
-		return err;
-
-	new_bucket = stackid_new_bucket(stackid, map, trace, flags);
-	if (!new_bucket)
-		return -ENOMEM;
 
 	if (stack_map_use_build_id(map)) {
 		struct bpf_stack_build_id *id_offs;
@@ -630,10 +621,12 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 {
 	u32 elem_size = stack_map_data_size(map);
 	bool user = flags & BPF_F_USER_STACK;
+	struct stack_map_bucket *new_bucket;
 	struct perf_callchain_entry *trace;
 	struct stackid stackid;
 	bool kernel = !user;
 	u32 max_depth;
+	int err;
 
 	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
 			       BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
@@ -647,7 +640,15 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 		/* couldn't fetch the stack trace */
 		return -EFAULT;
 
-	return __bpf_get_stackid(&stackid, map, trace, flags);
+	err = stackid_fastpath(&stackid, map, trace, flags);
+	if (err != -ENOENT)
+		return err;
+
+	new_bucket = stackid_new_bucket(&stackid, map, trace, flags);
+	if (!new_bucket)
+		return -ENOMEM;
+
+	return stackid_install(&stackid, map, new_bucket, flags);
 }
 
 const struct bpf_func_proto bpf_get_stackid_proto = {
@@ -675,6 +676,7 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 	   struct bpf_map *, map, u64, flags)
 {
 	struct perf_event *event = ctx->event;
+	struct stack_map_bucket *new_bucket;
 	struct perf_callchain_entry *trace;
 	struct stackid stackid;
 	bool kernel, user;
@@ -702,7 +704,6 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 
 	if (kernel) {
 		trace->nr = nr_kernel;
-		ret = __bpf_get_stackid(&stackid, map, trace, flags);
 	} else { /* user */
 		u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
 
@@ -711,12 +712,22 @@ BPF_CALL_3(bpf_get_stackid_pe, struct bpf_perf_event_data_kern *, ctx,
 			return -EFAULT;
 
 		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
-		ret = __bpf_get_stackid(&stackid, map, trace, flags);
 	}
 
+	ret = stackid_fastpath(&stackid, map, trace, flags);
+	if (ret != -ENOENT)
+		goto error;
+
+	new_bucket = stackid_new_bucket(&stackid, map, trace, flags);
+	if (new_bucket) {
+		trace->nr = nr;
+		return stackid_install(&stackid, map, new_bucket, flags);
+	}
+	ret = -ENOMEM;
+
+error:
 	/* restore nr */
 	trace->nr = nr;
-
 	return ret;
 }
 
-- 
2.54.0

[PATCH bpf-next 5/9] bpf: Disable preemption in bpf_get_stackid

[Jiri Olsa]

The get_perf_callchain call needs disabled preemption plus we need
it disabled as long as we access its returned trace entries buffer.

Note the bpf_get_stackid_pe function is executed already with
preemption disabled.

Cc: stable@vger.kernel.org
Fixes: d5a3b1f69186 ("bpf: introduce BPF_MAP_TYPE_STACK_TRACE")
Reported-by: Tao Chen <chen.dylane@linux.dev>
Closes: https://lore.kernel.org/bpf/20260206090653.1336687-2-chen.dylane@linux.dev/
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 83efd5892d90..a64b14ffd1e8 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -625,30 +625,37 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
 	struct perf_callchain_entry *trace;
 	struct stackid stackid;
 	bool kernel = !user;
+	int err = -EFAULT;
 	u32 max_depth;
-	int err;
 
 	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
 			       BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
 		return -EINVAL;
 
 	max_depth = stack_map_calculate_max_depth(map->value_size, elem_size, flags);
+
+	preempt_disable();
 	trace = get_perf_callchain(regs, kernel, user, max_depth,
 				   false, false, 0);
 
 	if (unlikely(!trace))
 		/* couldn't fetch the stack trace */
-		return -EFAULT;
+		goto out;
 
 	err = stackid_fastpath(&stackid, map, trace, flags);
 	if (err != -ENOENT)
-		return err;
+		goto out;
 
 	new_bucket = stackid_new_bucket(&stackid, map, trace, flags);
-	if (!new_bucket)
-		return -ENOMEM;
+	if (new_bucket) {
+		preempt_enable();
+		return stackid_install(&stackid, map, new_bucket, flags);
+	}
+	err = -ENOMEM;
 
-	return stackid_install(&stackid, map, new_bucket, flags);
+out:
+	preempt_enable();
+	return err;
 }
 
 const struct bpf_func_proto bpf_get_stackid_proto = {
-- 
2.54.0

[PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack

[Jiri Olsa]

The new callchain_store function stores trace entries buffer into
user supplied buffer. It covers both just-ip and buildid data.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index a64b14ffd1e8..b990643f25cd 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -747,6 +747,29 @@ const struct bpf_func_proto bpf_get_stackid_proto_pe = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+static u32 callchain_store(struct perf_callchain_entry *trace, void *buf, u32 size,
+			   u32 elem_size, u64 flags)
+{
+	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
+	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
+	u32 trace_nr, copy_len;
+	u64 *ips;
+
+	trace_nr = trace->nr - skip;
+	copy_len = trace_nr * elem_size;
+
+	ips = trace->ip + skip;
+	if (user_build_id) {
+		struct bpf_stack_build_id *id_offs = buf;
+
+		for (u32 i = 0; i < trace_nr; i++)
+			id_offs[i].ip = ips[i];
+	} else {
+		memcpy(buf, ips, copy_len);
+	}
+	return trace_nr;
+}
+
 static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 			    struct perf_callchain_entry *trace_in,
 			    void *buf, u32 size, u64 flags, bool may_fault)
@@ -759,7 +782,6 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 	struct perf_callchain_entry *trace;
 	bool kernel = !user;
 	int err = -EINVAL;
-	u64 *ips;
 
 	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
 			       BPF_F_USER_BUILD_ID)))
@@ -804,21 +826,10 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 		goto err_fault;
 	}
 
-	trace_nr = trace->nr - skip;
+	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
 	copy_len = trace_nr * elem_size;
 
-	ips = trace->ip + skip;
-	if (user_build_id) {
-		struct bpf_stack_build_id *id_offs = buf;
-		u32 i;
-
-		for (i = 0; i < trace_nr; i++)
-			id_offs[i].ip = ips[i];
-	} else {
-		memcpy(buf, ips, copy_len);
-	}
-
-	/* trace/ips should not be dereferenced after this point */
+	/* trace should not be dereferenced after this point */
 	if (may_fault)
 		rcu_read_unlock();
 
-- 
2.54.0

[PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack

[Jiri Olsa]

The new callchain_finalize function calls the build-id retrieval
(if needed) and zeroes the buffer.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index b990643f25cd..eea7b781300c 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -770,16 +770,29 @@ static u32 callchain_store(struct perf_callchain_entry *trace, void *buf, u32 si
 	return trace_nr;
 }
 
+static long callchain_finalize(void *buf, u32 size, u32 trace_nr, u32 elem_size,
+			       bool user_build_id, bool user, bool may_fault)
+{
+	u32 copy_len = trace_nr * elem_size;
+
+	if (user_build_id)
+		stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
+
+	if (size > copy_len)
+		memset(buf + copy_len, 0, size - copy_len);
+	return copy_len;
+}
+
 static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 			    struct perf_callchain_entry *trace_in,
 			    void *buf, u32 size, u64 flags, bool may_fault)
 {
-	u32 trace_nr, copy_len, elem_size, max_depth;
 	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
 	bool crosstask = task && task != current;
 	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
 	bool user = flags & BPF_F_USER_STACK;
 	struct perf_callchain_entry *trace;
+	u32 trace_nr, elem_size, max_depth;
 	bool kernel = !user;
 	int err = -EINVAL;
 
@@ -827,18 +840,12 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 	}
 
 	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
-	copy_len = trace_nr * elem_size;
 
 	/* trace should not be dereferenced after this point */
 	if (may_fault)
 		rcu_read_unlock();
 
-	if (user_build_id)
-		stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
-
-	if (size > copy_len)
-		memset(buf + copy_len, 0, size - copy_len);
-	return copy_len;
+	return callchain_finalize(buf, size, trace_nr, elem_size, user_build_id, user, may_fault);
 
 err_fault:
 	err = -EFAULT;
-- 
2.54.0

[PATCH bpf-next 8/9] bpf: Remove trace_in argument from __bpf_get_stack

[Jiri Olsa]

Now with the new callchain_* helper functions we can process trace_in
case directly in bpf_get_stack_pe function and remove it from
__bpf_get_stack which makes things easier for preemption fix in
following change.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 50 +++++++++++++++++++++++++++++++------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index eea7b781300c..57cd4c33403b 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -784,7 +784,6 @@ static long callchain_finalize(void *buf, u32 size, u32 trace_nr, u32 elem_size,
 }
 
 static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
-			    struct perf_callchain_entry *trace_in,
 			    void *buf, u32 size, u64 flags, bool may_fault)
 {
 	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
@@ -823,10 +822,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 */
 
-	if (trace_in) {
-		trace = trace_in;
-		trace->nr = min_t(u32, trace->nr, max_depth);
-	} else if (kernel && task) {
+	if (kernel && task) {
 		trace = get_callchain_entry_for_task(task, max_depth);
 	} else {
 		trace = get_perf_callchain(regs, kernel, user, max_depth,
@@ -857,7 +853,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 BPF_CALL_4(bpf_get_stack, struct pt_regs *, regs, void *, buf, u32, size,
 	   u64, flags)
 {
-	return __bpf_get_stack(regs, NULL, NULL, buf, size, flags, false /* !may_fault */);
+	return __bpf_get_stack(regs, NULL, buf, size, flags, false /* !may_fault */);
 }
 
 const struct bpf_func_proto bpf_get_stack_proto = {
@@ -873,7 +869,7 @@ const struct bpf_func_proto bpf_get_stack_proto = {
 BPF_CALL_4(bpf_get_stack_sleepable, struct pt_regs *, regs, void *, buf, u32, size,
 	   u64, flags)
 {
-	return __bpf_get_stack(regs, NULL, NULL, buf, size, flags, true /* may_fault */);
+	return __bpf_get_stack(regs, NULL, buf, size, flags, true /* may_fault */);
 }
 
 const struct bpf_func_proto bpf_get_stack_sleepable_proto = {
@@ -897,7 +893,7 @@ static long __bpf_get_task_stack(struct task_struct *task, void *buf, u32 size,
 
 	regs = task_pt_regs(task);
 	if (regs)
-		res = __bpf_get_stack(regs, task, NULL, buf, size, flags, may_fault);
+		res = __bpf_get_stack(regs, task, buf, size, flags, may_fault);
 	put_task_stack(task);
 
 	return res;
@@ -937,6 +933,33 @@ const struct bpf_func_proto bpf_get_task_stack_sleepable_proto = {
 	.arg4_type	= ARG_ANYTHING,
 };
 
+static int __bpf_get_stack_pe(struct perf_callchain_entry *trace, void *buf, u32 size,
+			      u64 flags)
+{
+	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
+	u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
+	bool user = flags & BPF_F_USER_STACK;
+	u32 elem_size, max_depth, nr_trace;
+	bool kernel = !user;
+
+	if (kernel && user_build_id)
+		return -EINVAL;
+
+	elem_size = user_build_id ? sizeof(struct bpf_stack_build_id) : sizeof(u64);
+	if (unlikely(size % elem_size))
+		return -EINVAL;
+
+	max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
+	trace->nr = min_t(u32, trace->nr, max_depth);
+
+	if (trace->nr < skip)
+		return -EFAULT;
+
+	nr_trace = callchain_store(trace, buf, size, elem_size, flags);
+	return callchain_finalize(buf, size, nr_trace, elem_size, user_build_id,
+				  user, false /* !may_fault */);
+}
+
 BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
 	   void *, buf, u32, size, u64, flags)
 {
@@ -948,7 +971,7 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
 	__u64 nr_kernel;
 
 	if (!(event->attr.sample_type & PERF_SAMPLE_CALLCHAIN))
-		return __bpf_get_stack(regs, NULL, NULL, buf, size, flags, false /* !may_fault */);
+		return __bpf_get_stack(regs, NULL, buf, size, flags, false /* !may_fault */);
 
 	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
 			       BPF_F_USER_BUILD_ID)))
@@ -968,7 +991,7 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
 		__u64 nr = trace->nr;
 
 		trace->nr = nr_kernel;
-		err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
+		err = __bpf_get_stack_pe(trace, buf, size, flags);
 
 		/* restore nr */
 		trace->nr = nr;
@@ -978,14 +1001,13 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
 		skip += nr_kernel;
 		if (skip > BPF_F_SKIP_FIELD_MASK)
 			goto clear;
-
 		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
-		err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
+		err = __bpf_get_stack_pe(trace, buf, size, flags);
 	}
-	return err;
 
 clear:
-	memset(buf, 0, size);
+	if (err < 0)
+		memset(buf, 0, size);
 	return err;
 
 }
-- 
2.54.0

[PATCH bpf-next 9/9] bpf: Disable preemption in __bpf_get_stack

[Jiri Olsa]

From: Daniel Borkmann <borkmann@iogearbox.net>

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() previously taken here 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: stable@vger.kernel.org
Fixes: c195651e565a ("bpf: add bpf_get_stack helper")
Reported-by: Tao Chen <chen.dylane@linux.dev>
Closes: https://lore.kernel.org/bpf/20260206090653.1336687-1-chen.dylane@linux.dev/
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <borkmann@iogearbox.net>
[ changed Fixes: commit ]
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/stackmap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 57cd4c33403b..37f8e46319b3 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -819,8 +819,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 
 	max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
 
-	if (may_fault)
-		rcu_read_lock(); /* need RCU for perf's callchain below */
+	preempt_disable();
 
 	if (kernel && task) {
 		trace = get_callchain_entry_for_task(task, max_depth);
@@ -830,16 +829,14 @@ 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;
 	}
 
 	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
 
 	/* 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, user_build_id, user, may_fault);
 
-- 
2.54.0

Re: [PATCH bpf-next 3/9] bpf: Factor stackid_new_bucket from __bpf_get_stackid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The newly introduced `stackid_new_bucket` function accepts `trace` and `flags` parameters which are completely unused within its body.
--

--- Patch [3]: [PATCH bpf-next 3/9] bpf: Factor stackid_new_bucket from __bpf_get_stackid ---
commit cee0ea77a9c1144b54fb0472b1788623044b5650
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor stackid_new_bucket from __bpf_get_stackid

This commit factors out the bucket allocation and initialization logic
from __bpf_get_stackid into a new stackid_new_bucket helper function.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 7bc2a966e3e81..48a2e28f0c68a 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -559,31 +559,53 @@ static int stackid_fastpath(struct stackid *stackid, struct bpf_map *map,
>  	return -ENOENT;
>  }
>  
> +static struct stack_map_bucket *
> +stackid_new_bucket(struct stackid *stackid, struct bpf_map *map,
> +		   struct perf_callchain_entry *trace, u64 flags)

[Severity: Low]
Are the trace and flags parameters needed here?

Looking at the implementation of stackid_new_bucket() below, neither trace
nor flags are used anywhere in the function body. Could they be removed
from the function signature to clean up the API?

> +{
> +	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
> +	struct bpf_stack_build_id *id_offs;
> +	struct stack_map_bucket *bucket;
> +	u32 i;
> +
> +	bucket = (struct stack_map_bucket *) pcpu_freelist_pop(&smap->freelist);
> +	if (unlikely(!bucket))
> +		return NULL;
> +
> +	if (stack_map_use_build_id(map)) {
> +		id_offs = (struct bpf_stack_build_id *)bucket->data;
> +		for (i = 0; i < stackid->nr; i++)
> +			id_offs[i].ip = stackid->ips[i];
> +	} else {
> +		memcpy(bucket->data, stackid->ips, stackid->len);
> +	}
> +
> +	bucket->hash = stackid->hash;
> +	bucket->nr = stackid->nr;
> +	return bucket;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=3

Re: [PATCH bpf-next 5/9] bpf: Disable preemption in bpf_get_stackid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for preemption vulnerability in stack helpers (__bpf_get_stack)
--

--- Patch [5]: [PATCH bpf-next 5/9] bpf: Disable preemption in bpf_get_stackid ---
commit eb097d7647defa7379264dcdf95711bc573a419a
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Disable preemption in bpf_get_stackid

This commit adds preempt_disable() and preempt_enable() around the
get_perf_callchain() call and trace buffer usage in bpf_get_stackid()
to prevent the per-CPU trace entries buffer from being corrupted if the
task is preempted.

Closes: https://lore.kernel.org/bpf/20260206090653.1336687-2-chen.dylane@linux.dev/

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 83efd5892d907..a64b14ffd1e8e 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -625,30 +625,37 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
>  	struct perf_callchain_entry *trace;
>  	struct stackid stackid;
>  	bool kernel = !user;
> +	int err = -EFAULT;
>  	u32 max_depth;
> -	int err;
>  
>  	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
>  			       BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
>  		return -EINVAL;
>  
>  	max_depth = stack_map_calculate_max_depth(map->value_size, elem_size, flags);
> +
> +	preempt_disable();
>  	trace = get_perf_callchain(regs, kernel, user, max_depth,
>  				   false, false, 0);

[Severity: High]
This isn't a bug introduced by this patch, but does __bpf_get_stack() also
need this preempt_disable() protection?

__bpf_get_stack() has a similar pattern where it calls get_perf_callchain()
or get_callchain_entry_for_task() and dereferences trace->ip via memcpy().

While it uses rcu_read_lock() when may_fault is true, on CONFIG_PREEMPT_RCU
kernels this does not prevent preemption.

Could a preempting task trigger another stack trace on the same CPU before
the memcpy() finishes, overwriting the per-CPU buffer in __bpf_get_stack()?

>  	if (unlikely(!trace))
>  		/* couldn't fetch the stack trace */
> -		return -EFAULT;
> +		goto out;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=5

Re: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The 'size' parameter in callchain_store() is unused.
--

--- Patch [6]: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack ---
commit aee65ba10526fcf4c6320ea660fc50a555551cc1
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor callchain_store function from __bpf_get_stack

This commit extracts the logic for storing trace entries into a user-supplied
buffer from __bpf_get_stack() and places it into a new helper function named
callchain_store().

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index a64b14ffd1e8e..b990643f25cd8 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -747,6 +747,29 @@ const struct bpf_func_proto bpf_get_stackid_proto_pe = {
>  	.arg3_type	= ARG_ANYTHING,
>  };
>  
> +static u32 callchain_store(struct perf_callchain_entry *trace, void *buf, u32 size,
> +			   u32 elem_size, u64 flags)

[Severity: Low]
This isn't a bug, but the 'size' parameter doesn't appear to be used anywhere
within callchain_store() in kernel/bpf/stackmap.c. Could it be safely removed
from the function signature since the bounds checking is handled in the caller?

> +{
> +	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
> +	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
> +	u32 trace_nr, copy_len;
> +	u64 *ips;
> +
> +	trace_nr = trace->nr - skip;
> +	copy_len = trace_nr * elem_size;
> +
> +	ips = trace->ip + skip;
> +	if (user_build_id) {
> +		struct bpf_stack_build_id *id_offs = buf;
> +
> +		for (u32 i = 0; i < trace_nr; i++)
> +			id_offs[i].ip = ips[i];
> +	} else {
> +		memcpy(buf, ips, copy_len);
> +	}
> +	return trace_nr;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=6

Re: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Sleepable BPF programs calling bpf_get_stack() or bpf_get_task_stack() can corrupt per-CPU perf callchain buffers and trigger preemption warnings.
--

--- Patch [7]: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack ---
commit a181c39a42a0e8c840b9a2a09b02e1efa512170b
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor callchain_finalize function from __bpf_get_stack
    
The new callchain_finalize function calls the build-id retrieval
(if needed) and zeroes the buffer.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index b990643f25cd8..eea7b781300ca 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -827,18 +840,12 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>  	}
>  
>  	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
> -	copy_len = trace_nr * elem_size;
>  
>  	/* trace should not be dereferenced after this point */
>  	if (may_fault)
>  		rcu_read_unlock();

[Severity: High]
This is a pre-existing issue, but can sleepable BPF programs calling this
function corrupt per-CPU perf callchain buffers and trigger preemption
warnings?

When may_fault is true (such as from bpf_get_stack_sleepable or
bpf_get_task_stack_sleepable), this code relies on rcu_read_lock() earlier
in the function. However, on PREEMPT_RCU kernels, rcu_read_lock() does not
disable preemption.

The earlier call to get_perf_callchain() eventually reaches
get_callchain_entry(), which relies on smp_processor_id() and requires
preemption to be explicitly disabled:

kernel/events/callchain.c:get_callchain_entry() {
    ...
    cpu = smp_processor_id();
    return (((void *)entries->cpu_entries[cpu]) +
            (*rctx * perf_callchain_entry__sizeof()));
}

Since preemption is not disabled during this sequence, can the task be
preempted during or after fetching the per-CPU buffer, allowing another BPF
program on the same CPU to overwrite the buffer before it is copied by
callchain_store()?

> -	if (user_build_id)
> -		stack_map_get_build_id_offset(buf, trace_nr, user, may_fault);
> -
> -	if (size > copy_len)
> -		memset(buf + copy_len, 0, size - copy_len);
> -	return copy_len;
> +	return callchain_finalize(buf, size, trace_nr, elem_size, user_build_id, user, may_fault);
>  
>  err_fault:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=7

Re: [PATCH bpf-next 8/9] bpf: Remove trace_in argument from __bpf_get_stack

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] bpf_get_stack_pe permanently truncates the shared perf event callchain for user stacks.
--

--- Patch [8]: [PATCH bpf-next 8/9] bpf: Remove trace_in argument from __bpf_get_stack ---
commit 42ed70b803080e71468d115e2948bdba98a800f5
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Remove trace_in argument from __bpf_get_stack

This commit removes the trace_in argument from __bpf_get_stack and
handles the trace_in case directly in a new __bpf_get_stack_pe function.
This simplifies __bpf_get_stack for upcoming preemption fixes.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index eea7b781300ca..57cd4c33403bd 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -978,14 +1001,13 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
>  		skip += nr_kernel;
>  		if (skip > BPF_F_SKIP_FIELD_MASK)
>  			goto clear;
> -
>  		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
> -		err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> +		err = __bpf_get_stack_pe(trace, buf, size, flags);

[Severity: High]
This is a pre-existing issue, but does this permanently truncate the shared
perf event callchain for user stacks?

In bpf_get_stack_pe(), the kernel stack branch explicitly saves and restores
trace->nr around the call to __bpf_get_stack_pe().

However, the user stack branch directly calls __bpf_get_stack_pe() without
saving the original trace->nr value. Inside __bpf_get_stack_pe(), the trace
size is modified in place:

static int __bpf_get_stack_pe(...)
{
        ...
        max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
        trace->nr = min_t(u32, trace->nr, max_depth);
        ...
}

If a BPF program attached to a perf event requests a user stack trace and
provides a buffer smaller than the actual callchain length, trace->nr is
truncated.

When the perf subsystem later processes this event and outputs it to the
perf ring buffer, could this result in incomplete or corrupted profiling
data due to the truncated user space callchain length?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=8

Re: [PATCH bpf-next 2/9] bpf: Factor stackid_fastpath function from __bpf_get_stackid

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A pre-existing TOCTOU race condition in `stackid_init` involving `sysctl_perf_event_max_stack` can cause a `max_depth - skip` underflow, leading to a kernel heap buffer overflow.
--

--- Patch [2]: [PATCH bpf-next 2/9] bpf: Factor stackid_fastpath function from __bpf_get_stackid ---
commit 1a0215d43cbe727ac7583d25380f0acdc326abc9
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor stackid_fastpath function from __bpf_get_stackid

This commit extracts the fast stack hash and trace check logic from
__bpf_get_stackid into a new stackid_fastpath function. It optimizes both
the just-ip and buildid code paths by avoiding new bucket allocation during
the fast path check.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 0eafe55b18289..7bc2a966e3e81 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -531,28 +532,46 @@ static int stackid_init(struct stackid *stackid, struct bpf_map *map,
>  	stackid->hash = jhash2((u32 *)stackid->ips, stackid->len / sizeof(u32), 0);
>  	stackid->id = stackid->hash & (smap->n_buckets - 1);
>  	stackid->bucket = READ_ONCE(smap->buckets[stackid->id]);
> +	stackid->hash_matches = stackid->bucket && stackid->bucket->hash == stackid->hash;
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but could the stackid calculation
in this function underflow and cause an out-of-bounds write?

Just above this diff in stackid_init(), stackid->nr is calculated like this:

	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);

If sysctl_perf_event_max_stack is lowered concurrently while a BPF program
is executing, max_depth gets capped to the new smaller sysctl value.

If the BPF program provides a skip value larger than the new sysctl but
smaller than the old one, max_depth - skip will underflow to a large u32
value.

The min_t() would then incorrectly select trace->nr - skip, resulting in an
unsafely large stackid->len.

Later, this length is used to write into a bucket that was allocated based
on the smaller map->value_size:

	memcpy(new_bucket->data, stackid->ips, stackid->len);

Does this sequence allow a kernel heap buffer overflow to occur?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=2

Re: [PATCH bpf-next 9/9] bpf: Disable preemption in __bpf_get_stack

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Replacing rcu_read_lock() with preempt_disable() triggers a lockdep warning ('suspicious rcu_dereference_check() usage') when __bpf_get_stack() is called from sleepable BPF programs.

Pre-existing issues:
- [Critical] Synchronous re-entrancy from BPF trampolines (fentry/fexit) enables a Time-Of-Check to Time-Of-Use (TOCTOU) vulnerability leading to an out-of-bounds memory write.
- [High] bpf_get_stack_pe permanently truncates the shared perf callchain length for user stacks.
--

--- Patch [9]: [PATCH bpf-next 9/9] bpf: Disable preemption in __bpf_get_stack ---
commit d1da66040bb1a7fc97bf16d7cb97a0712702bc93
Author: Daniel Borkmann <borkmann@iogearbox.net>

bpf: Disable preemption in __bpf_get_stack

This patch replaces rcu_read_lock() with preempt_disable() in
__bpf_get_stack() to prevent another task from reusing the per-CPU
perf_callchain_entry buffer. It ensures the callchain entry cannot be
reused and overwritten underneath the copy operation. Build ID resolution
is deferred until after preemption is re-enabled.

Closes: https://lore.kernel.org/bpf/20260206090653.1336687-1-chen.dylane@linux.dev/

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index 57cd4c33403bd..37f8e46319b34 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -819,8 +819,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>  
>  	max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
>  
> -	if (may_fault)
> -		rcu_read_lock(); /* need RCU for perf's callchain below */
> +	preempt_disable();

[Severity: High]
Does replacing rcu_read_lock() with preempt_disable() trigger a lockdep
warning when __bpf_get_stack() is called from sleepable BPF programs?

Sleepable programs run under rcu_read_lock_trace(), not rcu_read_lock().
With preempt_disable() replacing rcu_read_lock(), the lockdep rcu_lock_map
is not acquired.

When get_perf_callchain() calls get_callchain_entry(), it accesses
callchain_cpus_entries using rcu_dereference(). Since rcu_dereference()
explicitly checks for rcu_read_lock_held(), will this evaluate to false
and trigger a suspicious rcu_dereference_check() warning?

>  
>  	if (kernel && task) {
>  		trace = get_callchain_entry_for_task(task, max_depth);
> @@ -830,16 +829,14 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
>  	}

[Severity: Critical]
This is a pre-existing issue, but can synchronous re-entrancy from BPF
trampolines enable a Time-Of-Check to Time-Of-Use (TOCTOU) vulnerability?

get_perf_callchain() acquires the shared per-CPU perf_callchain_entry
buffer but drops the recursion lock via put_callchain_entry() right
before returning.

If a BPF fexit program is attached to get_perf_callchain(), it will run
synchronously before __bpf_get_stack() resumes. The fexit program can
recursively call bpf_get_stack() with a much larger size. Since the lock
was dropped, it re-acquires the identical buffer and inflates trace->nr.

Upon returning, __bpf_get_stack() passes the inflated trace to
callchain_store(). Because callchain_store() blindly trusts trace->nr
without re-validating it against max_depth, will this result in an
out-of-bounds memcpy()?

>  
>  	if (unlikely(!trace) || trace->nr < skip) {
> -		if (may_fault)
> -			rcu_read_unlock();
> +		preempt_enable();
>  		goto err_fault;
>  	}
>  
>  	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
>  
>  	/* 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, user_build_id, user, may_fault);

[Severity: High]
This is a pre-existing issue, but does bpf_get_stack_pe() permanently
truncate the shared perf callchain length for user stacks?

When bpf_get_stack_pe() is called with BPF_F_USER_STACK, it routes to
__bpf_get_stack_pe(), passing the shared perf event buffer:

kernel/bpf/stackmap.c:__bpf_get_stack_pe() {
    ...
    trace->nr = min_t(u32, trace->nr, max_depth);
    ...
}

Unlike the kernel branch which explicitly saves and restores trace->nr,
the user branch fails to restore the original length before returning in
bpf_get_stack_pe().

Can this inadvertently or maliciously corrupt the event record for any
subsequent users of the perf subsystem?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=9

Re: [PATCH bpf-next 3/9] bpf: Factor stackid_new_bucket from __bpf_get_stackid

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:01:40AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The newly introduced `stackid_new_bucket` function accepts `trace` and `flags` parameters which are completely unused within its body.
> --
> 
> --- Patch [3]: [PATCH bpf-next 3/9] bpf: Factor stackid_new_bucket from __bpf_get_stackid ---
> commit cee0ea77a9c1144b54fb0472b1788623044b5650
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Factor stackid_new_bucket from __bpf_get_stackid
> 
> This commit factors out the bucket allocation and initialization logic
> from __bpf_get_stackid into a new stackid_new_bucket helper function.
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index 7bc2a966e3e81..48a2e28f0c68a 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -559,31 +559,53 @@ static int stackid_fastpath(struct stackid *stackid, struct bpf_map *map,
> >  	return -ENOENT;
> >  }
> >  
> > +static struct stack_map_bucket *
> > +stackid_new_bucket(struct stackid *stackid, struct bpf_map *map,
> > +		   struct perf_callchain_entry *trace, u64 flags)
> 
> [Severity: Low]
> Are the trace and flags parameters needed here?
> 
> Looking at the implementation of stackid_new_bucket() below, neither trace
> nor flags are used anywhere in the function body. Could they be removed
> from the function signature to clean up the API?

right, will remove

jirka

> 
> > +{
> > +	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
> > +	struct bpf_stack_build_id *id_offs;
> > +	struct stack_map_bucket *bucket;
> > +	u32 i;
> > +
> > +	bucket = (struct stack_map_bucket *) pcpu_freelist_pop(&smap->freelist);
> > +	if (unlikely(!bucket))
> > +		return NULL;
> > +
> > +	if (stack_map_use_build_id(map)) {
> > +		id_offs = (struct bpf_stack_build_id *)bucket->data;
> > +		for (i = 0; i < stackid->nr; i++)
> > +			id_offs[i].ip = stackid->ips[i];
> > +	} else {
> > +		memcpy(bucket->data, stackid->ips, stackid->len);
> > +	}
> > +
> > +	bucket->hash = stackid->hash;
> > +	bucket->nr = stackid->nr;
> > +	return bucket;
> > +}
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=3

Re: [PATCH bpf-next 2/9] bpf: Factor stackid_fastpath function from __bpf_get_stackid

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:09:51AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] A pre-existing TOCTOU race condition in `stackid_init` involving `sysctl_perf_event_max_stack` can cause a `max_depth - skip` underflow, leading to a kernel heap buffer overflow.
> --
> 
> --- Patch [2]: [PATCH bpf-next 2/9] bpf: Factor stackid_fastpath function from __bpf_get_stackid ---
> commit 1a0215d43cbe727ac7583d25380f0acdc326abc9
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Factor stackid_fastpath function from __bpf_get_stackid
> 
> This commit extracts the fast stack hash and trace check logic from
> __bpf_get_stackid into a new stackid_fastpath function. It optimizes both
> the just-ip and buildid code paths by avoiding new bucket allocation during
> the fast path check.
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index 0eafe55b18289..7bc2a966e3e81 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -531,28 +532,46 @@ static int stackid_init(struct stackid *stackid, struct bpf_map *map,
> >  	stackid->hash = jhash2((u32 *)stackid->ips, stackid->len / sizeof(u32), 0);
> >  	stackid->id = stackid->hash & (smap->n_buckets - 1);
> >  	stackid->bucket = READ_ONCE(smap->buckets[stackid->id]);
> > +	stackid->hash_matches = stackid->bucket && stackid->bucket->hash == stackid->hash;
> >  	return 0;
> >  }
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could the stackid calculation
> in this function underflow and cause an out-of-bounds write?
> 
> Just above this diff in stackid_init(), stackid->nr is calculated like this:
> 
> 	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);
> 
> If sysctl_perf_event_max_stack is lowered concurrently while a BPF program
> is executing, max_depth gets capped to the new smaller sysctl value.

that can't happen, perf_event_max_stack_handler needs zero in nr_callchain_events
to change the sysctl_perf_event_max_stack, but it's held by stackmap

jirka

> 
> If the BPF program provides a skip value larger than the new sysctl but
> smaller than the old one, max_depth - skip will underflow to a large u32
> value.
> 
> The min_t() would then incorrectly select trace->nr - skip, resulting in an
> unsafely large stackid->len.
> 
> Later, this length is used to write into a bucket that was allocated based
> on the smaller map->value_size:
> 
> 	memcpy(new_bucket->data, stackid->ips, stackid->len);
> 
> Does this sequence allow a kernel heap buffer overflow to occur?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=2

Re: [PATCH bpf-next 5/9] bpf: Disable preemption in bpf_get_stackid

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:04:10AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Incomplete fix for preemption vulnerability in stack helpers (__bpf_get_stack)
> --
> 
> --- Patch [5]: [PATCH bpf-next 5/9] bpf: Disable preemption in bpf_get_stackid ---
> commit eb097d7647defa7379264dcdf95711bc573a419a
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Disable preemption in bpf_get_stackid
> 
> This commit adds preempt_disable() and preempt_enable() around the
> get_perf_callchain() call and trace buffer usage in bpf_get_stackid()
> to prevent the per-CPU trace entries buffer from being corrupted if the
> task is preempted.
> 
> Closes: https://lore.kernel.org/bpf/20260206090653.1336687-2-chen.dylane@linux.dev/
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index 83efd5892d907..a64b14ffd1e8e 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -625,30 +625,37 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
> >  	struct perf_callchain_entry *trace;
> >  	struct stackid stackid;
> >  	bool kernel = !user;
> > +	int err = -EFAULT;
> >  	u32 max_depth;
> > -	int err;
> >  
> >  	if (unlikely(flags & ~(BPF_F_SKIP_FIELD_MASK | BPF_F_USER_STACK |
> >  			       BPF_F_FAST_STACK_CMP | BPF_F_REUSE_STACKID)))
> >  		return -EINVAL;
> >  
> >  	max_depth = stack_map_calculate_max_depth(map->value_size, elem_size, flags);
> > +
> > +	preempt_disable();
> >  	trace = get_perf_callchain(regs, kernel, user, max_depth,
> >  				   false, false, 0);
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does __bpf_get_stack() also
> need this preempt_disable() protection?
> 
> __bpf_get_stack() has a similar pattern where it calls get_perf_callchain()
> or get_callchain_entry_for_task() and dereferences trace->ip via memcpy().
> 
> While it uses rcu_read_lock() when may_fault is true, on CONFIG_PREEMPT_RCU
> kernels this does not prevent preemption.
> 
> Could a preempting task trigger another stack trace on the same CPU before
> the memcpy() finishes, overwriting the per-CPU buffer in __bpf_get_stack()?

__bpf_get_stack is taken care of in the following patches

jirka

Re: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:07:20AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] The 'size' parameter in callchain_store() is unused.
> --
> 
> --- Patch [6]: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack ---
> commit aee65ba10526fcf4c6320ea660fc50a555551cc1
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Factor callchain_store function from __bpf_get_stack
> 
> This commit extracts the logic for storing trace entries into a user-supplied
> buffer from __bpf_get_stack() and places it into a new helper function named
> callchain_store().
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index a64b14ffd1e8e..b990643f25cd8 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> [ ... ]
> > @@ -747,6 +747,29 @@ const struct bpf_func_proto bpf_get_stackid_proto_pe = {
> >  	.arg3_type	= ARG_ANYTHING,
> >  };
> >  
> > +static u32 callchain_store(struct perf_callchain_entry *trace, void *buf, u32 size,
> > +			   u32 elem_size, u64 flags)
> 
> [Severity: Low]
> This isn't a bug, but the 'size' parameter doesn't appear to be used anywhere
> within callchain_store() in kernel/bpf/stackmap.c. Could it be safely removed
> from the function signature since the bounds checking is handled in the caller?

true, will remove

jirka

Re: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:08:10AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Sleepable BPF programs calling bpf_get_stack() or bpf_get_task_stack() can corrupt per-CPU perf callchain buffers and trigger preemption warnings.
> --
> 
> --- Patch [7]: [PATCH bpf-next 7/9] bpf: Factor callchain_finalize function from __bpf_get_stack ---
> commit a181c39a42a0e8c840b9a2a09b02e1efa512170b
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Factor callchain_finalize function from __bpf_get_stack
>     
> The new callchain_finalize function calls the build-id retrieval
> (if needed) and zeroes the buffer.
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index b990643f25cd8..eea7b781300ca 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> [ ... ]
> > @@ -827,18 +840,12 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
> >  	}
> >  
> >  	trace_nr = callchain_store(trace, buf, size, elem_size, flags);
> > -	copy_len = trace_nr * elem_size;
> >  
> >  	/* trace should not be dereferenced after this point */
> >  	if (may_fault)
> >  		rcu_read_unlock();
> 
> [Severity: High]
> This is a pre-existing issue, but can sleepable BPF programs calling this
> function corrupt per-CPU perf callchain buffers and trigger preemption
> warnings?
> 
> When may_fault is true (such as from bpf_get_stack_sleepable or
> bpf_get_task_stack_sleepable), this code relies on rcu_read_lock() earlier
> in the function. However, on PREEMPT_RCU kernels, rcu_read_lock() does not
> disable preemption.
> 
> The earlier call to get_perf_callchain() eventually reaches
> get_callchain_entry(), which relies on smp_processor_id() and requires
> preemption to be explicitly disabled:
> 
> kernel/events/callchain.c:get_callchain_entry() {
>     ...
>     cpu = smp_processor_id();
>     return (((void *)entries->cpu_entries[cpu]) +
>             (*rctx * perf_callchain_entry__sizeof()));
> }
> 
> Since preemption is not disabled during this sequence, can the task be
> preempted during or after fetching the per-CPU buffer, allowing another BPF
> program on the same CPU to overwrite the buffer before it is copied by
> callchain_store()?

we disable the preemption in patxh # 9

jirka

Re: [PATCH bpf-next 8/9] bpf: Remove trace_in argument from __bpf_get_stack

[Jiri Olsa]

On Mon, Jul 20, 2026 at 09:09:45AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] bpf_get_stack_pe permanently truncates the shared perf event callchain for user stacks.
> --
> 
> --- Patch [8]: [PATCH bpf-next 8/9] bpf: Remove trace_in argument from __bpf_get_stack ---
> commit 42ed70b803080e71468d115e2948bdba98a800f5
> Author: Jiri Olsa <jolsa@kernel.org>
> 
> bpf: Remove trace_in argument from __bpf_get_stack
> 
> This commit removes the trace_in argument from __bpf_get_stack and
> handles the trace_in case directly in a new __bpf_get_stack_pe function.
> This simplifies __bpf_get_stack for upcoming preemption fixes.
> 
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index eea7b781300ca..57cd4c33403bd 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> [ ... ]
> > @@ -978,14 +1001,13 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> >  		skip += nr_kernel;
> >  		if (skip > BPF_F_SKIP_FIELD_MASK)
> >  			goto clear;
> > -
> >  		flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
> > -		err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> > +		err = __bpf_get_stack_pe(trace, buf, size, flags);
> 
> [Severity: High]
> This is a pre-existing issue, but does this permanently truncate the shared
> perf event callchain for user stacks?
> 
> In bpf_get_stack_pe(), the kernel stack branch explicitly saves and restores
> trace->nr around the call to __bpf_get_stack_pe().
> 
> However, the user stack branch directly calls __bpf_get_stack_pe() without
> saving the original trace->nr value. Inside __bpf_get_stack_pe(), the trace
> size is modified in place:
> 
> static int __bpf_get_stack_pe(...)
> {
>         ...
>         max_depth = stack_map_calculate_max_depth(size, elem_size, flags);
>         trace->nr = min_t(u32, trace->nr, max_depth);
>         ...
> }
> 
> If a BPF program attached to a perf event requests a user stack trace and
> provides a buffer smaller than the actual callchain length, trace->nr is
> truncated.
> 
> When the perf subsystem later processes this event and outputs it to the
> perf ring buffer, could this result in incomplete or corrupted profiling
> data due to the truncated user space callchain length?

right, will fix that in next version

jirka