RE: [PATCH 1/2] setup_arg_pages can insert overlapping vma

From: Zou, Nanhai <nanhai.zou_at_intel.com>
Date: 2004-11-24 12:04:28
 <<ia64-vm-overlap.tar.gz>>  <<vma-overlap-fix.patch>> I think ia64 ia32
subsystem is not vulnerable to this kind of overlapping vm problem,
because it does not support a.out binary format, 
X84_64 is vulnerable to this. 

just do a 
perl -e'print"\x07\x01".("\x00"x10)."\x00\xe0\xff\xff".("\x00"x16)'>
evilaout
you will get it.
 
and IA64 is also vulnerable to this kind of bug in 64 bit elf support,
it just insert a vma of zero page without checking overlap, so user can
construct a elf with section begin from 0x0 to trigger this BUGON().I
attach a testcase to trigger this bug
I don't know what about s390. However, I think it's safe to check
overlap before we actually insert a vma into vma list.
 
And I also feel check vma overlap everywhere is unnecessary, because
invert_vm_struct will check it again, so the check is duplicated. It's
better to have invert_vm_struct return a value then let caller check if
it successes.
Here is a patch against 2.6.10.rc2-mm3
I have tested it on i386, x86_64 and ia64 machines.

Signed-off-by: Tony Luck <tony.luck@intel.com>
Signed-off-by: Zou Nan hai <Nanhai.zou@intel.com>


-----Original Message-----
From: linux-kernel-owner@vger.kernel.org
[mailto:linux-kernel-owner@vger.kernel.org] On Behalf Of Hugh Dickins
Sent: Friday, November 19, 2004 1:40 AM
To: Chris Wright
Cc: Andrew Morton; Linus Torvalds; Luck, Tony; Martin Schwidefsky; Andi
Kleen; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] setup_arg_pages can insert overlapping vma

On Tue, 16 Nov 2004, Chris Wright wrote:
> Florian Heinz built an a.out binary that could map bss from 0x0 to
> 0xc0000000, and setup_arg_pages() would BUG() in insert_vma_struct
> because the arg pages overlapped.  This just checks before inserting,
> and bails out if it would overlap.

Chris, shouldn't your patch also cover the setup_arg_pages clones for
32-bit support on 64-bit architectures, with something - uncompiled,
untested - like the below?  I'm not sure how necessary the additional
vma->vm_start < mpnt->vm_end test is, but suspect ia64 might need it.

Signed-off-by: Hugh Dickins <hugh@veritas.com>

--- 2.6.10-rc2-bk2/arch/ia64/ia32/binfmt_elf32.c	2004-10-18
22:57:03.000000000 +0100
+++ linux/arch/ia64/ia32/binfmt_elf32.c	2004-11-18 17:17:57.000000000
+0000
@@ -214,6 +214,8 @@ ia32_setup_arg_pages (struct linux_binpr
 
 	down_write(&current->mm->mmap_sem);
 	{
+		struct vm_area_struct *vma;
+
 		mpnt->vm_mm = current->mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
@@ -225,6 +227,12 @@ ia32_setup_arg_pages (struct linux_binpr
 			mpnt->vm_flags = VM_STACK_FLAGS;
 		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC)?
 					PAGE_COPY_EXEC: PAGE_COPY;
+		vma = find_vma(current->mm, mpnt->vm_start);
+		if (vma && vma->vm_start < mpnt->vm_end) {
+			up_write(&current->mm->mmap_sem);
+			kmem_cache_free(vm_area_cachep, mpnt);
+			return -ENOMEM;
+		}
 		insert_vm_struct(current->mm, mpnt);
 		current->mm->stack_vm = current->mm->total_vm =
vma_pages(mpnt);
 	}
--- 2.6.10-rc2-bk2/arch/s390/kernel/compat_exec.c	2004-10-18
22:56:50.000000000 +0100
+++ linux/arch/s390/kernel/compat_exec.c	2004-11-18
17:17:57.000000000 +0000
@@ -62,12 +62,20 @@ int setup_arg_pages32(struct linux_binpr
 
 	down_write(&mm->mmap_sem);
 	{
+		struct vm_area_struct *vma;
+
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
 		/* executable stack setting would be applied here */
 		mpnt->vm_page_prot = PAGE_COPY;
 		mpnt->vm_flags = VM_STACK_FLAGS;
+		vma = find_vma(mm, mpnt->vm_start);
+		if (vma && vma->vm_start < mpnt->vm_end) {
+			up_write(&mm->mmap_sem);
+			kmem_cache_free(vm_area_cachep, mpnt);
+			return -ENOMEM;
+		}
 		insert_vm_struct(mm, mpnt);
 		mm->stack_vm = mm->total_vm = vma_pages(mpnt);
 	} 
--- 2.6.10-rc2-bk2/arch/x86_64/ia32/ia32_binfmt.c	2004-11-15
16:20:34.000000000 +0000
+++ linux/arch/x86_64/ia32/ia32_binfmt.c	2004-11-18
17:17:57.000000000 +0000
@@ -357,6 +357,8 @@ int setup_arg_pages(struct linux_binprm 
 
 	down_write(&mm->mmap_sem);
 	{
+		struct vm_area_struct *vma;
+
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
@@ -368,6 +370,12 @@ int setup_arg_pages(struct linux_binprm 
 			mpnt->vm_flags = VM_STACK_FLAGS;
  		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC) ? 
  			PAGE_COPY_EXEC : PAGE_COPY;
+		vma = find_vma(mm, mpnt->vm_start);
+		if (vma && vma->vm_start < mpnt->vm_end) {
+			up_write(&mm->mmap_sem);
+			kmem_cache_free(vm_area_cachep, mpnt);
+			return -ENOMEM;
+		}
 		insert_vm_struct(mm, mpnt);
 		mm->stack_vm = mm->total_vm = vma_pages(mpnt);
 	} 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Received on Tue Nov 23 20:09:37 2004

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:32 EST