From: Linus Torvalds <torvalds@linux-foundation.org>
To: Borislav Petkov <bp@alien8.de>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
Rik van Riel <riel@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
Minchan Kim <minchan.kim@gmail.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Lee Schermerhorn <Lee.Schermerhorn@hp.com>,
Nick Piggin <npiggin@suse.de>,
Andrea Arcangeli <aarcange@redhat.com>,
Hugh Dickins <hugh.dickins@tiscali.co.uk>,
sgunderson@bigfoot.com
Subject: Re: [PATCH -v2] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA
Date: Sat, 10 Apr 2010 13:05:22 -0700 (PDT) [thread overview]
Message-ID: <alpine.LFD.2.00.1004101234210.3558@i5.linux-foundation.org> (raw)
In-Reply-To: <20100410185839.GA32035@a1.tnic>
On Sat, 10 Apr 2010, Borislav Petkov wrote:
> From: Borislav Petkov <bp@alien8.de>
> Date: Sat, Apr 10, 2010 at 08:51:45PM +0200
>
> > Anyways, testing...
>
> Nope, still b0rked. And this time is not a funny pattern but
> ffffffffffffffe0 we had originally.
Ok, I think that just depends on who happens to re-use the allocation and
how it does it.
I'm pretty sure it's a use-after-free issue, where we have free'd an
anon_vma too early, even though it has pages associated with it.
If it wasn't the RCU case, it's just something else.
I think it's worth looking at "vma_adjust()", because as I already
mentioned to Rik earlier - the code is very hard to understand, and it's
accrued crud over many many years.
And vma_adjust is the one place that does that anon_vma_merge(), which is
apart from the actual unmapping sequence the only other place that
actually free's anon_vmas. So there are reasons to be very suspicious of
that code.
And I think that code can actually lose an anon_vma chain. It's totally
screwing up the "import anonvma" case: when it does
if (anon_vma_clone(importer, vma)) {
return -ENOMEM;
}
importer->anon_vma = anon_vma;
we can actually have "importer == vma", but "anon_vma = next->anon_vma".
In which case we actually end up with an _empty_ chain (because importer
didn't have a chain to begin with!) but "importer->anon_vma" points to an
anon_vma.
And then when we do that "remove_next", we actually get rid of the only
chain we ever had, and have lost all our references to the anon_vma.
That looks _horribly_ buggy.
Also, the conditional nesting makes no sense (the whole anon_vma_clone()
only makes sense if importer is set, and it is only ever set _inside_ the
earlier if-statement, so the whole code should be moved inside there), nor
does some of the comments.
This patch is scary and untested, but the more I look at that code, the
more convinced I am that vma_adjust was _really_ badly screwed up. The
patch below may make things worse. I'll test it myself too, but I'm
sending it out first, since I was writing the email as I was looking at
the piece of cr*p.
Linus
---
mm/mmap.c | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index acb023e..f90ea92 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -507,11 +507,12 @@ int vma_adjust(struct vm_area_struct *vma, unsigned long start,
struct address_space *mapping = NULL;
struct prio_tree_root *root = NULL;
struct file *file = vma->vm_file;
- struct anon_vma *anon_vma = NULL;
long adjust_next = 0;
int remove_next = 0;
if (next && !insert) {
+ struct vm_area_struct *exporter = NULL;
+
if (end >= next->vm_end) {
/*
* vma expands, overlapping all the next, and
@@ -519,7 +520,7 @@ int vma_adjust(struct vm_area_struct *vma, unsigned long start,
*/
again: remove_next = 1 + (end > next->vm_end);
end = next->vm_end;
- anon_vma = next->anon_vma;
+ exporter = next;
importer = vma;
} else if (end > next->vm_start) {
/*
@@ -527,7 +528,7 @@ again: remove_next = 1 + (end > next->vm_end);
* mprotect case 5 shifting the boundary up.
*/
adjust_next = (end - next->vm_start) >> PAGE_SHIFT;
- anon_vma = next->anon_vma;
+ exporter = next;
importer = vma;
} else if (end < vma->vm_end) {
/*
@@ -536,28 +537,19 @@ again: remove_next = 1 + (end > next->vm_end);
* mprotect case 4 shifting the boundary down.
*/
adjust_next = - ((vma->vm_end - end) >> PAGE_SHIFT);
- anon_vma = next->anon_vma;
+ exporter = vma;
importer = next;
}
- }
- /*
- * When changing only vma->vm_end, we don't really need anon_vma lock.
- */
- if (vma->anon_vma && (insert || importer || start != vma->vm_start))
- anon_vma = vma->anon_vma;
- if (anon_vma) {
/*
* Easily overlooked: when mprotect shifts the boundary,
* make sure the expanding vma has anon_vma set if the
* shrinking vma had, to cover any anon pages imported.
*/
- if (importer && !importer->anon_vma) {
- /* Block reverse map lookups until things are set up. */
- if (anon_vma_clone(importer, vma)) {
+ if (exporter && exporter->anon_vma && !importer->anon_vma) {
+ if (anon_vma_clone(importer, exporter))
return -ENOMEM;
- }
- importer->anon_vma = anon_vma;
+ importer->anon_vma = exporter->anon_vma;
}
}
next prev parent reply other threads:[~2010-04-10 20:10 UTC|newest]
Thread overview: 231+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-30 17:50 Linux 2.6.34-rc3 Linus Torvalds
2010-03-30 21:16 ` [Regression, post-rc2] Commit a5ee4eb7541 breaks OpenGL on RS780 (was: Re: Linux 2.6.34-rc3) Rafael J. Wysocki
2010-03-31 20:34 ` [stable] " Greg KH
2010-04-01 1:13 ` Rafael J. Wysocki
2010-04-01 2:19 ` Alex Deucher
2010-04-01 6:36 ` Clemens Ladisch
2010-04-01 15:01 ` Alex Deucher
2010-04-01 20:28 ` Rafael J. Wysocki
2010-04-01 20:39 ` Alex Deucher
2010-04-01 20:48 ` Rafael J. Wysocki
2010-04-01 21:00 ` Alex Deucher
2010-04-01 21:01 ` Alex Deucher
2010-04-01 21:08 ` Rafael J. Wysocki
2010-04-01 21:13 ` Alex Deucher
2010-04-01 21:46 ` Rafael J. Wysocki
2010-04-01 22:07 ` Alex Deucher
2010-04-01 23:20 ` Rafael J. Wysocki
2010-04-02 0:23 ` Linus Torvalds
2010-04-02 16:46 ` Rafael J. Wysocki
2010-04-03 18:08 ` Clemens Ladisch
2010-04-03 19:33 ` Rafael J. Wysocki
2010-04-01 16:29 ` Linus Torvalds
2010-04-01 17:07 ` Alex Deucher
2010-04-01 17:24 ` Linus Torvalds
2010-04-01 17:50 ` [Regression, post-rc2] Commit a5ee4eb7541 breaks OpenGL on RS780 Clemens Ladisch
2010-04-01 17:53 ` [Regression, post-rc2] Commit a5ee4eb7541 breaks OpenGL on RS780 (was: Re: Linux 2.6.34-rc3) Alex Deucher
2010-04-01 20:17 ` Linus Torvalds
2010-04-01 20:23 ` Alex Deucher
2010-04-01 19:46 ` Rafael J. Wysocki
2010-04-01 22:48 ` Jesse Barnes
2010-04-01 23:23 ` Rafael J. Wysocki
2010-04-02 17:59 ` Ugly rmap NULL ptr deref oopsie on hibernate (was " Borislav Petkov
2010-04-02 18:09 ` Linus Torvalds
2010-04-02 15:24 ` Andrew Morton
2010-04-02 18:37 ` Linus Torvalds
2010-04-02 22:01 ` Rik van Riel
2010-04-03 0:19 ` Linus Torvalds
2010-04-04 16:12 ` Minchan Kim
2010-04-04 17:24 ` Rik van Riel
2010-04-04 23:09 ` [PATCH] rmap: fix anon_vma_fork() memory leak Rik van Riel
2010-04-04 23:56 ` Minchan Kim
2010-04-05 15:37 ` Linus Torvalds
2010-04-05 15:48 ` Minchan Kim
2010-04-05 16:04 ` Rik van Riel
2010-04-05 16:13 ` [PATCH -v2] " Rik van Riel
2010-04-06 8:53 ` Ugly rmap NULL ptr deref oopsie on hibernate (was Linux 2.6.34-rc3) KOSAKI Motohiro
2010-04-06 10:09 ` KOSAKI Motohiro
2010-04-06 14:34 ` Rik van Riel
2010-04-06 14:38 ` Rik van Riel
2010-04-06 15:34 ` Minchan Kim
2010-04-06 15:40 ` Rik van Riel
2010-04-06 15:58 ` Minchan Kim
2010-04-06 15:55 ` Linus Torvalds
2010-04-06 16:23 ` Minchan Kim
2010-04-06 16:28 ` Linus Torvalds
2010-04-06 16:45 ` Minchan Kim
2010-04-06 16:53 ` Linus Torvalds
2010-04-06 17:04 ` Rik van Riel
2010-04-06 18:28 ` Linus Torvalds
2010-04-06 19:03 ` Andrew Morton
2010-04-06 19:10 ` Steinar H. Gunderson
2010-04-06 19:10 ` Linus Torvalds
2010-04-06 19:35 ` Linus Torvalds
2010-04-06 19:42 ` Borislav Petkov
2010-04-06 20:02 ` Linus Torvalds
2010-04-06 20:46 ` Steinar H. Gunderson
2010-04-06 20:56 ` Linus Torvalds
2010-04-06 21:05 ` Steinar H. Gunderson
2010-04-06 20:51 ` Borislav Petkov
2010-04-06 21:27 ` Linus Torvalds
2010-04-06 22:59 ` Borislav Petkov
2010-04-06 23:27 ` Linus Torvalds
2010-04-06 23:54 ` [PATCH] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA Rik van Riel
2010-04-07 7:00 ` KOSAKI Motohiro
2010-04-07 14:48 ` Rik van Riel
2010-04-07 14:54 ` [PATCH -v2] " Rik van Riel
2010-04-07 15:30 ` Linus Torvalds
2010-04-07 15:52 ` Rik van Riel
2010-04-07 16:56 ` Linus Torvalds
2010-04-07 21:19 ` Linus Torvalds
2010-04-07 21:52 ` Rik van Riel
2010-04-07 22:09 ` Linus Torvalds
2010-04-07 22:15 ` Linus Torvalds
2010-04-08 0:38 ` Rik van Riel
2010-04-07 23:37 ` Linus Torvalds
2010-04-08 2:03 ` KOSAKI Motohiro
2010-04-08 2:33 ` Linus Torvalds
2010-04-08 5:47 ` Borislav Petkov
2010-04-08 14:11 ` Linus Torvalds
2010-04-08 18:25 ` Rik van Riel
2010-04-08 18:32 ` Linus Torvalds
2010-04-08 20:31 ` Borislav Petkov
2010-04-08 21:00 ` Borislav Petkov
2010-04-08 23:16 ` Linus Torvalds
2010-04-08 23:47 ` Borislav Petkov
2010-04-09 0:50 ` Linus Torvalds
2010-04-09 1:30 ` Borislav Petkov
2010-04-09 9:21 ` Borislav Petkov
2010-04-09 16:35 ` Linus Torvalds
2010-04-09 17:40 ` Borislav Petkov
2010-04-09 17:50 ` Linus Torvalds
2010-04-09 19:14 ` Borislav Petkov
2010-04-09 19:32 ` Linus Torvalds
2010-04-09 20:03 ` Rik van Riel
2010-04-09 20:43 ` Johannes Weiner
2010-04-09 20:57 ` Rik van Riel
2010-04-09 21:33 ` Borislav Petkov
2010-04-09 23:22 ` Linus Torvalds
2010-04-09 23:45 ` Rik van Riel
2010-04-10 0:03 ` Linus Torvalds
2010-04-10 0:11 ` Rik van Riel
2010-04-09 23:54 ` Johannes Weiner
2010-04-09 23:56 ` Linus Torvalds
2010-04-10 0:19 ` Rik van Riel
2010-04-10 0:31 ` Johannes Weiner
2010-04-10 0:32 ` Linus Torvalds
2010-04-10 7:27 ` Borislav Petkov
2010-04-10 11:26 ` Borislav Petkov
2010-04-10 14:45 ` Rik van Riel
2010-04-10 15:24 ` Linus Torvalds
2010-04-10 16:38 ` Borislav Petkov
2010-04-10 17:05 ` Linus Torvalds
2010-04-10 18:21 ` Linus Torvalds
2010-04-10 18:26 ` Linus Torvalds
2010-04-10 18:51 ` Borislav Petkov
2010-04-10 18:58 ` Borislav Petkov
2010-04-10 20:05 ` Linus Torvalds [this message]
2010-04-10 20:12 ` Linus Torvalds
2010-04-10 20:36 ` Borislav Petkov
2010-04-10 20:40 ` Linus Torvalds
2010-04-10 21:25 ` Borislav Petkov
2010-04-10 21:30 ` Linus Torvalds
2010-04-10 21:51 ` Borislav Petkov
2010-04-11 13:08 ` Borislav Petkov
2010-04-11 13:19 ` [PATCH 1/3] mm: make page freeing path RCU-safe Borislav Petkov
2010-04-11 13:19 ` [PATCH 2/3] mm: cleanup find_mergeable_anon_vma complexity Borislav Petkov
2010-04-11 13:19 ` [PATCH 3/3] mm: fixup vma_adjust Borislav Petkov
2010-04-11 13:25 ` [PATCH 2/3] mm: cleanup find_mergeable_anon_vma complexity Borislav Petkov
2010-04-11 17:07 ` [PATCH -v2] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA Linus Torvalds
2010-04-11 17:16 ` Linus Torvalds
2010-04-11 18:55 ` Borislav Petkov
2010-04-12 0:13 ` Linus Torvalds
2010-04-12 1:04 ` Linus Torvalds
2010-04-12 7:20 ` Borislav Petkov
2010-04-12 16:02 ` Linus Torvalds
2010-04-12 16:26 ` Linus Torvalds
2010-04-12 18:40 ` Rik van Riel
2010-04-12 19:00 ` Borislav Petkov
2010-04-12 19:17 ` Linus Torvalds
2010-04-12 20:22 ` [PATCH 1/4] Simplify and comment on anon_vma re-use for anon_vma_prepare() Linus Torvalds
2010-04-12 20:23 ` [PATCH 2/4] vma_adjust: fix the copying of anon_vma chains Linus Torvalds
2010-04-12 20:23 ` [PATCH 3/4] anon_vma: clone the anon_vma chain in the right order Linus Torvalds
2010-04-12 20:23 ` [PATCH 4/4] anonvma: when setting up page->mapping, we need to pick the _oldest_ anonvma Linus Torvalds
2010-04-12 21:03 ` Rik van Riel
2010-04-13 0:41 ` Johannes Weiner
2010-04-13 1:08 ` Linus Torvalds
2010-04-13 4:23 ` Minchan Kim
2010-04-13 4:26 ` Minchan Kim
2010-04-12 20:57 ` [PATCH 3/4] anon_vma: clone the anon_vma chain in the right order Rik van Riel
2010-04-13 0:18 ` Johannes Weiner
2010-04-13 4:16 ` Minchan Kim
2010-04-12 20:54 ` [PATCH 2/4] vma_adjust: fix the copying of anon_vma chains Rik van Riel
2010-04-12 23:59 ` Johannes Weiner
2010-04-13 4:15 ` Minchan Kim
2010-04-12 20:54 ` [PATCH 1/4] Simplify and comment on anon_vma re-use for anon_vma_prepare() Rik van Riel
2010-04-12 23:54 ` Johannes Weiner
2010-04-13 4:04 ` Minchan Kim
2010-04-13 9:51 ` Peter Zijlstra
2010-04-12 21:50 ` [PATCH -v2] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA Borislav Petkov
2010-04-12 22:11 ` Linus Torvalds
2010-04-12 22:18 ` Linus Torvalds
2010-04-12 22:29 ` Borislav Petkov
2010-04-13 9:38 ` Borislav Petkov
2010-04-14 21:59 ` [PATCH] rmap: add exclusively owned pages to the newest anon_vma Rik van Riel
2010-04-14 23:20 ` Johannes Weiner
2010-04-15 8:34 ` Borislav Petkov
2010-04-15 16:02 ` Minchan Kim
2010-04-15 20:01 ` Linus Torvalds
2010-04-16 6:09 ` Felipe Balbi
2010-04-16 14:48 ` Linus Torvalds
2010-04-11 19:49 ` [PATCH -v2] rmap: make anon_vma_prepare link in all the anon_vmas of a mergeable VMA Rik van Riel
2010-04-12 15:44 ` Linus Torvalds
2010-04-12 15:51 ` Rik van Riel
2010-04-11 21:45 ` Rik van Riel
2010-04-12 15:51 ` Linus Torvalds
2010-04-13 10:36 ` KOSAKI Motohiro
2010-04-10 20:24 ` Rik van Riel
2010-04-10 20:34 ` Linus Torvalds
2010-04-10 20:43 ` Rik van Riel
2010-04-10 20:32 ` Rik van Riel
2010-04-10 19:36 ` Rik van Riel
2010-04-12 14:40 ` Peter Zijlstra
2010-04-12 15:17 ` Minchan Kim
2010-04-12 15:33 ` Peter Zijlstra
2010-04-12 15:19 ` Rik van Riel
2010-04-12 16:01 ` Peter Zijlstra
2010-04-12 16:06 ` Rik van Riel
2010-04-12 16:46 ` Linus Torvalds
2010-04-12 18:40 ` Peter Zijlstra
2010-04-12 19:30 ` Peter Zijlstra
2010-04-12 19:44 ` Peter Zijlstra
2010-04-13 10:53 ` KOSAKI Motohiro
2010-04-13 11:30 ` Peter Zijlstra
2010-04-13 12:00 ` KOSAKI Motohiro
2010-04-14 14:27 ` Peter Zijlstra
2010-04-10 17:07 ` Borislav Petkov
2010-04-10 16:41 ` Linus Torvalds
2010-04-10 22:49 ` Johannes Weiner
2010-04-10 23:31 ` Linus Torvalds
2010-04-09 1:45 ` KOSAKI Motohiro
2010-04-07 15:55 ` Minchan Kim
2010-04-07 7:29 ` Ugly rmap NULL ptr deref oopsie on hibernate (was Linux 2.6.34-rc3) Borislav Petkov
2010-04-07 14:05 ` Paulo Marques
2010-04-07 14:13 ` Borislav Petkov
2010-04-06 23:37 ` Linus Torvalds
2010-04-06 23:22 ` Rik van Riel
2010-04-07 0:10 ` Linus Torvalds
2010-04-07 1:18 ` Rik van Riel
2010-04-07 7:22 ` Borislav Petkov
2010-04-07 10:09 ` Pekka Enberg
2010-04-07 10:12 ` KOSAKI Motohiro
2010-04-07 8:41 ` Peter Zijlstra
2010-04-07 8:36 ` Peter Zijlstra
2010-04-07 9:16 ` Johannes Weiner
2010-04-07 9:37 ` Peter Zijlstra
2010-04-07 14:12 ` Rik van Riel
2010-04-07 15:46 ` Linus Torvalds
2010-04-06 16:32 ` Linus Torvalds
2010-04-06 16:54 ` Minchan Kim
2010-04-07 8:37 ` Peter Zijlstra
2010-04-06 17:05 ` Borislav Petkov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LFD.2.00.1004101234210.3558@i5.linux-foundation.org \
--to=torvalds@linux-foundation.org \
--cc=Lee.Schermerhorn@hp.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=bp@alien8.de \
--cc=hannes@cmpxchg.org \
--cc=hugh.dickins@tiscali.co.uk \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=linux-kernel@vger.kernel.org \
--cc=minchan.kim@gmail.com \
--cc=npiggin@suse.de \
--cc=riel@redhat.com \
--cc=sgunderson@bigfoot.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).