Re: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The 'size' parameter in callchain_store() is unused.
--

--- Patch [6]: [PATCH bpf-next 6/9] bpf: Factor callchain_store function from __bpf_get_stack ---
commit aee65ba10526fcf4c6320ea660fc50a555551cc1
Author: Jiri Olsa <jolsa@kernel.org>

bpf: Factor callchain_store function from __bpf_get_stack

This commit extracts the logic for storing trace entries into a user-supplied
buffer from __bpf_get_stack() and places it into a new helper function named
callchain_store().

> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
> index a64b14ffd1e8e..b990643f25cd8 100644
> --- a/kernel/bpf/stackmap.c
> +++ b/kernel/bpf/stackmap.c
[ ... ]
> @@ -747,6 +747,29 @@ const struct bpf_func_proto bpf_get_stackid_proto_pe = {
>  	.arg3_type	= ARG_ANYTHING,
>  };
>  
> +static u32 callchain_store(struct perf_callchain_entry *trace, void *buf, u32 size,
> +			   u32 elem_size, u64 flags)

[Severity: Low]
This isn't a bug, but the 'size' parameter doesn't appear to be used anywhere
within callchain_store() in kernel/bpf/stackmap.c. Could it be safely removed
from the function signature since the bounds checking is handled in the caller?

> +{
> +	bool user_build_id = flags & BPF_F_USER_BUILD_ID;
> +	u32 skip = flags & BPF_F_SKIP_FIELD_MASK;
> +	u32 trace_nr, copy_len;
> +	u64 *ips;
> +
> +	trace_nr = trace->nr - skip;
> +	copy_len = trace_nr * elem_size;
> +
> +	ips = trace->ip + skip;
> +	if (user_build_id) {
> +		struct bpf_stack_build_id *id_offs = buf;
> +
> +		for (u32 i = 0; i < trace_nr; i++)
> +			id_offs[i].ip = ips[i];
> +	} else {
> +		memcpy(buf, ips, copy_len);
> +	}
> +	return trace_nr;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720085351.655075-1-jolsa@kernel.org?part=6