> 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
