Re: [PATCH 6.6.y] bpf: Fix stackmap overflow check in __bpf_get_stackid()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: KASAN OOB write due to missing max_depth < skip check
--

commit 02cac1c9914fa38a62e3ae2d92fc6a6dde2513ff
Author: Arnaud Lecomte <contact@arnaud-lcm.com>

bpf: Fix stackmap overflow check in __bpf_get_stackid()

This backport attempts to fix a KASAN slab-out-of-bounds write by capping the
stack trace depth in __bpf_get_stackid(). Since the helper function
stack_map_calculate_max_depth() is not available in 6.6.y, the logic is
open-coded.

Link: https://lore.kernel.org/bpf/20251025192941.1500-1-contact@arnaud-lcm.com

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index a330f38ae7335..04d873f1a552c 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -225,7 +226,12 @@ static long __bpf_get_stackid(struct bpf_map *map,
>  		/* skipping more than usable stack trace */
>  		return -EFAULT;
>  
> -	trace_nr = trace->nr - skip;
> +	curr_sysctl_max_stack = READ_ONCE(sysctl_perf_event_max_stack);
> +	max_depth = map->value_size / stack_map_data_size(map);
> +	max_depth += skip;
> +	if (max_depth > curr_sysctl_max_stack)
> +		max_depth = curr_sysctl_max_stack;
> +	trace_nr = min_t(u32, trace->nr - skip, max_depth - skip);

[Severity: High]
Does this code risk an unsigned integer underflow when calculating
max_depth - skip?

If sysctl_perf_event_max_stack is lowered concurrently to a value smaller
than skip (which can be up to 255), max_depth will be capped to this smaller
value. The expression max_depth - skip would then underflow to a large
positive value.

This would cause min_t() to select trace->nr - skip, neutralizing the intended
map capacity limit. If trace->nr is populated from pre-recorded perf event
data captured when the sysctl was higher, trace_nr could exceed the bucket
capacity and result in an out-of-bounds memcpy() write onto the heap.

It appears the upstream commit included a check to prevent this underflow,
such as returning -EFAULT if max_depth < skip. Should a similar check be
included in this open-coded backport?

>  	trace_len = trace_nr * sizeof(u64);
>  	ips = trace->ip + skip;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016023225.19034-1-artem@trailofbits.com?part=1