linux-kernel - Re: [RFC PATCH 1/3] memcg: add eBPF struct ops support for memory charging

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: <bb24c9ee360a4538d3aa5f00091586fe0d7ae4b7277ea8efe866457954a75696@mail.kernel.org>
Date: Wed, 19 Nov 2025 02:10:05 +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,linux-kernel@...r.kernel.org,linux-mm@...ck.org,cgroups@...r.kernel.org,bpf@...r.kernel.org,linux-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 1/3] memcg: add eBPF struct ops support for memory charging

> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 4deda3362..104c9e930 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -68,6 +68,7 @@
>  #include <net/ip.h>
>  #include "slab.h"
>  #include "memcontrol-v1.h"
> +#include "memcontrol_bpf.h"
>
>  #include <linux/uaccess.h>
>
> @@ -2301,13 +2302,14 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>  	int nr_retries = MAX_RECLAIM_RETRIES;
>  	struct mem_cgroup *mem_over_limit;
>  	struct page_counter *counter;
> -	unsigned long nr_reclaimed;
> +	unsigned long nr_reclaime, nr_reclaimed;
>  	bool passed_oom = false;
>  	unsigned int reclaim_options = MEMCG_RECLAIM_MAY_SWAP;
>  	bool drained = false;
>  	bool raised_max_event = false;
>  	unsigned long pflags;
>  	bool allow_spinning = gfpflags_allow_spinning(gfp_mask);
> +	bool charge_done = false;
>
>  retry:
>  	if (consume_stock(memcg, nr_pages))
> @@ -2320,20 +2322,30 @@ static int try_charge_memcg(struct mem_cgroup *memcg, gfp_t gfp_mask,
>  	if (!do_memsw_account() ||
>  	    page_counter_try_charge(&memcg->memsw, batch, &counter)) {
>  		if (page_counter_try_charge(&memcg->memory, batch, &counter))
> -			goto done_restock;
> -		if (do_memsw_account())
> -			page_counter_uncharge(&memcg->memsw, batch);
> -		mem_over_limit = mem_cgroup_from_counter(counter, memory);
> +			charge_done = true;
> +		else {
> +			if (do_memsw_account())
> +				page_counter_uncharge(&memcg->memsw, batch);
> +			mem_over_limit = mem_cgroup_from_counter(counter, memory);
> +		}
>  	} else {
>  		mem_over_limit = mem_cgroup_from_counter(counter, memsw);
>  		reclaim_options &= ~MEMCG_RECLAIM_MAY_SWAP;
>  	}
>
> -	if (batch > nr_pages) {
> +	if (!charge_done && batch > nr_pages) {
>  		batch = nr_pages;
>  		goto retry;
>  	}
>
> +	nr_reclaime = bpf_try_charge_memcg(memcg, gfp_mask, nr_pages,
> +					   mem_over_limit,
> +					   reclaim_options,
> +					   charge_done);
                                           ^^^^^^^^^^^^

When charge succeeds (charge_done = true), mem_over_limit is never
initialized but gets passed to bpf_try_charge_memcg(). Looking at
memcontrol_bpf.h, the BPF verifier allows read access to all fields
in struct try_charge_memcg via bpf_tracing_btf_ctx_access() (only
write access to nr_pages is restricted by memcg_ops_btf_struct_access).

Can the BPF program read this uninitialized pointer value? This would
leak kernel stack data to the BPF program.

> +
> +	if (charge_done)
> +		goto done_restock;

[ ... ]

> diff --git a/mm/memcontrol_bpf.h b/mm/memcontrol_bpf.h
> new file mode 100644
> index 000000000..ee2815fc3
> --- /dev/null
> +++ b/mm/memcontrol_bpf.h

[ ... ]

> +static inline unsigned long
> +bpf_try_charge_memcg(struct mem_cgroup *memcg,
> +		     gfp_t gfp_mask,
> +		     unsigned int nr_pages,
> +		     struct mem_cgroup *mem_over_limit,
> +		     unsigned int reclaim_options,
> +		     bool charge_done)
> +{
> +	struct memcg_ops *ops;
> +	struct try_charge_memcg tcm;
> +	int update_nr_pages;
> +
> +	if (likely(!static_branch_unlikely(&memcg_bpf_enable)))
> +		goto out;
> +
> +	ops = memcg_ops_get();
> +	if (unlikely(!ops))
> +		goto out;
> +
> +	tcm.memcg = memcg;
> +	tcm.gfp_mask = gfp_mask;
> +	tcm.nr_pages = nr_pages;
> +	tcm.mem_over_limit = mem_over_limit;
                              ^^^^^^^^^^^^^^^
Here mem_over_limit gets copied into the tcm struct, which is then
passed to the BPF program at the line below.

> +	tcm.reclaim_options = reclaim_options;
> +	tcm.charge_done = charge_done;
> +
> +	update_nr_pages = ops->try_charge_memcg(&tcm);

[ ... ]


---
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/19486904708