linux-kernel - Re: [PATCH stable 4.19] mm: memcontrol: switch to rcu protection in drain_all

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] [day] [month] [year] [list]

Date: Mon, 26 Feb 2024 08:16:10 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: "GONG, Ruiqi" <gongruiqi1@...wei.com>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Johannes Weiner <hannes@...xchg.org>,
	Michal Hocko <mhocko@...nel.org>,
	Roman Gushchin <roman.gushchin@...ux.dev>,
	Shakeel Butt <shakeelb@...gle.com>,
	Muchun Song <muchun.song@...ux.dev>, cgroups@...r.kernel.org,
	linux-mm@...ck.org, Wang Weiyang <wangweiyang2@...wei.com>,
	Xiu Jianfeng <xiujianfeng@...wei.com>
Subject: Re: [PATCH stable 4.19] mm: memcontrol: switch to rcu protection in
 drain_all_stock()

On Mon, Feb 26, 2024 at 11:01:40AM +0800, GONG, Ruiqi wrote:
> From: Roman Gushchin <guro@...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@...com>
> Acked-by: Michal Hocko <mhocko@...e.com>
> Cc: Hillf Danton <hdanton@...a.com>
> Cc: Johannes Weiner <hannes@...xchg.org>
> Cc: Vladimir Davydov <vdavydov.dev@...il.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: stable@...r.kernel.org  # 4.19
> Fixes: cdec2e4265df ("memcg: coalesce charging via percpu storage")
> Signed-off-by: GONG, Ruiqi <gongruiqi1@...wei.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