Re: [PATCH] mm/memcontrol: fix obj_cgroup leak in mem_cgroup_css_online() error path

[Qi Zheng <qi.zheng@linux.dev>]

On 3/23/26 3:36 AM, David Carlier wrote:
> When obj_cgroup_alloc() fails partway through the NUMA node loop in
> mem_cgroup_css_online(), the free_objcg error path drops the extra
> reference held by pn->orig_objcg but never kills the initial percpu_ref
> from obj_cgroup_alloc() stored in pn->objcg.
> 
> Since css_offline is never called when css_online fails,
> memcg_reparent_objcgs() never runs, so the percpu_ref_kill() that
> normally drops this initial reference never executes. The obj_cgroup and
> its per-cpu ref allocations are leaked.
> 
> Clear pn->objcg via rcu_replace_pointer() and add the missing
> percpu_ref_kill() in the error path, matching the normal teardown
> sequence in memcg_reparent_objcgs().
> 
> Also add a NULL check for pn in __mem_cgroup_free() to prevent a NULL
> pointer dereference when alloc_mem_cgroup_per_node_info() fails partway
> through the node loop in mem_cgroup_alloc().
> 
> Fixes: 098fad3e1621 ("mm: memcontrol: convert objcg to be per-memcg per-node type")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>   mm/memcontrol.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index a47fb68dd65f..00b3bb81aee4 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3936,6 +3936,8 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)
>   
>   	for_each_node(node) {
>   		struct mem_cgroup_per_node *pn = memcg->nodeinfo[node];
> +		if (!pn)
> +			continue;
>   
>   		obj_cgroup_put(pn->orig_objcg);
>   		free_mem_cgroup_per_node_info(pn);
> @@ -4137,8 +4139,11 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>   free_objcg:
>   	for_each_node(nid) {
>   		struct mem_cgroup_per_node *pn = memcg->nodeinfo[nid];

Nit: A newline character is needed here, otherwise the checkpatch might
complain.

> +		objcg = rcu_replace_pointer(pn->objcg, NULL, true);
> +		if (objcg)
> +			percpu_ref_kill(&objcg->refcnt);
>   
> -		if (pn && pn->orig_objcg) {
> +		if (pn->orig_objcg) {
>   			obj_cgroup_put(pn->orig_objcg);
>   			/*
>   			 * Reset pn->orig_objcg to NULL to prevent

Make sense, thanks!

Acked-by: Qi Zheng <zhengqi.arch@bytedance.com>