[PATCH] binfmt_elf.c : fix checks for bad address

Fix check for bad address; use macro instead of open-coding two checks.

Explanation from Ernie Petries in 2.6 commit :

  For background, the BAD_ADDR() macro should return TRUE if the address is
  TASK_SIZE, because that's the lowest address that is *not* valid for
  user-space mappings.  The macro was correct in binfmt_aout.c but was wrong
  for the "equal to" case in binfmt_elf.c.  There were two in-line validations
  of user-space addresses in binfmt_elf.c, which have been appropriately
  converted to use the corrected BAD_ADDR() macro in the patch you posted
  yesterday.  Note that the size checks against TASK_SIZE are okay as coded.

Note that this patch slightly differs from Ernie's in that the printk()
only got commented out instead of being removed, since a rate limited call
is expected soon.

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index b0ad905..32c8ec6 100644 (file)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -77,7 +77,7 @@ static struct linux_binfmt elf_format = {
        NULL, THIS_MODULE, load_elf_binary, load_elf_library, elf_core_dump, ELF_EXEC_PAGESIZE
 };
 
-#define BAD_ADDR(x)    ((unsigned long)(x) > TASK_SIZE)
+#define BAD_ADDR(x)    ((unsigned long)(x) >= TASK_SIZE)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -345,7 +345,7 @@ static unsigned long load_elf_interp(struct elfhdr * interp_elf_ex,
             * <= p_memsize so it is only necessary to check p_memsz.
             */
            k = load_addr + eppnt->p_vaddr;
-           if (k > TASK_SIZE || eppnt->p_filesz > eppnt->p_memsz ||
+           if (BAD_ADDR(k) || eppnt->p_filesz > eppnt->p_memsz ||
                eppnt->p_memsz > TASK_SIZE || TASK_SIZE - eppnt->p_memsz < k) {
                error = -ENOMEM;
                goto out_close;
@@ -772,7 +772,7 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
                 * allowed task size. Note that p_filesz must always be
                 * <= p_memsz so it is only necessary to check p_memsz.
                 */
-               if (k > TASK_SIZE || elf_ppnt->p_filesz > elf_ppnt->p_memsz ||
+               if (BAD_ADDR(k) || elf_ppnt->p_filesz > elf_ppnt->p_memsz ||
                    elf_ppnt->p_memsz > TASK_SIZE ||
                    TASK_SIZE - elf_ppnt->p_memsz < k) {
                        /* set_brk can never work.  Avoid overflows.  */
@@ -822,10 +822,13 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
                                                    interpreter,
                                                    &interp_load_addr);
                if (BAD_ADDR(elf_entry)) {
-                       printk(KERN_ERR "Unable to load interpreter %.128s\n",
-                               elf_interpreter);
+                       // FIXME - ratelimit this before re-enabling
+                       // printk(KERN_ERR "Unable to load interpreter %.128s\n",
+                       //        elf_interpreter);
+
                        force_sig(SIGSEGV, current);
-                       retval = IS_ERR((void *)elf_entry) ? PTR_ERR((void *)elf_entry) : -ENOEXEC;
+                       retval = IS_ERR((void *)elf_entry) ?
+                                       (int)elf_entry : -EINVAL;
                        goto out_free_dentry;
                }
                reloc_func_desc = interp_load_addr;
@@ -833,6 +836,12 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
                allow_write_access(interpreter);
                fput(interpreter);
                kfree(elf_interpreter);
+       } else {
+               if (BAD_ADDR(elf_entry)) {
+                       force_sig(SIGSEGV, current);
+                       retval = -EINVAL;
+                       goto out_free_dentry;
+               }
        }
 
        kfree(elf_phdata);