netdev - Re: [RFC PATCH bpf-next v3 09/12] selftests/bpf: Add tests for memcg_bpf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <9ab6aaa78c0dcba8b563eab2807eccb0c24bd14fa02d7417cff4284204de7d2d@mail.kernel.org>
Date: Fri, 23 Jan 2026 09:19:07 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: hui.zhu@...ux.dev,akpm@...ux-foundation.org,hannes@...xchg.org,mhocko@...nel.org,roman.gushchin@...ux.dev,shakeel.butt@...ux.dev,muchun.song@...ux.dev,ast@...nel.org,daniel@...earbox.net,andrii@...nel.org,martin.lau@...ux.dev,eddyz87@...il.com,song@...nel.org,yonghong.song@...ux.dev,john.fastabend@...il.com,kpsingh@...nel.org,sdf@...ichev.me,haoluo@...gle.com,jolsa@...nel.org,shuah@...nel.org,peterz@...radead.org,ojeda@...nel.org,nathan@...nel.org,kees@...nel.org,tj@...nel.org,jeffxu@...omium.org,mkoutny@...e.com,kernel@...rr.cc,brauner@...nel.org,rdunlap@...radead.org,brgerst@...il.com,masahiroy@...nel.org,davem@...emloft.net,kuba@...nel.org,hawk@...nel.org,inwardvessel@...il.com,willemb@...gle.com,kerneljasonxing@...il.com,paul.chaignon@...il.com,a.s.protopopov@...il.com,ameryhung@...il.com,chenridong@...weicloud.com,lance.yang@...ux.dev,jiayuan.chen@...ux.dev,linux-kernel@...r.kernel.org,linux-mm@...ck.org,cgroups@...r.kernel.org,bpf@...r.kernel.org,netdev@...r.kernel.org,linu
 x-kselftest@...r.kernel.org
Cc: zhuhui@...inos.cn,geliang@...nel.org,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [RFC PATCH bpf-next v3 09/12] selftests/bpf: Add tests for memcg_bpf_ops

> 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