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

[sashiko-bot@kernel.org]

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;

	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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715172127.2416388-1-ameryhung@gmail.com?part=2