Message ID | 20240226030140.129822-1-gongruiqi1@huawei.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | [stable,4.19] mm: memcontrol: switch to rcu protection in drain_all_stock() | expand |
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
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 8c04296df1c7..d187bfb43b1f 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2094,21 +2094,22 @@ static void drain_all_stock(struct mem_cgroup *root_memcg) for_each_online_cpu(cpu) { struct memcg_stock_pcp *stock = &per_cpu(memcg_stock, cpu); struct mem_cgroup *memcg; + bool flush = false; + rcu_read_lock(); memcg = stock->cached; - if (!memcg || !stock->nr_pages || !css_tryget(&memcg->css)) - continue; - if (!mem_cgroup_is_descendant(memcg, root_memcg)) { - css_put(&memcg->css); - continue; - } - if (!test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) { + if (memcg && stock->nr_pages && + mem_cgroup_is_descendant(memcg, root_memcg)) + flush = true; + rcu_read_unlock(); + + if (flush && + !test_and_set_bit(FLUSHING_CACHED_CHARGE, &stock->flags)) { if (cpu == curcpu) drain_local_stock(&stock->work); else schedule_work_on(cpu, &stock->work); } - css_put(&memcg->css); } put_cpu(); mutex_unlock(&percpu_charge_mutex);