binfmt_elf fails to detect errors

From: Pavel Machek (pavel@suse.cz)
Date: Fri Feb 15 2002 - 19:53:18 EST

Hi!

Error handling is just not there in binfmt_elf.c. Try this test
program:

#define MAXLEN 20
#define MAXTAG 10
double speedup[MAXLEN][MAXLEN][MAXLEN][MAXLEN][MAXTAG][MAXTAG][MAXTAG];

void
init_speedup(void)
{
        int i, j, k, l, o, m, n;
        for (i=0; i<MAXLEN; i++)
                for (j=0; j<MAXLEN; j++)
                        for (k=0; k<MAXLEN; k++)
                                for (l=0; l<MAXLEN; l++)
                                        for (o=0; o<MAXTAG; o++)
                                                for (m=0; m<MAXTAG; m++)
                                                        for (n=0; n<MAXTAG; n++)
                                                                speedup[i][j][k][l][o][m][n] = -1.0;
}

void main(void)
{
        init_speedup();
}

Compile, run, segfault. Okay, but its because binfmt_elf.c fails to
check error returns. Here is would-be patch (it makes it oops, its
probably broken); but there are more non-checked errors in there.

--- clean/fs/binfmt_elf.c Mon Feb 11 20:51:04 2002
+++ linux/fs/binfmt_elf.c Sat Feb 16 01:46:48 2002
@@ -79,13 +79,20 @@
 
 #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) {
+ printk("Error %d doing do_brk\n", error);
+ 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,7 +730,9 @@
         /* 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);
+ if (retval)
+ goto out_free_dentry;
 
         padzero(elf_bss);
 
@@ -745,6 +756,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
@@ -762,6 +774,8 @@
                 send_sig(SIGTRAP, current, 0);
         retval = 0;
 out:
+ if (retval)
+ printk("binfmt_elf: Returnning %d\n", retval);
         return retval;
 
         /* error cleanup */
@@ -842,9 +856,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);
 
                                                                        Pavel
PS: Who is right person to talk with? I choose Al because he likes to
kill bugs in fs/ where binfmt_elf resides, but... 

-- 
(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
-
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/

This archive was generated by hypermail 2b29 : Sat Feb 23 2002 - 21:00:13 EST