Re: [PATCHv2 bpf-next 11/11] bpf: Clear buf on error in __bpf_get_task_stack

On Fri, Jul 31, 2026 at 6:33 AM Jiri Olsa <[email protected]> wrote:
>
> On Thu, Jul 30, 2026 at 04:11:02PM -0700, Andrii Nakryiko wrote:
> > On Wed, Jul 29, 2026 at 1:40 AM Jiri Olsa <[email protected]> wrote:
> > >
> > > Both bpf_get_task_stack and bpf_get_task_stack helpers that use
> > > __bpf_get_task_stack have buf defined as ARG_PTR_TO_UNINIT_MEM
> > > argument, and we should initialize the buf on every return path.
> > >
> >
> > why "should"? what's the point to initialize it to all zeroes if we
> > failed to get stack trace? we shouldn't allow grabbing stack trace
> > without CAP_PERFMON, and with CAP_PERFMON we shouldn't be worried
> > about "leaking kernel memory" because CAP_PERFMON is plenty privileged
> > and allows to access any kernel memory.
>
> hum __bpf_get_stack already clears buf on error, so I did not question it ;-)
>
> there's this comment:
>
>         /* Pointer to memory does not need to be initialized, since helper function
>          * fills all bytes or clears them in error case.
>          */
>         ARG_PTR_TO_UNINIT_MEM           = MEM_UNINIT | MEM_WRITE | ARG_PTR_TO_MEM,
>
> IIUC from verifier POV bpf_get_task_stack switches un-initialized buffer to
> initialized regardless of the returned error and such buffer could be then
> passed to another helper that allows only initialized buffer
>

We used to not allow to read stack slots that were not initialized,
but then we changed that and we treat uninitialized stack slot as just
unknown scalar, so it wouldn't be a problem.

But for consistency, let's clear it. Looking at the code some more, we
set the "tail" of buffer on success with zeroes as well, which makes
it possible to use buffer as hashmap key (deterministic zeroes at the
end will result in the same key for the same stack trace), so ack for
the change, but motivation is not preventing reading uninitialized
memory, it's more the usability angle.

> jirka
>
> >
> > or am I missing more reasoning behind this change?
> >
> > > Adding missing buf memset for __bpf_get_task_stack fail paths.
> > > The __bpf_get_stack call does clear the buf properly.
> > >
> > > Fixes: 06ab134ce8ec ("bpf: Refcount task stack in bpf_get_task_stack")
> > > Fixes: b992f01e6615 ("bpf: Guard against accessing NULL pt_regs in bpf_get_task_stack()")
> > > Reported-by: Sashiko <[email protected]>
> > > Signed-off-by: Jiri Olsa <[email protected]>
> > > ---
> > >  kernel/bpf/stackmap.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> > > index f4827afbfed9..9ab0c2523a41 100644
> > > --- a/kernel/bpf/stackmap.c
> > > +++ b/kernel/bpf/stackmap.c
> > > @@ -890,14 +890,17 @@ static long __bpf_get_task_stack(struct task_struct *task, void *buf, u32 size,
> > >         struct pt_regs *regs;
> > >         long res = -EINVAL;
> > >
> > > -       if (!try_get_task_stack(task))
> > > +       if (!try_get_task_stack(task)) {
> > > +               memset(buf, 0, size);
> > >                 return -EFAULT;
> > > +       }
> > >
> > >         regs = task_pt_regs(task);
> > >         if (regs)
> > >                 res = __bpf_get_stack(regs, task, buf, size, flags, may_fault);
> > > +       else
> > > +               memset(buf, 0, size);
> > >         put_task_stack(task);
> > > -
> > >         return res;
> > >  }
> > >
> > > --
> > > 2.54.0
> > >