[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>

2 files changed, 1 insertions, 10 deletions

diff --git a/arch/x86_64/ia32/ia32_binfmt.c b/arch/x86_64/ia32/ia32_binfmt.c
index c8131f342cfc..d9161e395978 100644
--- 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 adbc5f8089e9..3a01329473ab 100644
--- 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;