* [PATCH bpf-next] selftests/bpf: Add tests for memcg kfuncs
@ 2026-10-13 14:38 listout
2026-10-13 14:49 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: listout @ 2026-10-13 14:38 UTC (permalink / raw)
To: bpf
Cc: bdas, Andrii Nakryiko, Eduard Zingerman, Ihor Solodrai,
Alexei Starovoitov, Daniel Borkmann, Kumar Kartikeya Dwivedi,
Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
Emil Tsalapatis, Shuah Khan, Roman Gushchin, Shakeel Butt,
JP Kobryn, linux-kselftest, linux-kernel
From: Brahmajit Das <bdas@suse.de>
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:
- usage: bpf_mem_cgroup_usage() agrees with memory.current.
- invalid_idx: out-of-range indices passed to
bpf_mem_cgroup_memory_events(), bpf_mem_cgroup_vm_events() and
bpf_mem_cgroup_page_state() return (unsigned long)-1.
- memory_events: after pushing the cgroup over memory.high,
MEMCG_HIGH is non-zero and the low/high/max/oom/oom_kill counters
match memory.events.
- root_memcg: bpf_get_root_mem_cgroup() can be acquired and released.
- reclaim_args: bpf_proactive_reclaim() rejects sizes below PAGE_SIZE
and out-of-range swappiness with -EINVAL, and accepts -1, 0,
MAX_SWAPPINESS and SWAPPINESS_ANON_ONLY.
- reclaim: bpf_proactive_reclaim() reclaims clean page cache and the
result is clamped to MEMCG_CHARGE_BATCH pages.
The page cache subtests skip when the working directory cannot back
a reclaimable O_TMPFILE, as in arena_memcg.
Also add memcg_kfuncs_failure with verifier tests for a leaked
reference from bpf_get_root_mem_cgroup(), a missing NULL check on its
result, and a call to bpf_proactive_reclaim() from a non-syscall
program.
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Brahmajit Das <bdas@suse.de>
---
.../selftests/bpf/prog_tests/memcg_kfuncs.c | 300 ++++++++++++++++++
.../selftests/bpf/progs/memcg_kfuncs.c | 101 ++++++
.../bpf/progs/memcg_kfuncs_failure.c | 54 ++++
3 files changed, 455 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/memcg_kfuncs.c
create mode 100644 tools/testing/selftests/bpf/progs/memcg_kfuncs.c
create mode 100644 tools/testing/selftests/bpf/progs/memcg_kfuncs_failure.c
diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_kfuncs.c b/tools/testing/selftests/bpf/prog_tests/memcg_kfuncs.c
new file mode 100644
index 000000000000..d5ab7e64ffe5
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/memcg_kfuncs.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <test_progs.h>
+#include <fcntl.h>
+#include <linux/magic.h>
+#include <sys/mman.h>
+#include <sys/vfs.h>
+#include <unistd.h>
+
+#include "cgroup_helpers.h"
+#include "memcg_kfuncs.skel.h"
+#include "memcg_kfuncs_failure.skel.h"
+
+#define CG_PATH "/memcg_kfuncs"
+
+/* From mm/internal.h, not exposed in BTF. */
+#define MAX_SWAPPINESS 200
+#define SWAPPINESS_ANON_ONLY (MAX_SWAPPINESS + 1)
+
+/* bpf_proactive_reclaim() clamps @size to MEMCG_CHARGE_BATCH pages. */
+#define MEMCG_CHARGE_BATCH 64
+
+#define ANON_SIZE (4 * 1024 * 1024)
+#define USAGE_SLACK (1 * 1024 * 1024)
+#define PAGECACHE_SIZE (32 * 1024 * 1024)
+#define HIGH_HEADROOM (1 * 1024 * 1024)
+#define RECLAIM_RETRIES 16
+
+static long page_size;
+
+static int run_prog(struct bpf_program *prog)
+{
+ LIBBPF_OPTS(bpf_test_run_opts, opts);
+ int err;
+
+ err = bpf_prog_test_run_opts(bpf_program__fd(prog), &opts);
+ if (!ASSERT_OK(err, "bpf_prog_test_run_opts"))
+ return -1;
+ if (!ASSERT_EQ(opts.retval, 0, "retval"))
+ return -1;
+ return 0;
+}
+
+static int read_memcg_ulong(const char *file, unsigned long *val)
+{
+ char buf[64];
+
+ if (read_cgroup_file(CG_PATH, file, buf, sizeof(buf)))
+ return -1;
+ *val = strtoul(buf, NULL, 10);
+ return 0;
+}
+
+static int read_memory_event(const char *key, unsigned long *val)
+{
+ char buf[512], name[64], *line, *saveptr;
+ unsigned long v;
+
+ if (read_cgroup_file(CG_PATH, "memory.events", buf, sizeof(buf)))
+ return -1;
+
+ for (line = strtok_r(buf, "\n", &saveptr); line;
+ line = strtok_r(NULL, "\n", &saveptr)) {
+ if (sscanf(line, "%63s %lu", name, &v) != 2)
+ continue;
+ if (!strcmp(name, key)) {
+ *val = v;
+ return 0;
+ }
+ }
+ return -1;
+}
+
+/*
+ * Fill the page cache with @size bytes of clean, reclaimable pages by
+ * reading a sparse temp file. Returns the fd, which must stay open, or
+ * -EOPNOTSUPP if the working directory cannot back such a file, -1 on error.
+ */
+static int alloc_pagecache(size_t size)
+{
+ struct statfs stfs;
+ char buf[4096];
+ size_t off;
+ int fd;
+
+ fd = open(".", O_TMPFILE | O_RDWR | O_EXCL, 0600);
+ if (fd < 0)
+ return errno == EOPNOTSUPP ? -EOPNOTSUPP : -1;
+ /* tmpfs hands out shmem pages, which are not reclaimable without swap */
+ if (fstatfs(fd, &stfs) || stfs.f_type == TMPFS_MAGIC) {
+ close(fd);
+ return -EOPNOTSUPP;
+ }
+ if (ftruncate(fd, size))
+ goto err;
+ for (off = 0; off < size; off += sizeof(buf))
+ if (read(fd, buf, sizeof(buf)) < 0)
+ goto err;
+ return fd;
+err:
+ close(fd);
+ return -1;
+}
+
+static int alloc_pagecache_or_skip(size_t size)
+{
+ int fd;
+
+ fd = alloc_pagecache(size);
+ if (fd == -EOPNOTSUPP) {
+ test__skip();
+ return -1;
+ }
+ ASSERT_OK_FD(fd, "alloc_pagecache");
+ return fd;
+}
+
+static void test_usage(struct memcg_kfuncs *skel)
+{
+ unsigned long before, after, lo, hi;
+ void *map;
+
+ map = mmap(NULL, ANON_SIZE, PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+ if (!ASSERT_NEQ(map, MAP_FAILED, "mmap anon"))
+ return;
+ memset(map, 1, ANON_SIZE);
+
+ if (!ASSERT_OK(read_memcg_ulong("memory.current", &before), "read before"))
+ goto out;
+ if (run_prog(skel->progs.memcg_query))
+ goto out;
+ if (!ASSERT_OK(read_memcg_ulong("memory.current", &after), "read after"))
+ goto out;
+
+ /*
+ * bpf_mem_cgroup_usage() and memory.current read the same counter,
+ * but the cgroup may still be charged or uncharged in between.
+ */
+ lo = (before < after ? before : after) - USAGE_SLACK;
+ hi = (before < after ? after : before) + USAGE_SLACK;
+ ASSERT_GE(skel->bss->usage, ANON_SIZE, "usage covers anon");
+ ASSERT_GE(skel->bss->usage, lo, "usage lower bound");
+ ASSERT_LE(skel->bss->usage, hi, "usage upper bound");
+out:
+ munmap(map, ANON_SIZE);
+}
+
+static void test_invalid_idx(struct memcg_kfuncs *skel)
+{
+ if (run_prog(skel->progs.memcg_query))
+ return;
+
+ ASSERT_EQ(skel->bss->bad_memory_event, -1UL, "bad memory event");
+ ASSERT_EQ(skel->bss->bad_vm_event, -1UL, "bad vm event");
+ ASSERT_EQ(skel->bss->bad_page_state, -1UL, "bad page state");
+}
+
+static void test_memory_events(struct memcg_kfuncs *skel)
+{
+ unsigned long cur, val;
+ char buf[64];
+ int fd;
+
+ if (!ASSERT_OK(read_memcg_ulong("memory.current", &cur), "memory.current"))
+ return;
+
+ /*
+ * Push the cgroup over memory.high to raise MEMCG_HIGH. Every charge
+ * above the limit reclaims, so only overshoot it slightly.
+ */
+ snprintf(buf, sizeof(buf), "%lu", cur + HIGH_HEADROOM);
+ if (!ASSERT_OK(write_cgroup_file(CG_PATH, "memory.high", buf), "set memory.high"))
+ return;
+ fd = alloc_pagecache_or_skip(2 * HIGH_HEADROOM);
+ write_cgroup_file(CG_PATH, "memory.high", "max");
+ if (fd < 0)
+ return;
+
+ if (run_prog(skel->progs.memcg_query))
+ goto out;
+
+ ASSERT_GT(skel->bss->ev_high, 0, "high event raised");
+
+ if (ASSERT_OK(read_memory_event("low", &val), "read low"))
+ ASSERT_EQ(skel->bss->ev_low, val, "low");
+ if (ASSERT_OK(read_memory_event("high", &val), "read high"))
+ ASSERT_EQ(skel->bss->ev_high, val, "high");
+ if (ASSERT_OK(read_memory_event("max", &val), "read max"))
+ ASSERT_EQ(skel->bss->ev_max, val, "max");
+ if (ASSERT_OK(read_memory_event("oom", &val), "read oom"))
+ ASSERT_EQ(skel->bss->ev_oom, val, "oom");
+ if (ASSERT_OK(read_memory_event("oom_kill", &val), "read oom_kill"))
+ ASSERT_EQ(skel->bss->ev_oom_kill, val, "oom_kill");
+out:
+ close(fd);
+}
+
+static void test_root_memcg(struct memcg_kfuncs *skel)
+{
+ if (run_prog(skel->progs.root_memcg_query))
+ return;
+
+ ASSERT_GT(skel->bss->root_usage, 0, "root usage");
+}
+
+static long do_reclaim(struct memcg_kfuncs *skel, unsigned long size,
+ int swappiness)
+{
+ skel->bss->reclaim_size = size;
+ skel->bss->reclaim_swappiness = swappiness;
+ skel->bss->reclaim_ret = 0;
+ if (run_prog(skel->progs.memcg_reclaim))
+ return LONG_MIN;
+ return skel->bss->reclaim_ret;
+}
+
+static void test_reclaim_args(struct memcg_kfuncs *skel)
+{
+ ASSERT_EQ(do_reclaim(skel, 0, -1), -EINVAL, "size 0");
+ ASSERT_EQ(do_reclaim(skel, page_size - 1, -1), -EINVAL, "size < PAGE_SIZE");
+ ASSERT_EQ(do_reclaim(skel, page_size, -2), -EINVAL, "swappiness -2");
+ ASSERT_EQ(do_reclaim(skel, page_size, SWAPPINESS_ANON_ONLY + 1), -EINVAL,
+ "swappiness too large");
+
+ ASSERT_GE(do_reclaim(skel, page_size, -1), 0, "swappiness -1");
+ ASSERT_GE(do_reclaim(skel, page_size, 0), 0, "swappiness 0");
+ ASSERT_GE(do_reclaim(skel, page_size, MAX_SWAPPINESS), 0, "swappiness max");
+ ASSERT_GE(do_reclaim(skel, page_size, SWAPPINESS_ANON_ONLY), 0,
+ "swappiness anon only");
+}
+
+static void test_reclaim(struct memcg_kfuncs *skel)
+{
+ long ret = 0;
+ int fd, i;
+
+ fd = alloc_pagecache_or_skip(PAGECACHE_SIZE);
+ if (fd < 0)
+ return;
+
+ /* Ask for more than one batch to check the clamp. */
+ for (i = 0; i < RECLAIM_RETRIES && ret <= 0; i++)
+ ret = do_reclaim(skel, 4 * MEMCG_CHARGE_BATCH * page_size, 0);
+
+ ASSERT_GT(ret, 0, "reclaimed");
+ ASSERT_LE(ret, MEMCG_CHARGE_BATCH * page_size, "reclaim clamped");
+ ASSERT_EQ(ret % page_size, 0, "reclaim page aligned");
+
+ close(fd);
+}
+
+void test_memcg_kfuncs(void)
+{
+ struct memcg_kfuncs *skel = NULL;
+ unsigned long cur;
+ int cgroup_fd;
+
+ page_size = sysconf(_SC_PAGESIZE);
+
+ cgroup_fd = cgroup_setup_and_join(CG_PATH);
+ if (!ASSERT_OK_FD(cgroup_fd, "cgroup_setup_and_join"))
+ return;
+
+ if (read_memcg_ulong("memory.current", &cur)) {
+ test__skip();
+ goto out;
+ }
+
+ skel = memcg_kfuncs__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "memcg_kfuncs__open_and_load"))
+ goto out;
+
+ skel->bss->cgid = get_cgroup_id(CG_PATH);
+ if (!ASSERT_NEQ(skel->bss->cgid, 0, "get_cgroup_id"))
+ goto out;
+
+ if (test__start_subtest("usage"))
+ test_usage(skel);
+ if (test__start_subtest("invalid_idx"))
+ test_invalid_idx(skel);
+ if (test__start_subtest("memory_events"))
+ test_memory_events(skel);
+ if (test__start_subtest("root_memcg"))
+ test_root_memcg(skel);
+ if (test__start_subtest("reclaim_args"))
+ test_reclaim_args(skel);
+ if (test__start_subtest("reclaim"))
+ test_reclaim(skel);
+
+out:
+ memcg_kfuncs__destroy(skel);
+ close(cgroup_fd);
+ cleanup_cgroup_environment();
+}
+
+void test_memcg_kfuncs_failure(void)
+{
+ RUN_TESTS(memcg_kfuncs_failure);
+}
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 000000000000..3c7866730aa0
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/memcg_kfuncs.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
+void bpf_cgroup_release(struct cgroup *cgrp) __ksym;
+
+/* in */
+u64 cgid;
+unsigned long reclaim_size;
+int reclaim_swappiness;
+
+/* out */
+unsigned long usage;
+unsigned long ev_low, ev_high, ev_max, ev_oom, ev_oom_kill;
+unsigned long bad_memory_event;
+unsigned long bad_vm_event;
+unsigned long bad_page_state;
+unsigned long root_usage;
+long reclaim_ret;
+
+static struct mem_cgroup *memcg_from_cgid(void)
+{
+ struct mem_cgroup *memcg;
+ struct cgroup *cgrp;
+
+ cgrp = bpf_cgroup_from_id(cgid);
+ if (!cgrp)
+ return NULL;
+
+ memcg = bpf_get_mem_cgroup(&cgrp->self);
+ bpf_cgroup_release(cgrp);
+ return memcg;
+}
+
+SEC("syscall")
+int memcg_query(void *ctx)
+{
+ struct mem_cgroup *memcg;
+
+ memcg = memcg_from_cgid();
+ if (!memcg)
+ return 1;
+
+ usage = bpf_mem_cgroup_usage(memcg);
+
+ ev_low = bpf_mem_cgroup_memory_events(memcg,
+ bpf_core_enum_value(enum memcg_memory_event, MEMCG_LOW));
+ ev_high = bpf_mem_cgroup_memory_events(memcg,
+ bpf_core_enum_value(enum memcg_memory_event, MEMCG_HIGH));
+ ev_max = bpf_mem_cgroup_memory_events(memcg,
+ bpf_core_enum_value(enum memcg_memory_event, MEMCG_MAX));
+ ev_oom = bpf_mem_cgroup_memory_events(memcg,
+ bpf_core_enum_value(enum memcg_memory_event, MEMCG_OOM));
+ ev_oom_kill = bpf_mem_cgroup_memory_events(memcg,
+ bpf_core_enum_value(enum memcg_memory_event, MEMCG_OOM_KILL));
+
+ /* 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);
+
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
+
+SEC("syscall")
+int root_memcg_query(void *ctx)
+{
+ struct mem_cgroup *memcg;
+
+ memcg = bpf_get_root_mem_cgroup();
+ if (!memcg)
+ return 1;
+
+ root_usage = bpf_mem_cgroup_usage(memcg);
+
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
+
+SEC("syscall")
+int memcg_reclaim(void *ctx)
+{
+ struct mem_cgroup *memcg;
+
+ memcg = memcg_from_cgid();
+ if (!memcg)
+ return 1;
+
+ reclaim_ret = bpf_proactive_reclaim(memcg, reclaim_size,
+ reclaim_swappiness);
+
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/memcg_kfuncs_failure.c b/tools/testing/selftests/bpf/progs/memcg_kfuncs_failure.c
new file mode 100644
index 000000000000..3fe9f5a14707
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/memcg_kfuncs_failure.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+
+#include "bpf_misc.h"
+
+char _license[] SEC("license") = "GPL";
+
+SEC("syscall")
+__failure __msg("Unreleased reference")
+int root_memcg_leak(void *ctx)
+{
+ struct mem_cgroup *memcg;
+
+ memcg = bpf_get_root_mem_cgroup();
+ if (!memcg)
+ return 0;
+
+ /* Missing bpf_put_mem_cgroup(). */
+ return 0;
+}
+
+SEC("syscall")
+__failure __msg("Possibly NULL pointer passed to trusted R1")
+int root_memcg_no_null_check(void *ctx)
+{
+ struct mem_cgroup *memcg;
+
+ memcg = bpf_get_root_mem_cgroup();
+ /* bpf_get_root_mem_cgroup() is KF_RET_NULL. */
+ bpf_mem_cgroup_usage(memcg);
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
+
+SEC("iter.s/cgroup")
+__failure __msg("calling kernel function bpf_proactive_reclaim is not allowed")
+int proactive_reclaim_not_syscall(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;
+
+ /* Only BPF_PROG_TYPE_SYSCALL may reclaim. */
+ bpf_proactive_reclaim(memcg, 4096, -1);
+ bpf_put_mem_cgroup(memcg);
+ return 0;
+}
--
2.56.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add tests for memcg kfuncs
2026-10-13 14:38 [PATCH bpf-next] selftests/bpf: Add tests for memcg kfuncs listout
@ 2026-10-13 14:49 ` sashiko-bot
2026-10-13 15:00 ` Brahmajit Das
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-10-13 14:49 UTC (permalink / raw)
To: listout, bpf
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: Add tests for memcg kfuncs
2026-10-13 14:49 ` sashiko-bot
@ 2026-10-13 15:00 ` Brahmajit Das
0 siblings, 0 replies; 3+ messages in thread
From: Brahmajit Das @ 2026-10-13 15:00 UTC (permalink / raw)
To: sashiko-reviews; +Cc: bpf
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-13 15:00 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 14:38 [PATCH bpf-next] selftests/bpf: Add tests for memcg kfuncs listout
2026-10-13 14:49 ` sashiko-bot
2026-10-13 15:00 ` Brahmajit Das
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.