[PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[PATCHv2 bpf-next 05/11] 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 43b1e5436047..e48ddb4edd47 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -624,30 +624,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);
-	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

[PATCHv2 bpf-next 10/11] 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() 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.

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index bdf48a7ad2e8..f4827afbfed9 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -820,6 +820,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 */
+	preempt_disable();
 
 	if (kernel && task) {
 		trace = get_callchain_entry_for_task(task, max_depth);
@@ -831,6 +832,7 @@ 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;
 	}
 
@@ -839,6 +841,7 @@ static long __bpf_get_stack(struct pt_regs *regs, struct task_struct *task,
 	/* 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: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Leon Hwang]

On 29/7/26 16:38, Jiri Olsa wrote:
> 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 43b1e5436047..e48ddb4edd47 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
> @@ -624,30 +624,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();

Better to use the guard-style guard(preempt)() in this series.

Thanks,
Leon

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

Re: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Jiri Olsa]

On Wed, Jul 29, 2026 at 06:30:26PM +0800, Leon Hwang wrote:
> On 29/7/26 16:38, Jiri Olsa wrote:
> > 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 43b1e5436047..e48ddb4edd47 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -624,30 +624,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();
> 
> Better to use the guard-style guard(preempt)() in this series.

we prefer to call stackid_install with preemption enabled

jirka

> 
> Thanks,
> Leon
> 
> >  	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);
> > -	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 = {
> 

Re: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Leon Hwang]

On 2026/7/30 19:38, Jiri Olsa wrote:
> On Wed, Jul 29, 2026 at 06:30:26PM +0800, Leon Hwang wrote:
>> On 29/7/26 16:38, Jiri Olsa wrote:
>>> 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 43b1e5436047..e48ddb4edd47 100644
>>> --- a/kernel/bpf/stackmap.c
>>> +++ b/kernel/bpf/stackmap.c
>>> @@ -624,30 +624,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();
>>
>> Better to use the guard-style guard(preempt)() in this series.
> 
> we prefer to call stackid_install with preemption enabled


Understand.

I wonder if we could avoid the "goto out".

scoped_guard(preempt) { ... } might work here.

But "goto out" is fine with me as well.

Thanks,
Leon

> 
> jirka
> 
>>
>> Thanks,
>> Leon
>>
>>>  	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);
>>> -	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 = {
>>

Re: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Andrii Nakryiko]

On Thu, Jul 30, 2026 at 6:43 AM Leon Hwang <leon.hwang@linux.dev> wrote:
>
> On 2026/7/30 19:38, Jiri Olsa wrote:
> > On Wed, Jul 29, 2026 at 06:30:26PM +0800, Leon Hwang wrote:
> >> On 29/7/26 16:38, Jiri Olsa wrote:
> >>> 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 43b1e5436047..e48ddb4edd47 100644
> >>> --- a/kernel/bpf/stackmap.c
> >>> +++ b/kernel/bpf/stackmap.c
> >>> @@ -624,30 +624,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();
> >>
> >> Better to use the guard-style guard(preempt)() in this series.
> >
> > we prefer to call stackid_install with preemption enabled
>
>
> Understand.
>
> I wonder if we could avoid the "goto out".
>
> scoped_guard(preempt) { ... } might work here.

+1, came here to say the same, I think it should be

scoped_guard(preempt) {
   ... do all the things with direct return ...
}

/* we got new bucket here */

return stackid_install(...);

>
> But "goto out" is fine with me as well.
>
> Thanks,
> Leon
>
> >
> > jirka
> >
> >>
> >> Thanks,
> >> Leon
> >>
> >>>     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);
> >>> -   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 = {
> >>
>

Re: [PATCHv2 bpf-next 05/11] bpf: Disable preemption in bpf_get_stackid

[Jiri Olsa]

On Thu, Jul 30, 2026 at 03:52:14PM -0700, Andrii Nakryiko wrote:
> On Thu, Jul 30, 2026 at 6:43 AM Leon Hwang <leon.hwang@linux.dev> wrote:
> >
> > On 2026/7/30 19:38, Jiri Olsa wrote:
> > > On Wed, Jul 29, 2026 at 06:30:26PM +0800, Leon Hwang wrote:
> > >> On 29/7/26 16:38, Jiri Olsa wrote:
> > >>> 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 43b1e5436047..e48ddb4edd47 100644
> > >>> --- a/kernel/bpf/stackmap.c
> > >>> +++ b/kernel/bpf/stackmap.c
> > >>> @@ -624,30 +624,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();
> > >>
> > >> Better to use the guard-style guard(preempt)() in this series.
> > >
> > > we prefer to call stackid_install with preemption enabled
> >
> >
> > Understand.
> >
> > I wonder if we could avoid the "goto out".
> >
> > scoped_guard(preempt) { ... } might work here.
> 
> +1, came here to say the same, I think it should be
> 
> scoped_guard(preempt) {
>    ... do all the things with direct return ...
> }
> 
> /* we got new bucket here */
> 
> return stackid_install(...);

looks good, will check on that

thanks,
jirka