Commit 2fd4ef85 authored by Hugh Dickins's avatar Hugh Dickins Committed by Linus Torvalds
Browse files

[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: default avatarHugh Dickins <hugh@veritas.com>
Signed-Off-By: default avatarKirill Korotaev <dev@sw.ru>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent fb085cf1

arch/ia64/ia32/binfmt_elf32.c

+0 −6
Original line number Diff line number Diff line
@@ -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);

arch/mips/kernel/sysirix.c

+2 −7
Original line number Diff line number Diff line
@@ -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:

arch/ppc64/kernel/vdso.c

+9 −6
Original line number Diff line number Diff line
@@ -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);

arch/x86_64/ia32/ia32_binfmt.c

+0 −5
Original line number Diff line number Diff line
@@ -354,11 +354,6 @@ int setup_arg_pages(struct linux_binprm *bprm, unsigned long stack_top, int exec 
	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(&mm->mmap_sem);

arch/x86_64/ia32/syscall32.c

+1 −5
Original line number Diff line number Diff line
@@ -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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE