From 3c456bfc4ba66e9cda210da7bc4fb0ba9fcc6972 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 20 Oct 2012 21:53:31 -0400 Subject: get rid of pt_regs argument of search_binary_handler() Signed-off-by: Al Viro --- fs/binfmt_misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/binfmt_misc.c') diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 790b3cddca6..226aeac22ac 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -199,7 +199,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs) bprm->recursion_depth++; - retval = search_binary_handler (bprm, regs); + retval = search_binary_handler(bprm); if (retval < 0) goto _error; -- cgit v1.2.3 From 71613c3b871c5a9f27cc48f124251bcd3aa23be1 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sat, 20 Oct 2012 22:00:48 -0400 Subject: get rid of pt_regs argument of ->load_binary() Signed-off-by: Al Viro --- fs/binfmt_misc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/binfmt_misc.c') diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 226aeac22ac..b0b70fbea06 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -104,7 +104,7 @@ static Node *check_file(struct linux_binprm *bprm) /* * the loader itself */ -static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs) +static int load_misc_binary(struct linux_binprm *bprm) { Node *fmt; struct file * interp_file = NULL; -- cgit v1.2.3 From d740269867021faf4ce38a449353d2b986c34a67 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 17 Dec 2012 16:03:20 -0800 Subject: exec: use -ELOOP for max recursion depth 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 Cc: halfdog Cc: P J P Cc: Alexander Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/binfmt_misc.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'fs/binfmt_misc.c') diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index b0b70fbea06..9be335fb8a7 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -117,10 +117,6 @@ static int load_misc_binary(struct linux_binprm *bprm) 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); @@ -197,8 +193,6 @@ static int load_misc_binary(struct linux_binprm *bprm) if (retval < 0) goto _error; - bprm->recursion_depth++; - retval = search_binary_handler(bprm); if (retval < 0) goto _error; -- cgit v1.2.3 From b66c5984017533316fd1951770302649baf1aa33 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 20 Dec 2012 15:05:16 -0800 Subject: 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 Cc: halfdog Cc: P J P Cc: Alexander Viro Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/binfmt_misc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'fs/binfmt_misc.c') diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 9be335fb8a7..0c8869fdd14 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -172,7 +172,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); -- cgit v1.2.3