linux-kernel - Re: [PATCH 3/3] mm,migration: Remove straggling migration PTEs when page tables are being moved after the VMA has already moved

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

Date:	Wed, 28 Apr 2010 00:58:52 +0200
From:	Andrea Arcangeli <aarcange@...hat.com>
To:	Mel Gorman <mel@....ul.ie>
Cc:	Linux-MM <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
	Minchan Kim <minchan.kim@...il.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Christoph Lameter <cl@...ux.com>,
	Rik van Riel <riel@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 3/3] mm,migration: Remove straggling migration PTEs
 when page tables are being moved after the VMA has already moved

On Wed, Apr 28, 2010 at 12:30:04AM +0200, Andrea Arcangeli wrote:
> I'll now evaluate the fix and see if I can find any other
> way to handle this.

I think a better fix for bug mentioned in patch 3, is like below. This
seems to work fine on aa.git with the old (stable) 2.6.33 anon-vma
code. Not sure if this also works with the new anon-vma code in
mainline but at first glance I think it should. At that point we
should be single threaded so it shouldn't matter if anon_vma is
temporary null.

Then you've to re-evaluate the vma_adjust fixes for mainline-only in
patch 2 at the light of the below (I didn't check patch 2 in detail).

Please try to reproduce with the below applied.

----
Subject: fix race between shift_arg_pages and rmap_walk

From: Andrea Arcangeli <aarcange@...hat.com>

migrate.c requires rmap to be able to find all ptes mapping a page at
all times, otherwise the migration entry can be instantiated, but it
can't be removed if the second rmap_walk fails to find the page.

So shift_arg_pages must run atomically with respect of rmap_walk, and
it's enough to run it under the anon_vma lock to make it atomic.

And split_huge_page() will have the same requirements as migrate.c
already has.

Signed-off-by: Andrea Arcangeli <aarcange@...hat.com>
---

diff --git a/fs/exec.c b/fs/exec.c
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -55,6 +55,7 @@
 #include <linux/fsnotify.h>
 #include <linux/fs_struct.h>
 #include <linux/pipe_fs_i.h>
+#include <linux/rmap.h>

 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
@@ -503,6 +504,7 @@ static int shift_arg_pages(struct vm_are
 	unsigned long new_start = old_start - shift;
 	unsigned long new_end = old_end - shift;
 	struct mmu_gather *tlb;
+	struct anon_vma *anon_vma;

 	BUG_ON(new_start > new_end);

@@ -513,6 +515,12 @@ static int shift_arg_pages(struct vm_are
 	if (vma != find_vma(mm, new_start))
 		return -EFAULT;

+	anon_vma = vma->anon_vma;
+	/* stop rmap_walk or it won't find the stack pages */
+	spin_lock(&anon_vma->lock);
+	/* avoid vma_adjust to take any further anon_vma lock */
+	vma->anon_vma = NULL;
+
 	/*
 	 * cover the whole range: [new_start, old_end)
 	 */
@@ -551,6 +559,9 @@ static int shift_arg_pages(struct vm_are
 	 */
 	vma_adjust(vma, new_start, new_end, vma->vm_pgoff, NULL);

+	vma->anon_vma = anon_vma;
+	spin_unlock(&anon_vma->lock);
+
 	return 0;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/