linux-kernel - [PATCH] mm: memcg: make memory.oom.group tolerable to task migration

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

Message-ID: <20200316223510.3176148-1-guro@fb.com>
Date:   Mon, 16 Mar 2020 15:35:10 -0700
From:   Roman Gushchin <guro@...com>
To:     Andrew Morton <akpm@...ux-foundation.org>
CC:     Michal Hocko <mhocko@...nel.org>, <linux-mm@...ck.org>,
        <kernel-team@...com>, <linux-kernel@...r.kernel.org>,
        Roman Gushchin <guro@...com>
Subject: [PATCH] mm: memcg: make memory.oom.group tolerable to task migration

If a task is getting moved out of the OOMing cgroup, it might
result in unexpected OOM killings if memory.oom.group is used
anywhere in the cgroup tree.

Imagine the following example:

          A (oom.group = 1)
         / \
  (OOM) B   C

Let's say B's memory.max is exceeded and it's OOMing. The OOM killer
selects a task in B as a victim, but someone asynchronously moves
the task into C. mem_cgroup_get_oom_group() will iterate over all
ancestors of C up to the root cgroup. In theory it had to stop
at the oom_domain level - the memory cgroup which is OOMing.
But because B is not an ancestor of C, it's not happening.
Instead it chooses A (because it's oom.group is set), and kills
all tasks in A. This behavior is wrong because the OOM happened in B,
so there is no reason to kill anything outside.

Fix this by checking it the memory cgroup to which the task belongs
is a descendant of the oom_domain. If not, memory.oom.group should
be ignored, and the OOM killer should kill only the victim task.

Signed-off-by: Roman Gushchin <guro@...com>
Reported-by: Dan Schatzberg <dschatzberg@...com>
---
 mm/memcontrol.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index daa399be4688..d8c4b7aa4e73 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1930,6 +1930,14 @@ struct mem_cgroup *mem_cgroup_get_oom_group(struct task_struct *victim,
 	if (memcg == root_mem_cgroup)
 		goto out;

+	/*
+	 * If the victim task has been asynchronously moved to a different
+	 * memory cgroup, we might end up killing tasks outside oom_domain.
+	 * In this case it's better to ignore memory.group.oom.
+	 */
+	if (unlikely(!mem_cgroup_is_descendant(memcg, oom_domain)))
+		goto out;
+
 	/*
 	 * Traverse the memory cgroup hierarchy from the victim task's
 	 * cgroup up to the OOMing cgroup (or root) to find the
-- 
2.24.1