handle suicide on late failure exits in execve() in search_binary_handler()
authorAl Viro <viro@zeniv.linux.org.uk>
Mon, 5 May 2014 00:11:36 +0000 (20:11 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 9 Oct 2014 06:39:00 +0000 (02:39 -0400)
... rather than doing that in the guts of ->load_binary().
[updated to fix the bug spotted by Shentino - for SIGSEGV we really need
something stronger than send_sig_info(); again, better do that in one place]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
arch/x86/ia32/ia32_aout.c
fs/binfmt_aout.c
fs/binfmt_elf.c
fs/binfmt_elf_fdpic.c
fs/exec.c

index d21ff89207cd655cb58c954e70fb351dd774e15a..df91466f973de7d568fe93b3ba64fb625ffc8e05 100644 (file)
@@ -308,11 +308,8 @@ static int load_aout_binary(struct linux_binprm *bprm)
                (current->mm->start_brk = N_BSSADDR(ex));
 
        retval = setup_arg_pages(bprm, IA32_STACK_TOP, EXSTACK_DEFAULT);
-       if (retval < 0) {
-               /* Someone check-me: is this error path enough? */
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                return retval;
-       }
 
        install_exec_creds(bprm);
 
@@ -324,17 +321,13 @@ static int load_aout_binary(struct linux_binprm *bprm)
 
                error = vm_brk(text_addr & PAGE_MASK, map_size);
 
-               if (error != (text_addr & PAGE_MASK)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != (text_addr & PAGE_MASK))
                        return error;
-               }
 
                error = read_code(bprm->file, text_addr, 32,
                                  ex.a_text + ex.a_data);
-               if ((signed long)error < 0) {
-                       send_sig(SIGKILL, current, 0);
+               if ((signed long)error < 0)
                        return error;
-               }
        } else {
 #ifdef WARN_OLD
                static unsigned long error_time, error_time2;
@@ -368,20 +361,16 @@ static int load_aout_binary(struct linux_binprm *bprm)
                                MAP_EXECUTABLE | MAP_32BIT,
                                fd_offset);
 
-               if (error != N_TXTADDR(ex)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != N_TXTADDR(ex))
                        return error;
-               }
 
                error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
                                PROT_READ | PROT_WRITE | PROT_EXEC,
                                MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE |
                                MAP_EXECUTABLE | MAP_32BIT,
                                fd_offset + ex.a_text);
-               if (error != N_DATADDR(ex)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != N_DATADDR(ex))
                        return error;
-               }
        }
 beyond_if:
        set_binfmt(&aout_format);
index ca0ba15a73061fb61aaee9c4951ba6097d10544b..929dec08c348ec01ada445e218f8a1bde05d58f9 100644 (file)
@@ -256,11 +256,8 @@ static int load_aout_binary(struct linux_binprm * bprm)
                (current->mm->start_brk = N_BSSADDR(ex));
 
        retval = setup_arg_pages(bprm, STACK_TOP, EXSTACK_DEFAULT);
-       if (retval < 0) {
-               /* Someone check-me: is this error path enough? */
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                return retval;
-       }
 
        install_exec_creds(bprm);
 
@@ -278,17 +275,13 @@ static int load_aout_binary(struct linux_binprm * bprm)
                map_size = ex.a_text+ex.a_data;
 #endif
                error = vm_brk(text_addr & PAGE_MASK, map_size);
-               if (error != (text_addr & PAGE_MASK)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != (text_addr & PAGE_MASK))
                        return error;
-               }
 
                error = read_code(bprm->file, text_addr, pos,
                                  ex.a_text+ex.a_data);
-               if ((signed long)error < 0) {
-                       send_sig(SIGKILL, current, 0);
+               if ((signed long)error < 0)
                        return error;
-               }
        } else {
                if ((ex.a_text & 0xfff || ex.a_data & 0xfff) &&
                    (N_MAGIC(ex) != NMAGIC) && printk_ratelimit())
@@ -315,28 +308,22 @@ static int load_aout_binary(struct linux_binprm * bprm)
                        MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
                        fd_offset);
 
-               if (error != N_TXTADDR(ex)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != N_TXTADDR(ex))
                        return error;
-               }
 
                error = vm_mmap(bprm->file, N_DATADDR(ex), ex.a_data,
                                PROT_READ | PROT_WRITE | PROT_EXEC,
                                MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE,
                                fd_offset + ex.a_text);
-               if (error != N_DATADDR(ex)) {
-                       send_sig(SIGKILL, current, 0);
+               if (error != N_DATADDR(ex))
                        return error;
-               }
        }
 beyond_if:
        set_binfmt(&aout_format);
 
        retval = set_brk(current->mm->start_brk, current->mm->brk);
-       if (retval < 0) {
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                return retval;
-       }
 
        current->mm->start_stack =
                (unsigned long) create_aout_tables((char __user *) bprm->p, bprm);
index 3892c1a2324143498f3e62fc171a0f5cdc6c44c9..d8fc0605b9d23039b558cc4d1327a40c246caa09 100644 (file)
@@ -738,10 +738,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
           change some of these later */
        retval = setup_arg_pages(bprm, randomize_stack_top(STACK_TOP),
                                 executable_stack);
-       if (retval < 0) {
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                goto out_free_dentry;
-       }
        
        current->mm->start_stack = bprm->p;
 
@@ -763,10 +761,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
                           and clear the area.  */
                        retval = set_brk(elf_bss + load_bias,
                                         elf_brk + load_bias);
-                       if (retval) {
-                               send_sig(SIGKILL, current, 0);
+                       if (retval)
                                goto out_free_dentry;
-                       }
                        nbyte = ELF_PAGEOFFSET(elf_bss);
                        if (nbyte) {
                                nbyte = ELF_MIN_ALIGN - nbyte;
@@ -820,7 +816,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
                error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
                                elf_prot, elf_flags, 0);
                if (BAD_ADDR(error)) {
-                       send_sig(SIGKILL, current, 0);
                        retval = IS_ERR((void *)error) ?
                                PTR_ERR((void*)error) : -EINVAL;
                        goto out_free_dentry;
@@ -851,7 +846,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
                    elf_ppnt->p_memsz > TASK_SIZE ||
                    TASK_SIZE - elf_ppnt->p_memsz < k) {
                        /* set_brk can never work. Avoid overflows. */
-                       send_sig(SIGKILL, current, 0);
                        retval = -EINVAL;
                        goto out_free_dentry;
                }
@@ -883,12 +877,9 @@ static int load_elf_binary(struct linux_binprm *bprm)
         * up getting placed where the bss needs to go.
         */
        retval = set_brk(elf_bss, elf_brk);
-       if (retval) {
-               send_sig(SIGKILL, current, 0);
+       if (retval)
                goto out_free_dentry;
-       }
        if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) {
-               send_sig(SIGSEGV, current, 0);
                retval = -EFAULT; /* Nobody gets to see this, but.. */
                goto out_free_dentry;
        }
@@ -909,7 +900,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
                        elf_entry += loc->interp_elf_ex.e_entry;
                }
                if (BAD_ADDR(elf_entry)) {
-                       force_sig(SIGSEGV, current);
                        retval = IS_ERR((void *)elf_entry) ?
                                        (int)elf_entry : -EINVAL;
                        goto out_free_dentry;
@@ -922,7 +912,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
        } else {
                elf_entry = loc->elf_ex.e_entry;
                if (BAD_ADDR(elf_entry)) {
-                       force_sig(SIGSEGV, current);
                        retval = -EINVAL;
                        goto out_free_dentry;
                }
@@ -934,19 +923,15 @@ static int load_elf_binary(struct linux_binprm *bprm)
 
 #ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
        retval = arch_setup_additional_pages(bprm, !!elf_interpreter);
-       if (retval < 0) {
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                goto out;
-       }
 #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */
 
        install_exec_creds(bprm);
        retval = create_elf_tables(bprm, &loc->elf_ex,
                          load_addr, interp_load_addr);
-       if (retval < 0) {
-               send_sig(SIGKILL, current, 0);
+       if (retval < 0)
                goto out;
-       }
        /* N.B. passed_fileno might not be initialized? */
        current->mm->end_code = end_code;
        current->mm->start_code = start_code;
index fe2a643ee005387719766b6bc4f3041cd410c4b2..d3634bfb7fe187b4de899e809188319c08b6ac23 100644 (file)
@@ -317,8 +317,8 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
                goto error;
 
        /* there's now no turning back... the old userspace image is dead,
-        * defunct, deceased, etc. after this point we have to exit via
-        * error_kill */
+        * defunct, deceased, etc.
+        */
        set_personality(PER_LINUX_FDPIC);
        if (elf_read_implies_exec(&exec_params.hdr, executable_stack))
                current->personality |= READ_IMPLIES_EXEC;
@@ -343,24 +343,22 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
 
        retval = setup_arg_pages(bprm, current->mm->start_stack,
                                 executable_stack);
-       if (retval < 0) {
-               send_sig(SIGKILL, current, 0);
-               goto error_kill;
-       }
+       if (retval < 0)
+               goto error;
 #endif
 
        /* load the executable and interpreter into memory */
        retval = elf_fdpic_map_file(&exec_params, bprm->file, current->mm,
                                    "executable");
        if (retval < 0)
-               goto error_kill;
+               goto error;
 
        if (interpreter_name) {
                retval = elf_fdpic_map_file(&interp_params, interpreter,
                                            current->mm, "interpreter");
                if (retval < 0) {
                        printk(KERN_ERR "Unable to load interpreter\n");
-                       goto error_kill;
+                       goto error;
                }
 
                allow_write_access(interpreter);
@@ -397,7 +395,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
        if (IS_ERR_VALUE(current->mm->start_brk)) {
                retval = current->mm->start_brk;
                current->mm->start_brk = 0;
-               goto error_kill;
+               goto error;
        }
 
        current->mm->brk = current->mm->start_brk;
@@ -410,7 +408,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
        install_exec_creds(bprm);
        if (create_elf_fdpic_tables(bprm, current->mm,
                                    &exec_params, &interp_params) < 0)
-               goto error_kill;
+               goto error;
 
        kdebug("- start_code  %lx", current->mm->start_code);
        kdebug("- end_code    %lx", current->mm->end_code);
@@ -449,12 +447,6 @@ error:
        kfree(interp_params.phdrs);
        kfree(interp_params.loadmap);
        return retval;
-
-       /* unrecoverable error - kill the process */
-error_kill:
-       send_sig(SIGSEGV, current, 0);
-       goto error;
-
 }
 
 /*****************************************************************************/
index a2b42a98c743b80941d7ae1f6caf9ca7158ca1ea..7302b75a9820fa5c6342e098bb8757c31d884a4e 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1372,18 +1372,23 @@ int search_binary_handler(struct linux_binprm *bprm)
                read_unlock(&binfmt_lock);
                bprm->recursion_depth++;
                retval = fmt->load_binary(bprm);
+               read_lock(&binfmt_lock);
+               put_binfmt(fmt);
                bprm->recursion_depth--;
-               if (retval >= 0 || retval != -ENOEXEC ||
-                   bprm->mm == NULL || bprm->file == NULL) {
-                       put_binfmt(fmt);
+               if (retval < 0 && !bprm->mm) {
+                       /* we got to flush_old_exec() and failed after it */
+                       read_unlock(&binfmt_lock);
+                       force_sigsegv(SIGSEGV, current);
+                       return retval;
+               }
+               if (retval != -ENOEXEC || !bprm->file) {
+                       read_unlock(&binfmt_lock);
                        return retval;
                }
-               read_lock(&binfmt_lock);
-               put_binfmt(fmt);
        }
        read_unlock(&binfmt_lock);
 
-       if (need_retry && retval == -ENOEXEC) {
+       if (need_retry) {
                if (printable(bprm->buf[0]) && printable(bprm->buf[1]) &&
                    printable(bprm->buf[2]) && printable(bprm->buf[3]))
                        return retval;