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

Vladimir Davydov vdavydov at virtuozzo.com
Thu Jul 7 07:33:30 PDT 2016


The commit is pushed to "branch-rh7-3.10.0-327.18.2.vz7.14.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.18.2.vz7.14.23
------>
commit 9c3a30958ef74ed70bcc92ecf3727d36dbe463a0
Author: Vladimir Davydov <vdavydov at virtuozzo.com>
Date:   Thu Jul 7 18:33:30 2016 +0400

    mm: memcontrol: fix race between user memory reparent and charge
    
    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);


More information about the Devel mailing list