exec: Two bug fixes in binfmt recursion
svn path=/dists/sid/linux/; revision=19595
This commit is contained in:
parent
4e2693e858
commit
7d16ce52f9
|
@ -97,6 +97,8 @@ linux (3.2.35-1) UNRELEASED; urgency=low
|
|||
* [x86] KVM: x86: invalid opcode oops on SET_SREGS with OSXSAVE bit set
|
||||
(CVE-2012-4461)
|
||||
* kmod: make __request_module() killable (CVE-2012-4398)
|
||||
* exec: do not leave bprm->interp on stack (CVE-2012-4530)
|
||||
* exec: use -ELOOP for max recursion depth
|
||||
|
||||
[ Ian Campbell ]
|
||||
* [xen] add support for microcode updating. (Closes: #693053)
|
||||
|
|
|
@ -0,0 +1,115 @@
|
|||
From: Kees Cook <keescook@chromium.org>
|
||||
Date: Thu, 6 Dec 2012 17:00:21 +1100
|
||||
Subject: [1/2] exec: do not leave bprm->interp on stack
|
||||
|
||||
commit 1e1b8374592f5fb347625e84d8a5f2f40d858a24 upstream.
|
||||
|
||||
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>
|
||||
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
||||
---
|
||||
fs/binfmt_misc.c | 5 ++++-
|
||||
fs/binfmt_script.c | 4 +++-
|
||||
fs/exec.c | 15 +++++++++++++++
|
||||
include/linux/binfmts.h | 1 +
|
||||
4 files changed, 23 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
|
||||
index b0b70fb..b0c1755 100644
|
||||
--- 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 8c95499..4834f2c 100644
|
||||
--- 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 b71b08c..bf50973 100644
|
||||
--- a/fs/exec.c
|
||||
+++ b/fs/exec.c
|
||||
@@ -1175,9 +1175,24 @@ void free_bprm(struct linux_binprm *bprm)
|
||||
mutex_unlock(¤t->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 2630c9b..7f0e297 100644
|
||||
--- 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);
|
|
@ -0,0 +1,133 @@
|
|||
From: Kees Cook <keescook@chromium.org>
|
||||
Date: Thu, 6 Dec 2012 17:00:21 +1100
|
||||
Subject: [2/2] exec: use -ELOOP for max recursion depth
|
||||
|
||||
commit a4706fd3dc5d23f58da814d03f3ef92fd9a4fc16 upstream.
|
||||
|
||||
To avoid an explosion of request_module calls on a chain of abusive
|
||||
scripts, fail maximum recursion with -ELOOP instead of -ENOEXEC. As soon
|
||||
as maximum recursion depth is hit, the error will fail all the way back
|
||||
up the chain, aborting immediately.
|
||||
|
||||
This also has the side-effect of stopping the user's shell from attempting
|
||||
to reexecute the top-level file as a shell script. As seen in the
|
||||
dash source:
|
||||
|
||||
if (cmd != path_bshell && errno == ENOEXEC) {
|
||||
*argv-- = cmd;
|
||||
*argv = cmd = path_bshell;
|
||||
goto repeat;
|
||||
}
|
||||
|
||||
The above logic was designed for running scripts automatically that lacked
|
||||
the "#!" header, not to re-try failed recursion. On a legitimate -ENOEXEC,
|
||||
things continue to behave as the shell expects.
|
||||
|
||||
Additionally, when tracking recursion, the binfmt handlers should not be
|
||||
involved. The recursion being tracked is the depth of calls through
|
||||
search_binary_handler(), so that function should be exclusively responsible
|
||||
for tracking the depth.
|
||||
|
||||
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>
|
||||
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
||||
[bwh: Backported to 3.2: adjust context]
|
||||
---
|
||||
fs/binfmt_em86.c | 1 -
|
||||
fs/binfmt_misc.c | 6 ------
|
||||
fs/binfmt_script.c | 4 +---
|
||||
fs/exec.c | 10 +++++-----
|
||||
include/linux/binfmts.h | 2 --
|
||||
5 files changed, 6 insertions(+), 17 deletions(-)
|
||||
|
||||
--- a/fs/binfmt_em86.c
|
||||
+++ b/fs/binfmt_em86.c
|
||||
@@ -42,7 +42,6 @@ static int load_em86(struct linux_binprm
|
||||
return -ENOEXEC;
|
||||
}
|
||||
|
||||
- bprm->recursion_depth++; /* Well, the bang-shell is implicit... */
|
||||
allow_write_access(bprm->file);
|
||||
fput(bprm->file);
|
||||
bprm->file = NULL;
|
||||
--- a/fs/binfmt_misc.c
|
||||
+++ b/fs/binfmt_misc.c
|
||||
@@ -116,10 +116,6 @@ static int load_misc_binary(struct linux
|
||||
if (!enabled)
|
||||
goto _ret;
|
||||
|
||||
- retval = -ENOEXEC;
|
||||
- if (bprm->recursion_depth > BINPRM_MAX_RECURSION)
|
||||
- goto _ret;
|
||||
-
|
||||
/* to keep locking time low, we copy the interpreter string */
|
||||
read_lock(&entries_lock);
|
||||
fmt = check_file(bprm);
|
||||
@@ -199,8 +195,6 @@ static int load_misc_binary(struct linux
|
||||
if (retval < 0)
|
||||
goto _error;
|
||||
|
||||
- bprm->recursion_depth++;
|
||||
-
|
||||
retval = search_binary_handler (bprm, regs);
|
||||
if (retval < 0)
|
||||
goto _error;
|
||||
--- a/fs/binfmt_script.c
|
||||
+++ b/fs/binfmt_script.c
|
||||
@@ -22,15 +22,13 @@ static int load_script(struct linux_binp
|
||||
char interp[BINPRM_BUF_SIZE];
|
||||
int retval;
|
||||
|
||||
- if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
|
||||
- (bprm->recursion_depth > BINPRM_MAX_RECURSION))
|
||||
+ if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!'))
|
||||
return -ENOEXEC;
|
||||
/*
|
||||
* This section does the #! interpretation.
|
||||
* Sorta complicated, but hopefully it will work. -TYT
|
||||
*/
|
||||
|
||||
- bprm->recursion_depth++;
|
||||
allow_write_access(bprm->file);
|
||||
fput(bprm->file);
|
||||
bprm->file = NULL;
|
||||
--- a/fs/exec.c
|
||||
+++ b/fs/exec.c
|
||||
@@ -1384,6 +1384,10 @@ int search_binary_handler(struct linux_b
|
||||
struct linux_binfmt *fmt;
|
||||
pid_t old_pid;
|
||||
|
||||
+ /* This allows 4 levels of binfmt rewrites before failing hard. */
|
||||
+ if (depth > 5)
|
||||
+ return -ELOOP;
|
||||
+
|
||||
retval = security_bprm_check(bprm);
|
||||
if (retval)
|
||||
return retval;
|
||||
@@ -1407,12 +1411,8 @@ int search_binary_handler(struct linux_b
|
||||
if (!try_module_get(fmt->module))
|
||||
continue;
|
||||
read_unlock(&binfmt_lock);
|
||||
+ bprm->recursion_depth = depth + 1;
|
||||
retval = fn(bprm, regs);
|
||||
- /*
|
||||
- * Restore the depth counter to its starting value
|
||||
- * in this call, so we don't have to rely on every
|
||||
- * load_binary function to restore it on return.
|
||||
- */
|
||||
bprm->recursion_depth = depth;
|
||||
if (retval >= 0) {
|
||||
if (depth == 0)
|
||||
--- a/include/linux/binfmts.h
|
||||
+++ b/include/linux/binfmts.h
|
||||
@@ -67,8 +67,6 @@ struct linux_binprm {
|
||||
#define BINPRM_FLAGS_EXECFD_BIT 1
|
||||
#define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
|
||||
|
||||
-#define BINPRM_MAX_RECURSION 4
|
||||
-
|
||||
/* Function parameter for binfmt->coredump */
|
||||
struct coredump_params {
|
||||
long signr;
|
|
@ -428,3 +428,5 @@ bugfix/all/usermodehelper-implement-UMH_KILLABLE.patch
|
|||
bugfix/all/usermodehelper-____call_usermodehelper-doesnt-need-do_exit.patch
|
||||
bugfix/all/kmod-introduce-call_modprobe-helper.patch
|
||||
bugfix/all/kmod-make-__request_module-killable.patch
|
||||
bugfix/all/exec-do-not-leave-bprm-interp-on-stack.patch
|
||||
bugfix/all/exec-use-ELOOP-for-max-recursion-depth.patch
|
||||
|
|
Loading…
Reference in New Issue