Re: [PATCHv2 bpf-next 08/11] bpf: Restore trace->nr value properly in bpf_get_stack_pe

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Jul 30, 2026 at 04:01:28PM -0700, Andrii Nakryiko wrote:
> On Wed, Jul 29, 2026 at 1:39 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > In bpf_get_stack_pe The __bpf_get_stack call changes trace->nr value,
> > so both kernel and user path need to restore its value back.
> >
> 
> something feels off here, this trace_in->nr restoration just in
> bpf_get_stack_pe is weird. maybe we should either make sure
> __bpf_get_stack doesn't modify trace_in->nr or restores it before
> returning to bpf_get_stack_pe()?..

I think it'd be cleaner not to modify the trace->nr and just
pass the wanted count as argument, will check

jirka

> 
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Fixes: e17d62fedd10 ("bpf: Refactor stack map trace depth calculation into helper function")
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  kernel/bpf/stackmap.c | 14 ++++++--------
> >  1 file changed, 6 insertions(+), 8 deletions(-)
> >
> > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > index 05269f2a9e08..875f530f3597 100644
> > --- a/kernel/bpf/stackmap.c
> > +++ b/kernel/bpf/stackmap.c
> > @@ -942,9 +942,9 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> >         struct pt_regs *regs = (struct pt_regs *)(ctx->regs);
> >         struct perf_event *event = ctx->event;
> >         struct perf_callchain_entry *trace;
> > +       __u64 nr, nr_kernel;
> >         bool kernel, user;
> >         int err = -EINVAL;
> > -       __u64 nr_kernel;
> >
> >         if (!(event->attr.sample_type & PERF_SAMPLE_CALLCHAIN))
> >                 return __bpf_get_stack(regs, NULL, NULL, buf, size, flags, false /* !may_fault */);
> > @@ -962,15 +962,12 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> >                 goto clear;
> >
> >         nr_kernel = count_kernel_ip(trace);
> > +       nr = trace->nr;
> >
> >         if (kernel) {
> > -               __u64 nr = trace->nr;
> > -
> >                 trace->nr = nr_kernel;
> >                 err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> >
> > -               /* restore nr */
> > -               trace->nr = nr;
> >         } else { /* user */
> >                 u64 skip = flags & BPF_F_SKIP_FIELD_MASK;
> >
> > @@ -981,12 +978,13 @@ BPF_CALL_4(bpf_get_stack_pe, struct bpf_perf_event_data_kern *, ctx,
> >                 flags = (flags & ~BPF_F_SKIP_FIELD_MASK) | skip;
> >                 err = __bpf_get_stack(regs, NULL, trace, buf, size, flags, false /* !may_fault */);
> >         }
> > -       return err;
> >
> > +       /* restore nr */
> > +       trace->nr = nr;
> >  clear:
> > -       memset(buf, 0, size);
> > +       if (err < 0)
> > +               memset(buf, 0, size);
> >         return err;
> > -
> >  }
> >
> >  const struct bpf_func_proto bpf_get_stack_proto_pe = {
> > --
> > 2.54.0
> >