Re: [PATCH] mm: kmem: properly initialize local objcg variable in current_obj_cgroup()

[Erhard Furtner <erhard_f@mailbox.org>]

On Thu, 16 Nov 2023 08:04:18 +0100
Vlastimil Babka <vbabka@suse.cz> wrote:

> On 11/16/23 03:51, Roman Gushchin wrote:
> > Actually the problem is caused by uninitialized local variable in
> > current_obj_cgroup(). If the root memory cgroup is set as an active
> > memory cgroup for a charging scope (as in the trace, where systemd
> > tries to create the first non-root cgroup, so the parent cgroup is
> > the root cgroup), the "for" loop is skipped and uninitialized objcg is
> > returned, causing a panic down the accounting stack.
> > 
> > The fix is trivial: initialize the objcg variable to NULL
> > unconditionally before the "for" loop.
> > 
> > Fixes: e86828e5446d ("mm: kmem: scoped objcg protection")
> > Reported-by: Erhard Furtner <erhard_f@mailbox.org>
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/1959
> > Signed-off-by: Roman Gushchin (Cruise) <roman.gushchin@linux.dev>
> > Cc: Shakeel Butt <shakeelb@google.com>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Dennis Zhou <dennis@kernel.org>
> > Cc: Johannes Weiner <hannes@cmpxchg.org>
> > Cc: Michal Hocko <mhocko@kernel.org>
> > Cc: Muchun Song <muchun.song@linux.dev>
> > Cc: stable@vger.kernel.org  
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> 
> We could also do this to make it less confusing?
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 774bd6e21e27..a08bcec661b6 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -3175,7 +3175,6 @@ __always_inline struct obj_cgroup *current_obj_cgroup(void)
>  		objcg = rcu_dereference_check(memcg->objcg, 1);
>  		if (likely(objcg))
>  			break;
> -		objcg = NULL;
>  	}
>  
>  	return objcg;
> 
> 

I can confirm the 1st patch from Roman fixes the issue on my amd64 and on my i686 box.

The 2nd patch from Vlastimil unfortunately does not (only tried on amd64).

Regards,
Erhard