--- /dev/null
+From 54a83d6bcbf8f4700013766b974bf9190d40b689 Mon Sep 17 00:00:00 2001
+From: Miles Chen <miles.chen@mediatek.com>
+Date: Tue, 13 Aug 2019 15:37:28 -0700
+Subject: mm/memcontrol.c: fix use after free in mem_cgroup_iter()
+
+From: Miles Chen <miles.chen@mediatek.com>
+
+commit 54a83d6bcbf8f4700013766b974bf9190d40b689 upstream.
+
+This patch is sent to report an use after free in mem_cgroup_iter()
+after merging commit be2657752e9e ("mm: memcg: fix use after free in
+mem_cgroup_iter()").
+
+I work with android kernel tree (4.9 & 4.14), and commit be2657752e9e
+("mm: memcg: fix use after free in mem_cgroup_iter()") has been merged
+to the trees. However, I can still observe use after free issues
+addressed in the commit be2657752e9e. (on low-end devices, a few times
+this month)
+
+backtrace:
+ css_tryget <- crash here
+ mem_cgroup_iter
+ shrink_node
+ shrink_zones
+ do_try_to_free_pages
+ try_to_free_pages
+ __perform_reclaim
+ __alloc_pages_direct_reclaim
+ __alloc_pages_slowpath
+ __alloc_pages_nodemask
+
+To debug, I poisoned mem_cgroup before freeing it:
+
+ static void __mem_cgroup_free(struct mem_cgroup *memcg)
+ for_each_node(node)
+ free_mem_cgroup_per_node_info(memcg, node);
+ free_percpu(memcg->stat);
+ + /* poison memcg before freeing it */
+ + memset(memcg, 0x78, sizeof(struct mem_cgroup));
+ kfree(memcg);
+ }
+
+The coredump shows the position=0xdbbc2a00 is freed.
+
+ (gdb) p/x ((struct mem_cgroup_per_node *)0xe5009e00)->iter[8]
+ $13 = {position = 0xdbbc2a00, generation = 0x2efd}
+
+ 0xdbbc2a00: 0xdbbc2e00 0x00000000 0xdbbc2800 0x00000100
+ 0xdbbc2a10: 0x00000200 0x78787878 0x00026218 0x00000000
+ 0xdbbc2a20: 0xdcad6000 0x00000001 0x78787800 0x00000000
+ 0xdbbc2a30: 0x78780000 0x00000000 0x0068fb84 0x78787878
+ 0xdbbc2a40: 0x78787878 0x78787878 0x78787878 0xe3fa5cc0
+ 0xdbbc2a50: 0x78787878 0x78787878 0x00000000 0x00000000
+ 0xdbbc2a60: 0x00000000 0x00000000 0x00000000 0x00000000
+ 0xdbbc2a70: 0x00000000 0x00000000 0x00000000 0x00000000
+ 0xdbbc2a80: 0x00000000 0x00000000 0x00000000 0x00000000
+ 0xdbbc2a90: 0x00000001 0x00000000 0x00000000 0x00100000
+ 0xdbbc2aa0: 0x00000001 0xdbbc2ac8 0x00000000 0x00000000
+ 0xdbbc2ab0: 0x00000000 0x00000000 0x00000000 0x00000000
+ 0xdbbc2ac0: 0x00000000 0x00000000 0xe5b02618 0x00001000
+ 0xdbbc2ad0: 0x00000000 0x78787878 0x78787878 0x78787878
+ 0xdbbc2ae0: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2af0: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b00: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b10: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b20: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b30: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b40: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b50: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b60: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b70: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2b80: 0x78787878 0x78787878 0x00000000 0x78787878
+ 0xdbbc2b90: 0x78787878 0x78787878 0x78787878 0x78787878
+ 0xdbbc2ba0: 0x78787878 0x78787878 0x78787878 0x78787878
+
+In the reclaim path, try_to_free_pages() does not setup
+sc.target_mem_cgroup and sc is passed to do_try_to_free_pages(), ...,
+shrink_node().
+
+In mem_cgroup_iter(), root is set to root_mem_cgroup because
+sc->target_mem_cgroup is NULL. It is possible to assign a memcg to
+root_mem_cgroup.nodeinfo.iter in mem_cgroup_iter().
+
+ try_to_free_pages
+ struct scan_control sc = {...}, target_mem_cgroup is 0x0;
+ do_try_to_free_pages
+ shrink_zones
+ shrink_node
+ mem_cgroup *root = sc->target_mem_cgroup;
+ memcg = mem_cgroup_iter(root, NULL, &reclaim);
+ mem_cgroup_iter()
+ if (!root)
+ root = root_mem_cgroup;
+ ...
+
+ css = css_next_descendant_pre(css, &root->css);
+ memcg = mem_cgroup_from_css(css);
+ cmpxchg(&iter->position, pos, memcg);
+
+My device uses memcg non-hierarchical mode. When we release a memcg:
+invalidate_reclaim_iterators() reaches only dead_memcg and its parents.
+If non-hierarchical mode is used, invalidate_reclaim_iterators() never
+reaches root_mem_cgroup.
+
+ static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg)
+ {
+ struct mem_cgroup *memcg = dead_memcg;
+
+ for (; memcg; memcg = parent_mem_cgroup(memcg)
+ ...
+ }
+
+So the use after free scenario looks like:
+
+ CPU1 CPU2
+
+ try_to_free_pages
+ do_try_to_free_pages
+ shrink_zones
+ shrink_node
+ mem_cgroup_iter()
+ if (!root)
+ root = root_mem_cgroup;
+ ...
+ css = css_next_descendant_pre(css, &root->css);
+ memcg = mem_cgroup_from_css(css);
+ cmpxchg(&iter->position, pos, memcg);
+
+ invalidate_reclaim_iterators(memcg);
+ ...
+ __mem_cgroup_free()
+ kfree(memcg);
+
+ try_to_free_pages
+ do_try_to_free_pages
+ shrink_zones
+ shrink_node
+ mem_cgroup_iter()
+ if (!root)
+ root = root_mem_cgroup;
+ ...
+ mz = mem_cgroup_nodeinfo(root, reclaim->pgdat->node_id);
+ iter = &mz->iter[reclaim->priority];
+ pos = READ_ONCE(iter->position);
+ css_tryget(&pos->css) <- use after free
+
+To avoid this, we should also invalidate root_mem_cgroup.nodeinfo.iter
+in invalidate_reclaim_iterators().
+
+[cai@lca.pw: fix -Wparentheses compilation warning]
+ Link: http://lkml.kernel.org/r/1564580753-17531-1-git-send-email-cai@lca.pw
+Link: http://lkml.kernel.org/r/20190730015729.4406-1-miles.chen@mediatek.com
+Fixes: 5ac8fb31ad2e ("mm: memcontrol: convert reclaim iterator to simple css refcounting")
+Signed-off-by: Miles Chen <miles.chen@mediatek.com>
+Signed-off-by: Qian Cai <cai@lca.pw>
+Acked-by: Michal Hocko <mhocko@suse.com>
+Cc: Johannes Weiner <hannes@cmpxchg.org>
+Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
+Cc: <stable@vger.kernel.org>
+Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
+Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+
+
+---
+ mm/memcontrol.c | 39 +++++++++++++++++++++++++++++----------
+ 1 file changed, 29 insertions(+), 10 deletions(-)
+
+--- a/mm/memcontrol.c
++++ b/mm/memcontrol.c
+@@ -871,26 +871,45 @@ void mem_cgroup_iter_break(struct mem_cg
+ css_put(&prev->css);
+ }
+
+-static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg)
++static void __invalidate_reclaim_iterators(struct mem_cgroup *from,
++ struct mem_cgroup *dead_memcg)
+ {
+- struct mem_cgroup *memcg = dead_memcg;
+ struct mem_cgroup_reclaim_iter *iter;
+ struct mem_cgroup_per_node *mz;
+ int nid;
+ int i;
+
+- for (; memcg; memcg = parent_mem_cgroup(memcg)) {
+- for_each_node(nid) {
+- mz = mem_cgroup_nodeinfo(memcg, nid);
+- for (i = 0; i <= DEF_PRIORITY; i++) {
+- iter = &mz->iter[i];
+- cmpxchg(&iter->position,
+- dead_memcg, NULL);
+- }
++ for_each_node(nid) {
++ mz = mem_cgroup_nodeinfo(from, nid);
++ for (i = 0; i <= DEF_PRIORITY; i++) {
++ iter = &mz->iter[i];
++ cmpxchg(&iter->position,
++ dead_memcg, NULL);
+ }
+ }
+ }
+
++static void invalidate_reclaim_iterators(struct mem_cgroup *dead_memcg)
++{
++ struct mem_cgroup *memcg = dead_memcg;
++ struct mem_cgroup *last;
++
++ do {
++ __invalidate_reclaim_iterators(memcg, dead_memcg);
++ last = memcg;
++ } while ((memcg = parent_mem_cgroup(memcg)));
++
++ /*
++ * When cgruop1 non-hierarchy mode is used,
++ * parent_mem_cgroup() does not walk all the way up to the
++ * cgroup root (root_mem_cgroup). So we have to handle
++ * dead_memcg from cgroup root separately.
++ */
++ if (last != root_mem_cgroup)
++ __invalidate_reclaim_iterators(root_mem_cgroup,
++ dead_memcg);
++}
++
+ /*
+ * Iteration constructs for visiting all cgroups (under a tree). If
+ * loops are exited prematurely (break), mem_cgroup_iter_break() must
projects / thirdparty / kernel / stable-queue.git / commitdiff
