Re: [PATCH stable 4.19] mm: memcontrol: switch to rcu protection in drain_all_stock()

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Feb 26, 2024 at 11:01:40AM +0800, GONG, Ruiqi wrote:
> From: Roman Gushchin <guro@fb.com>
> 
> commit e1a366be5cb4f849ec4de170d50eebc08bb0af20 upstream.
> 
> Commit 72f0184c8a00 ("mm, memcg: remove hotplug locking from try_charge")
> introduced css_tryget()/css_put() calls in drain_all_stock(), which are
> supposed to protect the target memory cgroup from being released during
> the mem_cgroup_is_descendant() call.
> 
> However, it's not completely safe.  In theory, memcg can go away between
> reading stock->cached pointer and calling css_tryget().
> 
> This can happen if drain_all_stock() races with drain_local_stock()
> performed on the remote cpu as a result of a work, scheduled by the
> previous invocation of drain_all_stock().
> 
> The race is a bit theoretical and there are few chances to trigger it, but
> the current code looks a bit confusing, so it makes sense to fix it
> anyway.  The code looks like as if css_tryget() and css_put() are used to
> protect stocks drainage.  It's not necessary because stocked pages are
> holding references to the cached cgroup.  And it obviously won't work for
> works, scheduled on other cpus.
> 
> So, let's read the stock->cached pointer and evaluate the memory cgroup
> inside a rcu read section, and get rid of css_tryget()/css_put() calls.
> 
> Link: http://lkml.kernel.org/r/20190802192241.3253165-1-guro@fb.com
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> Cc: Hillf Danton <hdanton@sina.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: stable@vger.kernel.org  # 4.19
> Fixes: cdec2e4265df ("memcg: coalesce charging via percpu storage")
> Signed-off-by: GONG, Ruiqi <gongruiqi1@huawei.com>
> ---
> 
> This patch [1] fixed a UAF problem in drain_all_stock() existed prior to
> 5.9, and following discussions [2] mentioned that the fix depends on an
> RCU read protection to stock->cached (introduced in 5.4), which doesn't
> existed in 4.19. So backport this part to 4.19 as well.

Now queued up, thanks.

greg k-h