exec: do not leave bprm->interp on stack

If a series of scripts are executed, each triggering module loading via
unprintable bytes in the script header, kernel stack contents can leak
into the command line.

Normally execution of binfmt_script and binfmt_misc happens recursively.
However, when modules are enabled, and unprintable bytes exist in the
bprm->buf, execution will restart after attempting to load matching binfmt
modules.  Unfortunately, the logic in binfmt_script and binfmt_misc does
not expect to get restarted.  They leave bprm->interp pointing to their
local stack.  This means on restart bprm->interp is left pointing into
unused stack memory which can then be copied into the userspace argv
areas.

After additional study, it seems that both recursion and restart remains
the desirable way to handle exec with scripts, misc, and modules.  As
such, we need to protect the changes to interp.

This changes the logic to require allocation for any changes to the
bprm->interp.  To avoid adding a new kmalloc to every exec, the default
value is left as-is.  Only when passing through binfmt_script or
binfmt_misc does an allocation take place.

For a proof of concept, see DoTest.sh from:
http://www.halfdog.net/Security/2012/LinuxKernelBinfmtScriptStackDataDisclosure/

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: halfdog <me@halfdog.net>
Cc: P J P <ppandit@redhat.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index b0b70fbea06cb03e1adfe854a987f01bbf9956a9..b0c1755b8baaced05a59931df0f93ffe835e6b05 100644 (file)
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -176,7 +176,10 @@ static int load_misc_binary(struct linux_binprm *bprm)
                goto _error;
        bprm->argc ++;
 
-       bprm->interp = iname;   /* for binfmt_script */
+       /* Update interp in case binfmt_script needs it. */
+       retval = bprm_change_interp(iname, bprm);
+       if (retval < 0)
+               goto _error;
 
        interp_file = open_exec (iname);
        retval = PTR_ERR (interp_file);

diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 8c954997e7f73a22b39718a9564c0c9ed3465a58..4834f2ceeab179db9222dc1ea34b45817b39a141 100644 (file)
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -82,7 +82,9 @@ static int load_script(struct linux_binprm *bprm)
        retval = copy_strings_kernel(1, &i_name, bprm);
        if (retval) return retval; 
        bprm->argc++;
-       bprm->interp = interp;
+       retval = bprm_change_interp(interp, bprm);
+       if (retval < 0)
+               return retval;
 
        /*
         * OK, now restart the process with the interpreter's dentry.

diff --git a/fs/exec.c b/fs/exec.c
index 721a299295117f92d271f17afd224db1787712a1..0595108eefcaa5dcfc7da32fa2be1bcbad8dbc17 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1175,9 +1175,24 @@ void free_bprm(struct linux_binprm *bprm)
                mutex_unlock(&current->signal->cred_guard_mutex);
                abort_creds(bprm->cred);
        }
+       /* If a binfmt changed the interp, free it. */
+       if (bprm->interp != bprm->filename)
+               kfree(bprm->interp);
        kfree(bprm);
 }
 
+int bprm_change_interp(char *interp, struct linux_binprm *bprm)
+{
+       /* If a binfmt changed the interp, free it first. */
+       if (bprm->interp != bprm->filename)
+               kfree(bprm->interp);
+       bprm->interp = kstrdup(interp, GFP_KERNEL);
+       if (!bprm->interp)
+               return -ENOMEM;
+       return 0;
+}
+EXPORT_SYMBOL(bprm_change_interp);
+
 /*
  * install the new credentials for this executable
  */

diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 2630c9b41a8680c9411475f99b6f81f3efdc4f79..7f0e297b13f9cd9658e9a91f91b732181d589214 100644 (file)
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -114,6 +114,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
                           unsigned long stack_top,
                           int executable_stack);
 extern int bprm_mm_init(struct linux_binprm *bprm);
+extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
 extern int copy_strings_kernel(int argc, const char *const *argv,
                               struct linux_binprm *bprm);
 extern int prepare_bprm_creds(struct linux_binprm *bprm);