Re: [PATCHv4 bpf-next 01/12] bpf: Factor stackid_init function from __bpf_get_stackid

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Integer underflow in stack_map_calculate_max_depth causes a heap buffer overflow when sysctl_perf_event_max_stack is concurrently modified.
--

--- Patch [1]: [PATCHv4 bpf-next 01/12] bpf: Factor stackid_init function from __bpf_get_stackid ---
commit 036b334d8e7b51a2339ee19596a116e90ed56c01
Author: Jiri Olsa <[email protected]>

bpf: Factor stackid_init function from __bpf_get_stackid

The new stackid_init function stores all the necessary bits for stackid
trace and it will be used by other functions in following changes.

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c

[ ... ]

> +static int stackid_init(struct stackid *stackid, struct bpf_map *map,
> +			struct perf_callchain_entry *trace, u64 flags)
>  {
>  	struct bpf_stack_map *smap = container_of(map, struct bpf_stack_map, map);
>  	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
> +	u32 max_depth;
>  
>  	if (trace->nr <= skip)
>  		/* skipping more than usable stack trace */
>  		return -EFAULT;
>  
>  	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);

[Severity: High]
This is a pre-existing issue, but can this max_depth - skip calculation
underflow?

If sysctl_perf_event_max_stack is decreased concurrently,
stack_map_calculate_max_depth() can return a max_depth smaller than skip:

kernel/bpf/stackmap.c:stack_map_calculate_max_depth() {
	...
	max_depth += skip;
	if (max_depth > curr_sysctl_max_stack)
		return curr_sysctl_max_stack;
	...
}

This would cause max_depth - skip to underflow to a large integer. If
trace->nr (which may have been captured with the older, larger sysctl) is
greater than skip, it passes the early return check.

The min_t() call would then select trace->nr - skip, which can exceed the
stack map's element capacity and lead to a heap buffer overflow later when
copying entries into the bucket buffer in __bpf_get_stackid():

kernel/bpf/stackmap.c:__bpf_get_stackid() {
	...
		memcpy(new_bucket->data, stackid->ips, stackid->len);
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1