linux-kernel - Re: [PATCH bpf-next v1 6/6] bpf: selftests: selftests for memcg stat kfuncs

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: <2xhmcrporen72rskghn6hmg6obnojptuerwzqgu7mqzhnxaxs5@33dxlwa6rqlh>
Date: Fri, 19 Dec 2025 15:07:11 -0800
From: Shakeel Butt <shakeel.butt@...ux.dev>
To: Roman Gushchin <roman.gushchin@...ux.dev>
Cc: bpf@...r.kernel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	JP Kobryn <inwardvessel@...il.com>, Alexei Starovoitov <ast@...nel.org>, 
	Daniel Borkmann <daniel@...earbox.net>, Michal Hocko <mhocko@...nel.org>, 
	Johannes Weiner <hannes@...xchg.org>
Subject: Re: [PATCH bpf-next v1 6/6] bpf: selftests: selftests for memcg stat
 kfuncs

On Thu, Dec 18, 2025 at 05:57:50PM -0800, Roman Gushchin wrote:
> From: JP Kobryn <inwardvessel@...il.com>
> 
> Add test coverage for the kfuncs that fetch memcg stats. Using some common
> stats, test scenarios ensuring that the given stat increases by some
> arbitrary amount. The stats selected cover the three categories represented
> by the enums: node_stat_item, memcg_stat_item, vm_event_item.
> 
> Since only a subset of all stats are queried, use a static struct made up
> of fields for each stat. Write to the struct with the fetched values when
> the bpf program is invoked and read the fields in the user mode program for
> verification.
> 
> Signed-off-by: JP Kobryn <inwardvessel@...il.com>

Need your signoff

[...]
> +
> +#define NR_PIPES 64
> +static void test_kmem(struct bpf_link *link, struct memcg_query *memcg_query)
> +{
> +	int fds[NR_PIPES][2], i;
> +
> +	/*
> +	 * Increase kmem value by creating pipes which will allocate some
> +	 * kernel buffers.
> +	 */
> +	for (i = 0; i < NR_PIPES; i++) {
> +		if (!ASSERT_OK(pipe(fds[i]), "pipe"))
> +			goto cleanup;
> +	}
> +
> +	if (!ASSERT_OK(read_stats(link), "read stats"))
> +		goto cleanup;
> +
> +	ASSERT_GT(memcg_query->memcg_kmem, 0, "kmem value");
> +
> +cleanup:
> +	for (i = 0; i < NR_PIPES; i++) {

Instead of from 0 to NR_PIPES, we need to go from i-1 to (and equal to) 0
otherwise we can potentially close() junk values.

> +		close(fds[i][0]);
> +		close(fds[i][1]);
> +	}
> +}
> +

[...]

> +
> +SEC("iter.s/cgroup")
> +int cgroup_memcg_query(struct bpf_iter__cgroup *ctx)
> +{
> +	struct cgroup *cgrp = ctx->cgroup;
> +	struct cgroup_subsys_state *css;
> +	struct mem_cgroup *memcg;
> +
> +	if (!cgrp)
> +		return 1;
> +
> +	css = &cgrp->self;
> +	if (!css)

Will css ever be NULL here?

> +		return 1;
> +
> +	memcg = bpf_get_mem_cgroup(css);
> +	if (!memcg)
> +		return 1;
> +
> +	bpf_mem_cgroup_flush_stats(memcg);
> +
> +	memcg_query.nr_anon_mapped = bpf_mem_cgroup_page_state(memcg, NR_ANON_MAPPED);
> +	memcg_query.nr_shmem = bpf_mem_cgroup_page_state(memcg, NR_SHMEM);
> +	memcg_query.nr_file_pages = bpf_mem_cgroup_page_state(memcg, NR_FILE_PAGES);
> +	memcg_query.nr_file_mapped = bpf_mem_cgroup_page_state(memcg, NR_FILE_MAPPED);
> +	memcg_query.memcg_kmem = bpf_mem_cgroup_page_state(memcg, MEMCG_KMEM);
> +	memcg_query.pgfault = bpf_mem_cgroup_vm_events(memcg, PGFAULT);
> +
> +	bpf_put_mem_cgroup(memcg);
> +
> +	return 0;
> +}
> -- 
> 2.52.0
>