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

[Brahmajit Das <listout@listout.xyz>]

On 13.10.2026 14:49, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: missing negative bounds checks in memcg event kfuncs allow OOB reads
> --
> 
> commit fbb677e1e6cb5f158ac286d23535b8f06e966597
> Author: Brahmajit Das <bdas@suse.de>
> 
> selftests/bpf: Add tests for memcg kfuncs
> 
> The existing cgroup_iter_memcg test covers bpf_get_mem_cgroup(),
> bpf_put_mem_cgroup(), bpf_mem_cgroup_flush_stats(),
> bpf_mem_cgroup_page_state() and bpf_mem_cgroup_vm_events(), but
> bpf_get_root_mem_cgroup(), bpf_mem_cgroup_usage(),
> bpf_mem_cgroup_memory_events() and bpf_proactive_reclaim() have no
> selftest coverage.
> 
> Add a memcg_kfuncs test that drives them from syscall programs against
> a test cgroup.
> 
> > diff --git a/tools/testing/selftests/bpf/progs/memcg_kfuncs.c b/tools/testing/selftests/bpf/progs/memcg_kfuncs.c
> > new file mode 100644
> > index 0000000000000..3c7866730aa0c
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/memcg_kfuncs.c
> > @@ -0,0 +1,101 @@
> 
> [ ... ]
> 
> > +SEC("syscall")
> > +int memcg_query(void *ctx)
> > +{
> > +	struct mem_cgroup *memcg;
> > +
> > +	memcg = memcg_from_cgid();
> > +	if (!memcg)
> > +		return 1;
> 
> [ ... ]
> 
> > +	/* 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);
> 
> [Severity: High]
> Does this test expose an incomplete bounds check in the underlying kfuncs?
> 
> The BPF verifier treats enum arguments as signed scalars and does not
> restrict them to valid enum ranges, so BPF programs can pass negative values.
> 
> In mm/bpf_memcontrol.c:bpf_mem_cgroup_memory_events(), the signed check
> event >= MEMCG_NR_MEMORY_EVENTS is bypassed by negative values like -1.
> The function then indexes memcg->memory_events[event], leading to an
> out-of-bounds kernel memory read.
> 
> Similarly, bpf_mem_cgroup_vm_events() delegates to memcg_vm_event_item_valid()
> in mm/memcontrol.c, which checks idx >= NR_VM_EVENT_ITEMS. A negative index
> bypasses this and accesses mem_cgroup_events_index[idx] out of bounds.
> 
> Could malicious or unprivileged BPF programs exploit this to read
> out-of-bounds kernel memory?
> 
> > +
> > +	bpf_put_mem_cgroup(memcg);
> > +	return 0;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261013143853.1379307-1-listout@listout.xyz?part=1

I believe this is a false positive. enum memcg_memory_event and enum
vm_event_item have no negative enumerators, so their underlying type is
unsigned int and both bounds checks are unsigned comparisons. A negative
value passed from BPF is seen as a large unsigned index and rejected
with (unsigned long)-1.

The generated code (x86_64, bpf-next) confirms it:

  bpf_mem_cgroup_memory_events:
      cmp    $0x9,%esi
      ja     <return -1>

  memcg_vm_event_item_valid:
      cmp    $0x67,%edi
      ja     <return false>

Both use ja (unsigned), not jg. bpf_mem_cgroup_page_state() already
casts to u32 explicitly, and the test passes it -1.

That said, it is a cheap case to cover, so v2 will also pass -1 to
bpf_mem_cgroup_memory_events() and bpf_mem_cgroup_vm_events() and check
that both return (unsigned long)-1. 

-- 
Regards,
listout