mm: Hold a file reference in madvise_remove
 

From: Andy Lutomirski <luto@amacapital.net>

CVE-2012-3511

BugLink: http://bugs.launchpad.net/bugs/1042447

Otherwise the code races with munmap (causing a use-after-free
of the vma) or with close (causing a use-after-free of the struct
file).

The bug was introduced by commit 90ed52ebe481 ("[PATCH] holepunch: fix
mmap_sem i_mutex deadlock")

Cc: Hugh Dickins <hugh@veritas.com>
Cc: Miklos Szeredi <mszeredi@suse.cz>
Cc: Badari Pulavarty <pbadari@us.ibm.com>
Cc: Nick Piggin <npiggin@suse.de>
Cc: stable@vger.kernel.org
Signed-off-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
(back ported from commit 9ab4233dd08036fe34a89c7dc6f47a8bf2eb29eb)

Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
mm/madvise.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 35b1479..e51291d 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c
@@ -5,6 +5,7 @@
* Copyright (C) 2002 Christoph Hellwig
*/

+#include <linux/file.h>
#include <linux/mman.h>
#include <linux/pagemap.h>
#include <linux/syscalls.h>
@@ -190,14 +191,16 @@ static long madvise_remove(struct vm_area_struct *vma,
struct address_space *mapping;
loff_t offset, endoff;
int error;
+ struct file *f;

*prev = NULL; /* tell sys_madvise we drop mmap_sem */

if (vma->vm_flags & (VM_LOCKED|VM_NONLINEAR|VM_HUGETLB))
return -EINVAL;

- if (!vma->vm_file || !vma->vm_file->f_mapping
- || !vma->vm_file->f_mapping->host) {
+ f = vma->vm_file;
+
+ if (!f || !f->f_mapping || !f->f_mapping->host) {
return -EINVAL;
}

@@ -211,9 +214,16 @@ static long madvise_remove(struct vm_area_struct *vma,
endoff = (loff_t)(end - vma->vm_start - 1)
+ ((loff_t)vma->vm_pgoff << PAGE_SHIFT);

- /* vmtruncate_range needs to take i_mutex and i_alloc_sem */
+ /*
+ * Filesystem's fallocate may need to take i_mutex. We need to
+ * explicitly grab a reference because the vma (and hence the
+ * vma's reference to the file) can go away as soon as we drop
+ * mmap_sem.
+ */
+ get_file(f);
up_read(&current->mm->mmap_sem);
error = vmtruncate_range(mapping->host, offset, endoff);
+ fput(f);
down_read(&current->mm->mmap_sem);
return error;
}
--
1.7.9.5


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team

mm: Hold a file reference in madvise_remove
 

On Thu, Aug 30, 2012 at 09:22:48AM -0600, Tim Gardner wrote:
> @@ -211,9 +214,16 @@ static long madvise_remove(struct vm_area_struct *vma,
> endoff = (loff_t)(end - vma->vm_start - 1)
> + ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
>
> - /* vmtruncate_range needs to take i_mutex and i_alloc_sem */
> + /*
> + * Filesystem's fallocate may need to take i_mutex. We need to

Just a minor nit here: for older versions it's still vmtruncate_range,
so perhaps the comment could be kept as:

"vmtruncate_range needs to take i_mutex and i_alloc_sem. We need to..."

but it doesn't change the result, just noted this. In any case Ack on
the backport.

> + * explicitly grab a reference because the vma (and hence the
> + * vma's reference to the file) can go away as soon as we drop
> + * mmap_sem.
> + */
> + get_file(f);
> up_read(&current->mm->mmap_sem);
> error = vmtruncate_range(mapping->host, offset, endoff);
> + fput(f);
> down_read(&current->mm->mmap_sem);
> return error;
> }
> --
> 1.7.9.5

--
[]'s
Herton

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team