Re: [RFC PATCH bpf-next v3 09/12] selftests/bpf: Add tests for memcg_bpf_ops
From: bot+bpf-ci@kernel.org
Date: 2026-01-23 09:19:08
Also in:
bpf, cgroups, linux-kselftest, linux-mm, lkml
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/memcg_ops.c b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c new file mode 100644 index 000000000000..9a8d16296f2d --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/memcg_ops.c
[ ... ]
+static void real_test_memcg_ops(int read_times)
+{
+ int ret;
+ char data_file1[] = "/tmp/test_data_XXXXXX";
+ char data_file2[] = "/tmp/test_data_XXXXXX";
+ char time_file1[] = "/tmp/test_time_XXXXXX";
+ char time_file2[] = "/tmp/test_time_XXXXXX";
+ pid_t pid1, pid2;
+ double time1, time2;[ ... ]
+ pid1 = fork(); + if (!ASSERT_GE(pid1, 0, "fork")) + goto cleanup; + if (pid1 == 0) + real_test_memcg_ops_child_work(CG_LOW_DIR, + data_file1, + time_file1, + read_times); + + pid2 = fork(); + if (!ASSERT_GE(pid1, 0, "fork"))
^^^^ Should this check pid2 instead of pid1? After assigning pid2 = fork(), checking pid1 means a failed second fork() would go undetected if the first fork() succeeded.
+ goto cleanup; + if (pid2 == 0) + real_test_memcg_ops_child_work(CG_HIGH_DIR, + data_file2, + time_file2, + read_times);
[ ... ]
+void test_memcg_ops_over_high(void)
+{
+ int err, map_fd;
+ struct memcg_ops *skel;^^^^^^^^^^^^^^^ The skel variable is not initialized to NULL here. If setup_cgroup() fails early and jumps to the out label, memcg_ops__detach(skel) would be called with an uninitialized pointer. Also, the cleanup path calls memcg_ops__detach() but never calls memcg_ops__destroy(). Would this leak the skeleton object and its associated BPF resources?
+ struct bpf_map *map; + size_t bss_sz; + struct memcg_ops__bss *bss_data; + __u32 key = 0; + struct bpf_program *prog = NULL; + struct bpf_link *link = NULL, *link2 = NULL; + DECLARE_LIBBPF_OPTS(bpf_struct_ops_opts, opts); + int high_cgroup_id, low_cgroup_fd = -1; + + err = setup_cgroup(&high_cgroup_id, &low_cgroup_fd, NULL); + if (!ASSERT_OK(err, "setup_cgroup")) + goto out; + + skel = memcg_ops__open_and_load();
[ ... ]
+out: + bpf_link__destroy(link); + bpf_link__destroy(link2); + memcg_ops__detach(skel); + close(low_cgroup_fd); + cleanup_cgroup_environment(); +}
[ ... ]
+void test_memcg_ops_below_low_over_high(void)
+{
+ int err, map_fd;
+ struct memcg_ops *skel;Same issue here - skel is not initialized to NULL. [ ... ]
+out: + bpf_link__destroy(link); + bpf_link__destroy(link_high); + bpf_link__destroy(link_low); + memcg_ops__detach(skel);
[ ... ]
+void test_memcg_ops_below_min_over_high(void)
+{
+ int err, map_fd;
+ struct memcg_ops *skel;Same issue here as well - skel is not initialized to NULL. [ ... ]
+out: + bpf_link__destroy(link); + bpf_link__destroy(link_high); + bpf_link__destroy(link_low); + memcg_ops__detach(skel); + close(high_cgroup_fd); + close(low_cgroup_fd); + cleanup_cgroup_environment(); +}
--- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/21280790825