Re: [PATCH v4 2/4] selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds check for negative enum values in memcg BPF kfuncs (`bpf_mem_cgroup_memory_events` and `bpf_mem_cgroup_vm_events`), leading to potential out-of-bounds array reads.
--

commit 75dfb2e14e4c5552066e9fdeb8f96969135f991d
Author: Ziyang Men <ziyang.meme@gmail.com>

selftests/cgroup: add memcg_stat_cross_cpu correctness test for flush

Add test_memcg_stat_cross_cpu, which verifies that flushing a memcg
subtree produces the same statistics through the BPF memcg kfuncs and
cgroupfs.

> diff --git a/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> new file mode 100644
> index 0000000000000..c3c69ecd888f0
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> @@ -0,0 +1,95 @@
[ ... ]
> +int memcg_kfuncs_probe(struct bpf_iter__cgroup *ctx)
> +{
> +	struct cgroup *cgrp = ctx->cgroup;
> +	struct mem_cgroup *memcg;
> +
> +	if (!cgrp)
> +		return 0;
> +
> +	memcg = bpf_get_mem_cgroup(&cgrp->self);
> +	if (!memcg)
> +		return 0;
> +
> +	bpf_mem_cgroup_flush_stats(memcg);
> +	bpf_mem_cgroup_page_state(memcg, 0);
> +	bpf_mem_cgroup_vm_events(memcg, 0);

[Severity: High]
This isn't a bug introduced by this patch, but can this kfunc and related
memcg kfuncs be exploited by passing negative enum values to read
out-of-bounds memory?

In mm/bpf_memcontrol.c, the bpf_mem_cgroup_vm_events() kfunc takes an enum
event parameter:

mm/bpf_memcontrol.c:bpf_mem_cgroup_vm_events() {
	if (unlikely(!memcg_vm_event_item_valid(event)))
		return (unsigned long)-1;
	...
}

The helper memcg_vm_event_item_valid() in mm/memcontrol.c uses a signed
comparison that doesn't catch negative values:

mm/memcontrol.c:memcg_vm_event_item_valid() {
	if (idx >= NR_VM_EVENT_ITEMS)
		return false;

	return !BAD_STAT_IDX(memcg_events_index(idx));
}

Similarly, bpf_mem_cgroup_memory_events() lacks a negative check:

mm/bpf_memcontrol.c:bpf_mem_cgroup_memory_events() {
	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))
		return (unsigned long)-1;

	return atomic_long_read(&memcg->memory_events[event]);
}

Since the BPF verifier treats enums as plain integers and doesn't enforce
their valid ranges, a negative index can bypass these validations and lead to
a backward out-of-bounds read from the target array.

> +	bpf_put_mem_cgroup(memcg);
> +	return 0;
> +}

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