| 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
| ||
|
Message-ID: <20100409013012.GA8153@liondog.tnic>
Date: Fri, 9 Apr 2010 03:30:12 +0200
From: Borislav Petkov <bp@...en8.de>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
Rik van Riel <riel@...hat.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Minchan Kim <minchan.kim@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Lee Schermerhorn <Lee.Schermerhorn@...com>,
Nick Piggin <npiggin@...e.de>,
Andrea Arcangeli <aarcange@...hat.com>,
Hugh Dickins <hugh.dickins@...cali.co.uk>,
sgunderson@...foot.com, hannes@...xchg.org
Subject: Re: [PATCH -v2] rmap: make anon_vma_prepare link in all the
anon_vmas of a mergeable VMA
From: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Thu, Apr 08, 2010 at 05:50:21PM -0700
> > Yep, looks good: its mmap_region()...
>
> Can you double-check your current diffs - maybe something got corrupted.
>
> mmap_region installs the vma with vma_link(), and the last thing
> vma_link() does with my patch is that "anon_vma_prepare()".
Right, it looks like it. I'll add some more debugging calls there
tomorrow - it might give us more clues in case someone hasn't caught it
until then.
> Maybe with all the patches flying around, you had a reject or something,
> and you lost that one anon_vma_prepare()?
>
> Or maybe I screwed up somewhere and sent you the wrong patch. Here it is
> again, just in case.
Doesn't look like it - here's the diff between yours and what I have
applied here (yep, only minor fuzz but no code differences) Also, I've
added my version at the end:
--- a.diff 2010-04-09 03:03:35.000000000 +0200
+++ b.diff 2010-04-09 03:03:52.000000000 +0200
@@ -1,8 +1,8 @@
diff --git a/mm/memory.c b/mm/memory.c
-index 1d2ea39..bd7ea7f 100644
+index 833952d..08d4423 100644
--- a/mm/memory.c
+++ b/mm/memory.c
-@@ -2224,9 +2224,6 @@ reuse:
+@@ -2223,9 +2223,6 @@ reuse:
gotten:
pte_unmap_unlock(page_table, ptl);
@@ -12,7 +12,7 @@ index 1d2ea39..bd7ea7f 100644
if (is_zero_pfn(pte_pfn(orig_pte))) {
new_page = alloc_zeroed_user_highpage_movable(vma, address);
if (!new_page)
-@@ -2767,8 +2764,6 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
+@@ -2766,8 +2763,6 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
/* Allocate our own private page. */
pte_unmap(page_table);
@@ -21,7 +21,7 @@ index 1d2ea39..bd7ea7f 100644
page = alloc_zeroed_user_highpage_movable(vma, address);
if (!page)
goto oom;
-@@ -2864,10 +2859,6 @@ static int __do_fault(struct mm_struct *mm, struct vm_area_struct *vma,
+@@ -2863,10 +2858,6 @@ static int __do_fault(struct mm_struct *mm, struct vm_area_struct *vma,
if (flags & FAULT_FLAG_WRITE) {
if (!(vma->vm_flags & VM_SHARED)) {
anon = 1;
@@ -32,7 +32,7 @@ index 1d2ea39..bd7ea7f 100644
page = alloc_page_vma(GFP_HIGHUSER_MOVABLE,
vma, address);
if (!page) {
-@@ -3116,6 +3107,9 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
+@@ -3115,6 +3106,9 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
pmd_t *pmd;
pte_t *pte;
@@ -43,7 +43,7 @@ index 1d2ea39..bd7ea7f 100644
count_vm_event(PGFAULT);
diff --git a/mm/mmap.c b/mm/mmap.c
-index bf0600c..4592a93 100644
+index 75557c6..82392c2 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -463,6 +463,8 @@ static void vma_link(struct mm_struct *mm, struct vm_area_struct *vma,
> [ I have a horrible cold, and can hardly think straight. So who knows,
> maybe I'm missing something. But if you have lost one of the
> 'anon_vma_prepare()' call sites, that would certainly explain why you
> get NULL anon_vma's ]
Oh, sorry to hear that. Ok, let's stop for today - it is 3am here and
even if some would say, "well, this is just getting interesting" :), I
think it would be best to "sleep on it." :)
Thanks.
--
commit 2156db98fd84d07e3b86564f429fcc8c6b7d61df
Author: Linus Torvalds <torvalds@...ux-foundation.org>
Date: Thu Apr 8 22:09:53 2010 +0200
rmap: preallocate anon VMAs
On Thu, 8 Apr 2010, Borislav Petkov wrote:
>
> There are still issues: vma_adjust() grabs mapping->i_mmap_lock for file
> mappings while we might sleep in anon_vma_prepare():
Ahh. Good catch. So I can't actually do that anon_vma_prepare() thing in
__insert_vm_struct.
It should be simple enough to just move it into the caller, just after it
releases that lock. There's only one user of that __insert_vm_struct()
anyway. You can do it yourself, or you can replace my previous patch with
this..
[ The patch below also makes it warn once and return SIGBUS for the case
where there is no anon_vma. I decided I still want to hear about it if
there might be some path that tries to insert a vma on its own ]
Linus
diff --git a/mm/memory.c b/mm/memory.c
index 1d2ea39..bd7ea7f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2224,9 +2224,6 @@ reuse:
gotten:
pte_unmap_unlock(page_table, ptl);
- if (unlikely(anon_vma_prepare(vma)))
- goto oom;
-
if (is_zero_pfn(pte_pfn(orig_pte))) {
new_page = alloc_zeroed_user_highpage_movable(vma, address);
if (!new_page)
@@ -2767,8 +2764,6 @@ static int do_anonymous_page(struct mm_struct *mm, struct vm_area_struct *vma,
/* Allocate our own private page. */
pte_unmap(page_table);
- if (unlikely(anon_vma_prepare(vma)))
- goto oom;
page = alloc_zeroed_user_highpage_movable(vma, address);
if (!page)
goto oom;
@@ -2864,10 +2859,6 @@ static int __do_fault(struct mm_struct *mm, struct vm_area_struct *vma,
if (flags & FAULT_FLAG_WRITE) {
if (!(vma->vm_flags & VM_SHARED)) {
anon = 1;
- if (unlikely(anon_vma_prepare(vma))) {
- ret = VM_FAULT_OOM;
- goto out;
- }
page = alloc_page_vma(GFP_HIGHUSER_MOVABLE,
vma, address);
if (!page) {
@@ -3116,6 +3107,9 @@ int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
pmd_t *pmd;
pte_t *pte;
+ if (WARN_ONCE(!vma->anon_vma, "Mapping with no anon_vma"))
+ return VM_FAULT_SIGBUS;
+
__set_current_state(TASK_RUNNING);
count_vm_event(PGFAULT);
diff --git a/mm/mmap.c b/mm/mmap.c
index bf0600c..4592a93 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -463,6 +463,8 @@ static void vma_link(struct mm_struct *mm, struct vm_area_struct *vma,
mm->map_count++;
validate_mm(mm);
+
+ anon_vma_prepare(vma);
}
/*
@@ -628,6 +630,8 @@ again: remove_next = 1 + (end > next->vm_end);
if (mapping)
spin_unlock(&mapping->i_mmap_lock);
+ anon_vma_prepare(vma);
+
if (remove_next) {
if (file) {
fput(file);
@@ -1674,12 +1678,6 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
if (!(vma->vm_flags & VM_GROWSUP))
return -EFAULT;
- /*
- * We must make sure the anon_vma is allocated
- * so that the anon_vma locking is not a noop.
- */
- if (unlikely(anon_vma_prepare(vma)))
- return -ENOMEM;
anon_vma_lock(vma);
/*
@@ -1720,13 +1718,6 @@ static int expand_downwards(struct vm_area_struct *vma,
{
int error;
- /*
- * We must make sure the anon_vma is allocated
- * so that the anon_vma locking is not a noop.
- */
- if (unlikely(anon_vma_prepare(vma)))
- return -ENOMEM;
-
address &= PAGE_MASK;
error = security_file_mmap(NULL, 0, 0, 0, address, 1);
if (error)
--
Regards/Gruss,
Boris.
--
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/
Powered by blists - more mailing lists