[PATCH] error path in setup_arg_pages() misses vm_unacct_memory()

Pavel Emelianov and Kirill Korotaev observe that fs and arch users of
security_vm_enough_memory tend to forget to vm_unacct_memory when a
failure occurs further down (typically in setup_arg_pages variants).

These are all users of insert_vm_struct, and that reservation will only
be unaccounted on exit if the vma is marked VM_ACCOUNT: which in some
cases it is (hidden inside VM_STACK_FLAGS) and in some cases it isn't.

So x86_64 32-bit and ppc64 vDSO ELFs have been leaking memory into
Committed_AS each time they're run.  But don't add VM_ACCOUNT to them,
it's inappropriate to reserve against the very unlikely case that gdb
be used to COW a vDSO page - we ought to do something about that in
do_wp_page, but there are yet other inconsistencies to be resolved.

The safe and economical way to fix this is to let insert_vm_struct do
the security_vm_enough_memory check when it finds VM_ACCOUNT is set.

And the MIPS irix_brk has been calling security_vm_enough_memory before
calling do_brk which repeats it, doubly accounting and so also leaking.
Remove that, and all the fs and arch calls to security_vm_enough_memory:
give it a less misleading name later on.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-Off-By: Kirill Korotaev <dev@sw.ru>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
index 31de70b..a7280d9 100644 (file)
--- a/arch/ia64/ia32/binfmt_elf32.c
+++ b/arch/ia64/ia32/binfmt_elf32.c
@@ -216,12 +216,6 @@ ia32_setup_arg_pages (struct linux_binprm *bprm, int executable_stack)
        if (!mpnt)
                return -ENOMEM;
 
-       if (security_vm_enough_memory((IA32_STACK_TOP - (PAGE_MASK & (unsigned long) bprm->p))
-                                     >> PAGE_SHIFT)) {
-               kmem_cache_free(vm_area_cachep, mpnt);
-               return -ENOMEM;
-       }
-
        memset(mpnt, 0, sizeof(*mpnt));
 
        down_write(&current->mm->mmap_sem);

diff --git a/arch/mips/kernel/sysirix.c b/arch/mips/kernel/sysirix.c
index 4de1556..7ae4af4 100644 (file)
--- a/arch/mips/kernel/sysirix.c
+++ b/arch/mips/kernel/sysirix.c
@@ -581,18 +581,13 @@ asmlinkage int irix_brk(unsigned long brk)
        }
 
        /*
-        * Check if we have enough memory..
+        * Ok, looks good - let it rip.
         */
-       if (security_vm_enough_memory((newbrk-oldbrk) >> PAGE_SHIFT)) {
+       if (do_brk(oldbrk, newbrk-oldbrk) != oldbrk) {
                ret = -ENOMEM;
                goto out;
        }
-
-       /*
-        * Ok, looks good - let it rip.
-        */
        mm->brk = brk;
-       do_brk(oldbrk, newbrk-oldbrk);
        ret = 0;
 
 out:

diff --git a/arch/ppc64/kernel/vdso.c b/arch/ppc64/kernel/vdso.c
index 4777676..efa985f 100644 (file)
--- a/arch/ppc64/kernel/vdso.c
+++ b/arch/ppc64/kernel/vdso.c
@@ -224,10 +224,7 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int executable_stack)
        vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
        if (vma == NULL)
                return -ENOMEM;
-       if (security_vm_enough_memory(vdso_pages)) {
-               kmem_cache_free(vm_area_cachep, vma);
-               return -ENOMEM;
-       }
+
        memset(vma, 0, sizeof(*vma));
 
        /*
@@ -237,8 +234,10 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int executable_stack)
         */
        vdso_base = get_unmapped_area(NULL, vdso_base,
                                      vdso_pages << PAGE_SHIFT, 0, 0);
-       if (vdso_base & ~PAGE_MASK)
+       if (vdso_base & ~PAGE_MASK) {
+               kmem_cache_free(vm_area_cachep, vma);
                return (int)vdso_base;
+       }
 
        current->thread.vdso_base = vdso_base;
 
@@ -266,7 +265,11 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int executable_stack)
        vma->vm_ops = &vdso_vmops;
 
        down_write(&mm->mmap_sem);
-       insert_vm_struct(mm, vma);
+       if (insert_vm_struct(mm, vma)) {
+               up_write(&mm->mmap_sem);
+               kmem_cache_free(vm_area_cachep, vma);
+               return -ENOMEM;
+       }
        mm->total_vm += (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
        up_write(&mm->mmap_sem);
 

diff --git a/arch/x86_64/ia32/ia32_binfmt.c b/arch/x86_64/ia32/ia32_binfmt.c
index c8131f3..d9161e3 100644 (file)
--- a/arch/x86_64/ia32/ia32_binfmt.c
+++ b/arch/x86_64/ia32/ia32_binfmt.c
@@ -353,11 +353,6 @@ int setup_arg_pages(struct linux_binprm *bprm, unsigned long stack_top, int exec
        mpnt = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
        if (!mpnt) 
                return -ENOMEM; 
-       
-       if (security_vm_enough_memory((IA32_STACK_TOP - (PAGE_MASK & (unsigned long) bprm->p))>>PAGE_SHIFT)) {
-               kmem_cache_free(vm_area_cachep, mpnt);
-               return -ENOMEM;
-       }
 
        memset(mpnt, 0, sizeof(*mpnt));
 

diff --git a/arch/x86_64/ia32/syscall32.c b/arch/x86_64/ia32/syscall32.c
index adbc5f8..3a01329 100644 (file)
--- a/arch/x86_64/ia32/syscall32.c
+++ b/arch/x86_64/ia32/syscall32.c
@@ -52,17 +52,13 @@ int syscall32_setup_pages(struct linux_binprm *bprm, int exstack)
        vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
        if (!vma)
                return -ENOMEM;
-       if (security_vm_enough_memory(npages)) {
-               kmem_cache_free(vm_area_cachep, vma);
-               return -ENOMEM;
-       }
 
        memset(vma, 0, sizeof(struct vm_area_struct));
        /* Could randomize here */
        vma->vm_start = VSYSCALL32_BASE;
        vma->vm_end = VSYSCALL32_END;
        /* MAYWRITE to allow gdb to COW and set breakpoints */
-       vma->vm_flags = VM_READ|VM_EXEC|VM_MAYREAD|VM_MAYEXEC|VM_MAYEXEC|VM_MAYWRITE;
+       vma->vm_flags = VM_READ|VM_EXEC|VM_MAYREAD|VM_MAYEXEC|VM_MAYWRITE;
        vma->vm_flags |= mm->def_flags;
        vma->vm_page_prot = protection_map[vma->vm_flags & 7];
        vma->vm_ops = &syscall32_vm_ops;

diff --git a/fs/exec.c b/fs/exec.c
index 6fae59a..a04a575 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -421,11 +421,6 @@ int setup_arg_pages(struct linux_binprm *bprm,
        if (!mpnt)
                return -ENOMEM;
 
-       if (security_vm_enough_memory(arg_size >> PAGE_SHIFT)) {
-               kmem_cache_free(vm_area_cachep, mpnt);
-               return -ENOMEM;
-       }
-
        memset(mpnt, 0, sizeof(*mpnt));
 
        down_write(&mm->mmap_sem);

diff --git a/mm/mmap.c b/mm/mmap.c
index 12334ae..8b8e05f 100644 (file)
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1993,6 +1993,9 @@ int insert_vm_struct(struct mm_struct * mm, struct vm_area_struct * vma)
        __vma = find_vma_prepare(mm,vma->vm_start,&prev,&rb_link,&rb_parent);
        if (__vma && __vma->vm_start < vma->vm_end)
                return -ENOMEM;
+       if ((vma->vm_flags & VM_ACCOUNT) &&
+            security_vm_enough_memory(vma_pages(vma)))
+               return -ENOMEM;
        vma_link(mm, vma, prev, rb_link, rb_parent);
        return 0;
 }