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

[sashiko-bot@kernel.org]

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.

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