aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKees Cook <keescook@chromium.org>2012-12-20 18:05:16 -0500
committerLinus Torvalds <torvalds@linux-foundation.org>2012-12-20 20:40:19 -0500
commitb66c5984017533316fd1951770302649baf1aa33 (patch)
tree78d1e5fc82a057c62699734602c8e5f7ca86b7a2
parent9f9c9cbb60576a1518d0bf93fb8e499cffccf377 (diff)
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> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--fs/binfmt_misc.c5
-rw-r--r--fs/binfmt_script.c4
-rw-r--r--fs/exec.c15
-rw-r--r--include/linux/binfmts.h1
4 files changed, 23 insertions, 2 deletions
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 9be335fb8a7c..0c8869fdd14e 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -172,7 +172,10 @@ static int load_misc_binary(struct linux_binprm *bprm)
172 goto _error; 172 goto _error;
173 bprm->argc ++; 173 bprm->argc ++;
174 174
175 bprm->interp = iname; /* for binfmt_script */ 175 /* Update interp in case binfmt_script needs it. */
176 retval = bprm_change_interp(iname, bprm);
177 if (retval < 0)
178 goto _error;
176 179
177 interp_file = open_exec (iname); 180 interp_file = open_exec (iname);
178 retval = PTR_ERR (interp_file); 181 retval = PTR_ERR (interp_file);
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 1610a91637e5..5027a3e14922 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -80,7 +80,9 @@ static int load_script(struct linux_binprm *bprm)
80 retval = copy_strings_kernel(1, &i_name, bprm); 80 retval = copy_strings_kernel(1, &i_name, bprm);
81 if (retval) return retval; 81 if (retval) return retval;
82 bprm->argc++; 82 bprm->argc++;
83 bprm->interp = interp; 83 retval = bprm_change_interp(interp, bprm);
84 if (retval < 0)
85 return retval;
84 86
85 /* 87 /*
86 * OK, now restart the process with the interpreter's dentry. 88 * OK, now restart the process with the interpreter's dentry.
diff --git a/fs/exec.c b/fs/exec.c
index d8e1191cb112..237d5342786c 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1175,9 +1175,24 @@ void free_bprm(struct linux_binprm *bprm)
1175 mutex_unlock(&current->signal->cred_guard_mutex); 1175 mutex_unlock(&current->signal->cred_guard_mutex);
1176 abort_creds(bprm->cred); 1176 abort_creds(bprm->cred);
1177 } 1177 }
1178 /* If a binfmt changed the interp, free it. */
1179 if (bprm->interp != bprm->filename)
1180 kfree(bprm->interp);
1178 kfree(bprm); 1181 kfree(bprm);
1179} 1182}
1180 1183
1184int bprm_change_interp(char *interp, struct linux_binprm *bprm)
1185{
1186 /* If a binfmt changed the interp, free it first. */
1187 if (bprm->interp != bprm->filename)
1188 kfree(bprm->interp);
1189 bprm->interp = kstrdup(interp, GFP_KERNEL);
1190 if (!bprm->interp)
1191 return -ENOMEM;
1192 return 0;
1193}
1194EXPORT_SYMBOL(bprm_change_interp);
1195
1181/* 1196/*
1182 * install the new credentials for this executable 1197 * install the new credentials for this executable
1183 */ 1198 */
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index a4c2b565c835..bdf3965f0a29 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -112,6 +112,7 @@ extern int setup_arg_pages(struct linux_binprm * bprm,
112 unsigned long stack_top, 112 unsigned long stack_top,
113 int executable_stack); 113 int executable_stack);
114extern int bprm_mm_init(struct linux_binprm *bprm); 114extern int bprm_mm_init(struct linux_binprm *bprm);
115extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
115extern int copy_strings_kernel(int argc, const char *const *argv, 116extern int copy_strings_kernel(int argc, const char *const *argv,
116 struct linux_binprm *bprm); 117 struct linux_binprm *bprm);
117extern int prepare_bprm_creds(struct linux_binprm *bprm); 118extern int prepare_bprm_creds(struct linux_binprm *bprm);