Re: [PATCH bpf-next v2] selftests/bpf: Add tests for memcg kfuncs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: Missing negative bounds check in bpf_mem_cgroup_*_events() allows OOB read
--

commit a4794021755b1042b63dca630dda69e60c099827
Author: Brahmajit Das <bdas@suse.de>

selftests/bpf: Add tests for memcg kfuncs

This patch adds new selftest coverage for various memcg kfuncs,
including bpf_mem_cgroup_usage(), bpf_mem_cgroup_memory_events(),
and bpf_proactive_reclaim(). It validates their behavior against
a test cgroup from syscall programs.

Link: https://lore.kernel.org/bpf/20261013143853.1379307-1-listout@listout.xyz/

> diff --git a/tools/testing/selftests/bpf/progs/memcg_kfuncs.c b/tools/testing/selftests/bpf/progs/memcg_kfuncs.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/memcg_kfuncs.c
[ ... ]
> @@ -0,0 +1,106 @@
[ ... ]
> +SEC("syscall")
> +int memcg_query(void *ctx)
> +{
[ ... ]
> +	/* Out-of-range indices must be rejected with (unsigned long)-1. */
> +	bad_memory_event = bpf_mem_cgroup_memory_events(memcg,
> +		bpf_core_enum_value(enum memcg_memory_event, MEMCG_NR_MEMORY_EVENTS));
> +	bad_vm_event = bpf_mem_cgroup_vm_events(memcg,
> +		bpf_core_enum_value(enum vm_event_item, NR_VM_EVENT_ITEMS));
> +	bad_page_state = bpf_mem_cgroup_page_state(memcg, -1);
> +	/* Negative indices must not bypass the bounds checks. */
> +	neg_memory_event = bpf_mem_cgroup_memory_events(memcg, -1);
> +	neg_vm_event = bpf_mem_cgroup_vm_events(memcg, -1);

[Severity: High]
Can this test sequence inadvertently trigger an out-of-bounds read regression
in the kernel?

The BPF verifier does not enforce bounds for enum arguments, so a negative
value of -1 is passed directly to the kernel kfunc. Looking at
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]);

Because event is a signed enum, the bounds check uses a signed comparison.
Passing -1 evaluates as less than MEMCG_NR_MEMORY_EVENTS, bypassing the check,
and proceeds to access the memory_events array with a negative index.

A similar pattern appears to exist for bpf_mem_cgroup_vm_events() which routes
through memcg_vm_event_item_valid() in mm/memcontrol.c and performs a signed
comparison against NR_VM_EVENT_ITEMS.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013151621.1526699-1-listout@listout.xyz?part=1