Re: [PATCH bpf-next v1 2/2] bpf: Zero kfunc arg meta before error paths can read it

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Wed Jul 15, 2026 at 7:38 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] BPF kfuncs bpf_mem_cgroup_memory_events and bpf_mem_cgroup_vm_events do not handle negative enum values, bypassing bounds checks and allowing out-of-bounds array reads.
> --
>
> --- Patch [2]: [PATCH bpf-next v1 2/2] bpf: Zero kfunc arg meta before error paths can read it ---
> commit 53f60dae3990fbb436a03cb878e7564339ccd1e9
> Author: Amery Hung <ameryhung@gmail.com>
>
> bpf: Zero kfunc arg meta before error paths can read it
>
> check_kfunc_call() reads meta.func_name when bpf_fetch_kfunc_arg_meta()
> returns -EACCES, but that error can come from fetch_kfunc_meta() (e.g.
> fd_array_get_btf() rejecting BTF binding for a signed program) before
> meta is memset(), leaving it uninitialized and risking a garbage deref
> in verbose().
>
> Move the memset() to the start of bpf_fetch_kfunc_arg_meta() so meta is
> zeroed on every error return. The intended "not allowed" -EACCES path
> still sets func_name first, so its message is unchanged.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index c87e5fec5a857..ddfa4f57016b7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> [Severity: High]
> This isn't a bug introduced by this patch, but this is a pre-existing issue.
> Do the bounds checks in bpf_mem_cgroup_memory_events() and
> bpf_mem_cgroup_vm_events() properly handle negative values?
>
> Since enums evaluate to signed integers in C, and the BPF verifier does
> not enforce enum ranges, a BPF program could pass a negative value to
> bpf_mem_cgroup_memory_events() in mm/bpf_memcontrol.c:
>
> 	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))
> 		return (unsigned long)-1;
>
> 	return atomic_long_read(&memcg->memory_events[event]);
>
> For a negative value, the bounds check above evaluates to false, which
> allows an out-of-bounds read on the memory_events array.
>
> Can a similar out-of-bounds read happen in bpf_mem_cgroup_vm_events()?
>
> 	if (unlikely(!memcg_vm_event_item_valid(event)))
> 		return (unsigned long)-1;
>

+Cc Roman

I think we should just do:

	if (unlikely(event < 0 || !memcg_vm_event_item_valid(event)))
 		return (unsigned long)-1;

as fix?

> 	return memcg_events(memcg, event);
>
> The validation routine in mm/memcontrol.c uses the same signed comparison:
>
> bool memcg_vm_event_item_valid(enum vm_event_item idx)
> {
> 	if (idx >= NR_VM_EVENT_ITEMS)
> 		return false;
>
> 	return !BAD_STAT_IDX(memcg_events_index(idx));
> }
>
> This allows negative index values to reach memcg_events_index():
>
> static inline int memcg_events_index(enum vm_event_item idx)
> {
> 	return mem_cgroup_events_index[idx];
> }
>
> Could this allow a BPF program to read arbitrary out-of-bounds kernel memory?