[Devel] [PATCH rh7] mm: memcontrol: fix race between user memory reparent and charge

[Vladimir Davydov]

When a memory cgroup is destroyed (via rmdir), user memory pages
accounted to it get recharged to the parent cgroup - see
mem_cgroup_css_offlie() and mem_cgroup_reparent_charges(). If, for some
reason, a page is left charged to the destroyed cgroup after
mem_cgroup_reparent_charges() was done, we might get use-after-free,
because user memory charges do not hold reference to the cgroup.

And it seems to be possible in case reparenting races with
__mem_cgroup_try_charge() as follows:

  __mem_cgroup_try_charge
   get memcg from mm, inc ref
                                        mem_cgroup_css_offline
                                         mem_cgroup_reparent_charges
   charge page to memcg
   put ref to memcg

To fix this issue, let's make __mem_cgroup_try_charge() cancel charge if
it detects that the cgroup was destroyed.

https://jira.sw.ru/browse/PSBM-49117

Signed-off-by: Vladimir Davydov <vdavydov at virtuozzo.com>
---
 mm/memcontrol.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 8151d4259c6b..e3a16b99ccc6 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -314,6 +314,7 @@ struct mem_cgroup {
 	 * Should the accounting and control be hierarchical, per subtree?
 	 */
 	bool use_hierarchy;
+	bool is_offline;
 	unsigned long kmem_account_flags; /* See KMEM_ACCOUNTED_*, below */
 
 	bool		oom_lock;
@@ -2952,6 +2953,23 @@ again:
 		}
 	} while (ret != CHARGE_OK);
 
+	/*
+	 * Cancel charge in case this cgroup was destroyed while we were here,
+	 * otherwise we can get a pending user memory charge to an offline
+	 * cgroup, which might result in use-after-free after the cgroup gets
+	 * released (see also mem_cgroup_css_offline()).
+	 *
+	 * Note, no need to issue an explicit barrier here, because a
+	 * successful charge implies full memory barrier.
+	 */
+	if (unlikely(memcg->is_offline)) {
+		res_counter_uncharge(&memcg->res, batch * PAGE_SIZE);
+		if (do_swap_account)
+			res_counter_uncharge(&memcg->memsw, batch * PAGE_SIZE);
+		css_put(&memcg->css);
+		goto bypass;
+	}
+
 	if (batch > nr_pages)
 		refill_stock(memcg, batch - nr_pages);
 
@@ -6657,6 +6675,15 @@ static void mem_cgroup_css_offline(struct cgroup *cont)
 {
 	struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
 
+	/*
+	 * Mark memory cgroup as offline before going to reparent charges.
+	 * This guarantees that __mem_cgroup_try_charge() either charges before
+	 * reparenting starts or doesn't charge at all, hence we won't have
+	 * pending user memory charges after reparenting is done.
+	 */
+	memcg->is_offline = true;
+	smp_mb();
+
 	memcg_deactivate_kmem(memcg);
 
 	mem_cgroup_invalidate_reclaim_iterators(memcg);
-- 
2.1.4