Re: [PATCHv2 bpf-next 11/11] bpf: Clear buf on error in __bpf_get_task_stack
Jiri Olsa <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <amykLSjSzayP4WHa@krava> |
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 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 > >