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

[Jiri Olsa <olsajiri@gmail.com>]

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