Commit 4b396ba6 authored by Roland McGrath's avatar Roland McGrath Committed by James Toy

In fs/binfmt_elf.c, load_elf_interp() calls padzero() for .bss even if the

PT_LOAD has no PROT_WRITE and no .bss.  This generates EFAULT.

Here is a small test case.  (Yes, there are other, useful PT_INTERP which
have only .text and no .data/.bss.)

	----- ptinterp.S
	_start: .globl _start
		 nop
		 int3
	-----
	$ gcc -m32 -nostartfiles -nostdlib -o ptinterp ptinterp.S
	$ gcc -m32 -Wl,--dynamic-linker=ptinterp -o hello hello.c
	$ ./hello
	Segmentation fault  # during execve() itself

	After applying the patch:
	$ ./hello
	Trace trap  # user-mode execution after execve() finishes

If the ELF headers are actually self-inconsistent, then dying is fine. 
But having no PROT_WRITE segment is perfectly normal and correct if there
is no segment with p_memsz > p_filesz (i.e.  bss).  John Reiser suggested
checking for PROT_WRITE in the bss logic.  I think it makes most sense to
simply apply the bss logic only when there is bss.

This patch looks less trivial than it is due to some reindentation.  It
just moves the "if (last_bss > elf_bss) {" test up to include the
partial-page bss logic as well as the more-pages bss logic.
Reported-by: default avatarJohn Reiser <jreiser@bitwagon.com>
Signed-off-by: default avatarRoland McGrath <roland@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: James Morris <jmorris@namei.org>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
parent 28bc12b2
...@@ -501,6 +501,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, ...@@ -501,6 +501,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
} }
} }
if (last_bss > elf_bss) {
/* /*
* Now fill out the bss section. First pad the last page up * Now fill out the bss section. First pad the last page up
* to the page boundary, and then perform a mmap to make sure * to the page boundary, and then perform a mmap to make sure
...@@ -516,7 +517,6 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, ...@@ -516,7 +517,6 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1); elf_bss = ELF_PAGESTART(elf_bss + ELF_MIN_ALIGN - 1);
/* Map the last of the bss segment */ /* Map the last of the bss segment */
if (last_bss > elf_bss) {
down_write(&current->mm->mmap_sem); down_write(&current->mm->mmap_sem);
error = do_brk(elf_bss, last_bss - elf_bss); error = do_brk(elf_bss, last_bss - elf_bss);
up_write(&current->mm->mmap_sem); up_write(&current->mm->mmap_sem);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment