execve() fails to report errors

execve() fails to report errors

[Pavel Machek]

Hi!

Take this trivial .c program. Obviously correct.


struct foo {
        char fill[1*1024*1024*1024];
};

struct foo a;

void
main(void)
{
}

Compile. Run. Segfault.

Whose fault? Kernels; it fails to corectly report not enough address
space.

Now, I do not know if this is the right fix (binfmt_elf looks
spaghetty to me) but its certainly better than it was.

							Pavel

--- clean/fs/binfmt_elf.c	Tue Mar  5 21:52:44 2002
+++ linux/fs/binfmt_elf.c	Wed Mar  6 00:14:31 2002
@@ -79,13 +79,19 @@
 
 #define BAD_ADDR(x)	((unsigned long)(x) > TASK_SIZE)
 
-static void set_brk(unsigned long start, unsigned long end)
+static int set_brk(unsigned long start, unsigned long end)
 {
+	int error;
 	start = ELF_PAGEALIGN(start);
 	end = ELF_PAGEALIGN(end);
 	if (end <= start)
-		return;
-	do_brk(start, end - start);
+		return 0;
+	
+	error = do_brk(start, end - start);
+	if (error!=start) {
+		return error;
+	}
+	return 0;
 }
 
 
@@ -606,7 +613,9 @@
 			/* There was a PT_LOAD segment with p_memsz > p_filesz
 			   before this one. Map anonymous pages, if needed,
 			   and clear the area.  */
-			set_brk (elf_bss + load_bias, elf_brk + load_bias);
+			retval = set_brk (elf_bss + load_bias, elf_brk + load_bias);
+			if (retval)
+				goto out_free_dentry;
 			nbyte = ELF_PAGEOFFSET(elf_bss);
 			if (nbyte) {
 				nbyte = ELF_MIN_ALIGN - nbyte;
@@ -721,8 +730,7 @@
 	/* Calling set_brk effectively mmaps the pages that we need
 	 * for the bss and break sections
 	 */
-	set_brk(elf_bss, elf_brk);
-
+	retval = set_brk(elf_bss, elf_brk);
 	padzero(elf_bss);
 
 #if 0
@@ -745,6 +753,7 @@
 		error = do_mmap(NULL, 0, 4096, PROT_READ | PROT_EXEC,
 				MAP_FIXED | MAP_PRIVATE, 0);
 		up_write(&current->mm->mmap_sem);
+		/* FIXME: Check return from mmap! */
 	}
 
 #ifdef ELF_PLAT_INIT
@@ -760,8 +769,7 @@
 	start_thread(regs, elf_entry, bprm->p);
 	if (current->ptrace & PT_PTRACED)
 		send_sig(SIGTRAP, current, 0);
-	retval = 0;
 out:
 	return retval;
 
 	/* error cleanup */
@@ -842,9 +852,9 @@
 
 	len = ELF_PAGESTART(elf_phdata->p_filesz + elf_phdata->p_vaddr + ELF_MIN_ALIGN - 1);
 	bss = elf_phdata->p_memsz + elf_phdata->p_vaddr;
-	if (bss > len)
-		do_brk(len, bss - len);
 	error = 0;
+	if (bss > len)
+		error = do_brk(len, bss - len);
 
 out_free_ph:
 	kfree(elf_phdata);

-- 
(about SSSCA) "I don't say this lightly.  However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa

Re: execve() fails to report errors

[Richard B. Johnson]

On Wed, 6 Mar 2002, Pavel Machek wrote:

> Hi!
> 
> Take this trivial .c program. Obviously correct.
> 
> 
> struct foo {
>         char fill[1*1024*1024*1024];
> };
> 
> struct foo a;
> 
> void
> main(void)
> {
> }
> 
> Compile. Run. Segfault.
> 
> Whose fault? Kernels; it fails to corectly report not enough address
> space.

I think the bug has to be found earlier on up the food chain.
If you do:


char a[1*1024*1024*1024] = {0,};


int main(void);
int main()
{
    return 0;
}


The linker will run forever, un-killable...

      gcc -o xxx xxx.c 
     0     0 31478 31466   9   0    752   280 wait4       S    1  0:00
      /usr/local/lib/gcc-lib/i686-pc-linux-gnu/egcs-2.91.66/collect2
      -m elf_i386 -dynamic-linke
     0     0 31479 31478  14   0 1050176 255184 lock_page   D    1  0:23
      /usr/local/i686-pc-linux-gnu/bin/ld -m elf_i386 -dynamic-linker
      /lib/ld-linux.so.2 -o xxx

I can kill the top-level, but the task in lock_page runs forever, paging
to swap-file like crazy.

The output file grew to 1 gb, then stopped.


Cheers,
Dick Johnson

Penguin : Linux version 2.4.18 on an i686 machine (799.53 BogoMips).

	Bill Gates? Who?

Re: execve() fails to report errors

[Pavel Machek]

Hi!

> > Take this trivial .c program. Obviously correct.
> > 
> > 
> > struct foo {
> >         char fill[1*1024*1024*1024];
> > };
> > 
> > struct foo a;
> > 
> > void
> > main(void)
> > {
> > }
> > 
> > Compile. Run. Segfault.
> > 
> > Whose fault? Kernels; it fails to corectly report not enough address
> > space.
> 
> I think the bug has to be found earlier on up the food chain.
> If you do:

Well, nothing earlier can know if there  will be enough address space
at runtime. (It depends on kernel config .. 3GB vs 2GB ...)
								Pavel

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: execve() fails to report errors

[Russell King]

On Wed, Mar 06, 2002 at 12:34:37AM +0100, Pavel Machek wrote:
> Now, I do not know if this is the right fix (binfmt_elf looks
> spaghetty to me) but its certainly better than it was.

Here's another fix in this area.  If setup_arg_pages() fails, we continue
although nothing went wrong.  The following patch kills the process
instead.

Linus/Marcelo - please apply.

--- orig/fs/binfmt_elf.c	Fri Mar 15 10:14:29 2002
+++ linux/fs/binfmt_elf.c	Mon Mar 11 17:29:03 2002
@@ -585,7 +585,12 @@
 	/* Do this so that we can load the interpreter, if need be.  We will
 	   change some of these later */
 	current->mm->rss = 0;
-	setup_arg_pages(bprm); /* XXX: check error */
+	retval = setup_arg_pages(bprm);
+	if (retval < 0) {
+		send_sig(SIGKILL, current, 0);
+		return retval;
+	}
+	
 	current->mm->start_stack = bprm->p;
 
 	/* Now we do a little grungy work by mmaping the ELF image into


-- 
Russell King (rmk@arm.linux.org.uk)                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html