Re: [PATCH v5 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] Kfuncs taking enum parameters lack negative bounds checks, allowing out-of-bounds array accesses.
--

commit ba61caf79f3e8d8099a1b75ad1df31bbdc5f2942
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..fff3b9256aa39
> --- /dev/null
> +++ b/tools/testing/selftests/cgroup/memcg_stat_cross_cpu.bpf.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +#define BPF_NO_KFUNC_PROTOTYPES
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_core_read.h>
> +#include "memcg_stat_cross_cpu.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +/* Ensure the upper-bound checks also reject negative BPF arguments. */
> +_Static_assert((enum vm_event_item)-1 >= NR_VM_EVENT_ITEMS,
> +	       "negative vm_event_item must fail the bounds check");
> +_Static_assert((enum memcg_memory_event)-1 >= MEMCG_NR_MEMORY_EVENTS,
> +	       "negative memcg_memory_event must fail the bounds check");

[Severity: High]
This is a pre-existing issue, but while these static asserts guarantee the BPF
compiler evaluates the check correctly, does the host kernel kfunc itself
still lack negative bounds checks?

Looking at bpf_mem_cgroup_memory_events() in mm/bpf_memcontrol.c:

	if (unlikely(event >= MEMCG_NR_MEMORY_EVENTS))
		return (unsigned long)-1;

Since the BPF verifier does not validate enum ranges, a BPF program can pass a
negative value as the enum parameter. Because enums can be treated as signed
integers in C, wouldn't a negative value bypass this upper-bound check (e.g.
-1 >= MEMCG_NR_MEMORY_EVENTS evaluates to false), leading to an out-of-bounds
array access?

A similar issue appears to exist in bpf_mem_cgroup_vm_events() which calls
memcg_vm_event_item_valid() in mm/memcontrol.c:

	if (idx >= NR_VM_EVENT_ITEMS)
		return false;

Could this allow an out-of-bounds array read that leaks kernel memory or
causes a panic?

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