[PATCH] use new unshare_files helper

From: Chris Wright <chrisw@osdl.org>

Use unshare_files during binary loading to eliminate potential leak of
the binary's fd installed during execve().  As is, this breaks
binfmt_som.c

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 969fdbbba9d449237054f85c7d4366c0e9a9898d..69552f9854df896a91013cd93b04098bc3542f52 100644 (file)
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -466,6 +466,7 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
        struct elfhdr interp_elf_ex;
        struct exec interp_ex;
        char passed_fileno[6];
+       struct files_struct *files;
        
        /* Get the exec-header */
        elf_ex = *((struct elfhdr *) bprm->buf);
@@ -498,9 +499,20 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
        if (retval < 0)
                goto out_free_ph;
 
+       files = current->files;         /* Refcounted so ok */
+       if(unshare_files() < 0)
+               goto out_free_ph;
+       if (files == current->files) {
+               put_files_struct(files);
+               files = NULL;
+       }
+
+       /* exec will make our files private anyway, but for the a.out
+          loader stuff we need to do it earlier */
+
        retval = get_unused_fd();
        if (retval < 0)
-               goto out_free_ph;
+               goto out_free_fh;
        get_file(bprm->file);
        fd_install(elf_exec_fileno = retval, bprm->file);
 
@@ -631,6 +643,12 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
        if (retval)
                goto out_free_dentry;
 
+       /* Discard our unneeded old files struct */
+       if (files) {
+               put_files_struct(files);
+               files = NULL;
+       }
+
        /* OK, This is the point of no return */
        current->mm->start_data = 0;
        current->mm->end_data = 0;
@@ -745,19 +763,17 @@ static int load_elf_binary(struct linux_binprm * bprm, struct pt_regs * regs)
                        elf_entry = load_elf_interp(&interp_elf_ex,
                                                    interpreter,
                                                    &interp_load_addr);
-
-               allow_write_access(interpreter);
-               fput(interpreter);
-               kfree(elf_interpreter);
-
                if (BAD_ADDR(elf_entry)) {
                        printk(KERN_ERR "Unable to load interpreter\n");
-                       kfree(elf_phdata);
                        send_sig(SIGSEGV, current, 0);
                        retval = -ENOEXEC; /* Nobody gets to see this, but.. */
-                       goto out;
+                       goto out_free_dentry;
                }
                reloc_func_desc = interp_load_addr;
+
+               allow_write_access(interpreter);
+               fput(interpreter);
+               kfree(elf_interpreter);
        } else {
                elf_entry = elf_ex.e_entry;
        }
@@ -835,6 +851,11 @@ out_free_interp:
                kfree(elf_interpreter);
 out_free_file:
        sys_close(elf_exec_fileno);
+out_free_fh:
+       if (files) {
+               put_files_struct(current->files);
+               current->files = files;
+       }
 out_free_ph:
        kfree(elf_phdata);
        goto out;

diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index 3c9e596596ea2c8b6fb1244009c405f83b0a9cff..a4e7b03ff836d40a577e349ab0d8c891f465ab38 100644 (file)
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -217,7 +217,7 @@ load_som_binary(struct linux_binprm * bprm, struct pt_regs * regs)
                        (char *) hpuxhdr, size);
        if (retval < 0)
                goto out_free;
-
+#error "Fix security hole before enabling me"
        retval = get_unused_fd();
        if (retval < 0)
                goto out_free;

diff --git a/fs/exec.c b/fs/exec.c
index 84d224e71d3ae9ddb8f58dc36c0fe6b802517b4a..c8527e3d021bfb0785ae4d3d96e0a919b127db5f 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -779,6 +779,7 @@ int flush_old_exec(struct linux_binprm * bprm)
 {
        char * name;
        int i, ch, retval;
+       struct files_struct *files;
 
        /*
         * Make sure we have a private signal table and that
@@ -788,16 +789,26 @@ int flush_old_exec(struct linux_binprm * bprm)
        if (retval)
                goto out;
 
+       /*
+        * Make sure we have private file handles. Ask the
+        * fork helper to do the work for us and the exit
+        * helper to do the cleanup of the old one.
+        */
+       files = current->files;         /* refcounted so safe to hold */
+       retval = unshare_files();
+       if (retval)
+               goto out;
        /*
         * Release all of the old mmap stuff
         */
        retval = exec_mmap(bprm->mm);
        if (retval)
-               goto out;
+               goto mmap_failed;
 
        bprm->mm = NULL;                /* We're using it now */
 
        /* This is the point of no return */
+       put_files_struct(files);
 
        current->sas_ss_sp = current->sas_ss_size = 0;
 
@@ -830,6 +841,9 @@ int flush_old_exec(struct linux_binprm * bprm)
 
        return 0;
 
+mmap_failed:
+       put_files_struct(current->files);
+       current->files = files;
 out:
        return retval;
 }