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_script.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/binfmt_script.c') diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index d3b8c1f6315..798b729f01d 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -95,7 +95,7 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs) retval = prepare_binprm(bprm); if (retval < 0) return retval; - return search_binary_handler(bprm,regs); + return search_binary_handler(bprm); } static struct linux_binfmt script_format = { -- cgit v1.2.3-70-g09d2 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 --- arch/alpha/kernel/binfmt_loader.c | 2 +- arch/x86/ia32/ia32_aout.c | 5 +++-- fs/binfmt_aout.c | 5 +++-- fs/binfmt_elf.c | 5 +++-- fs/binfmt_elf_fdpic.c | 6 +++--- fs/binfmt_em86.c | 2 +- fs/binfmt_flat.c | 5 +++-- fs/binfmt_misc.c | 2 +- fs/binfmt_script.c | 2 +- fs/binfmt_som.c | 5 +++-- fs/exec.c | 4 ++-- include/linux/binfmts.h | 2 +- 12 files changed, 25 insertions(+), 20 deletions(-) (limited to 'fs/binfmt_script.c') diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c index 54abbef5d07..9525660c93c 100644 --- a/arch/alpha/kernel/binfmt_loader.c +++ b/arch/alpha/kernel/binfmt_loader.c @@ -5,7 +5,7 @@ #include #include -static int load_binary(struct linux_binprm *bprm, struct pt_regs *regs) +static int load_binary(struct linux_binprm *bprm) { struct exec *eh = (struct exec *)bprm->buf; unsigned long loader; diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c index 07b3a68d2d2..a703af19c28 100644 --- a/arch/x86/ia32/ia32_aout.c +++ b/arch/x86/ia32/ia32_aout.c @@ -35,7 +35,7 @@ #undef WARN_OLD #undef CORE_DUMP /* definitely broken */ -static int load_aout_binary(struct linux_binprm *, struct pt_regs *regs); +static int load_aout_binary(struct linux_binprm *); static int load_aout_library(struct file *); #ifdef CORE_DUMP @@ -260,9 +260,10 @@ static u32 __user *create_aout_tables(char __user *p, struct linux_binprm *bprm) * These are the functions used to load a.out style executables and shared * libraries. There is no binary dependent code anywhere else. */ -static int load_aout_binary(struct linux_binprm *bprm, struct pt_regs *regs) +static int load_aout_binary(struct linux_binprm *bprm) { unsigned long error, fd_offset, rlim; + struct pt_regs *regs = current_pt_regs(); struct exec ex; int retval; diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c index 0e7a6f81ae3..6043567b95c 100644 --- a/fs/binfmt_aout.c +++ b/fs/binfmt_aout.c @@ -30,7 +30,7 @@ #include #include -static int load_aout_binary(struct linux_binprm *, struct pt_regs * regs); +static int load_aout_binary(struct linux_binprm *); static int load_aout_library(struct file*); #ifdef CONFIG_COREDUMP @@ -201,8 +201,9 @@ static unsigned long __user *create_aout_tables(char __user *p, struct linux_bin * libraries. There is no binary dependent code anywhere else. */ -static int load_aout_binary(struct linux_binprm * bprm, struct pt_regs * regs) +static int load_aout_binary(struct linux_binprm * bprm) { + struct pt_regs *regs = current_pt_regs(); struct exec ex; unsigned long error; unsigned long fd_offset; diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index fbd9f60bd76..6d7d1647a68 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -44,7 +44,7 @@ #define user_siginfo_t siginfo_t #endif -static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs); +static int load_elf_binary(struct linux_binprm *bprm); static int load_elf_library(struct file *); static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *, int, int, unsigned long); @@ -558,7 +558,7 @@ static unsigned long randomize_stack_top(unsigned long stack_top) #endif } -static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs) +static int load_elf_binary(struct linux_binprm *bprm) { struct file *interpreter = NULL; /* to shut gcc up */ unsigned long load_addr = 0, load_bias = 0; @@ -575,6 +575,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs) unsigned long reloc_func_desc __maybe_unused = 0; int executable_stack = EXSTACK_DEFAULT; unsigned long def_flags = 0; + struct pt_regs *regs = current_pt_regs(); struct { struct elfhdr elf_ex; struct elfhdr interp_elf_ex; diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index a4604915410..dc84732e554 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -56,7 +56,7 @@ typedef char *elf_caddr_t; MODULE_LICENSE("GPL"); -static int load_elf_fdpic_binary(struct linux_binprm *, struct pt_regs *); +static int load_elf_fdpic_binary(struct linux_binprm *); static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *, struct file *); static int elf_fdpic_map_file(struct elf_fdpic_params *, struct file *, struct mm_struct *, const char *); @@ -164,10 +164,10 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params, /* * load an fdpic binary into various bits of memory */ -static int load_elf_fdpic_binary(struct linux_binprm *bprm, - struct pt_regs *regs) +static int load_elf_fdpic_binary(struct linux_binprm *bprm) { struct elf_fdpic_params exec_params, interp_params; + struct pt_regs *regs = current_pt_regs(); struct elf_phdr *phdr; unsigned long stack_size, entryaddr; #ifdef ELF_FDPIC_PLAT_INIT diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index 7e125718a75..4e6cce57d11 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -22,7 +22,7 @@ #define EM86_INTERP "/usr/bin/em86" #define EM86_I_NAME "em86" -static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs) +static int load_em86(struct linux_binprm *bprm) { char *interp, *i_name, *i_arg; struct file * file; diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index e280352b28f..b56371981d1 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -88,7 +88,7 @@ struct lib_info { static int load_flat_shared_library(int id, struct lib_info *p); #endif -static int load_flat_binary(struct linux_binprm *, struct pt_regs * regs); +static int load_flat_binary(struct linux_binprm *); static int flat_core_dump(struct coredump_params *cprm); static struct linux_binfmt flat_format = { @@ -858,9 +858,10 @@ out: * libraries. There is no binary dependent code anywhere else. */ -static int load_flat_binary(struct linux_binprm * bprm, struct pt_regs * regs) +static int load_flat_binary(struct linux_binprm * bprm) { struct lib_info libinfo; + struct pt_regs *regs = current_pt_regs(); unsigned long p = bprm->p; unsigned long stack_len; unsigned long start_addr; 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; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 798b729f01d..8c954997e7f 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -14,7 +14,7 @@ #include #include -static int load_script(struct linux_binprm *bprm,struct pt_regs *regs) +static int load_script(struct linux_binprm *bprm) { const char *i_arg, *i_name; char *cp; diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c index 4517aaff61b..4e00ed68d4a 100644 --- a/fs/binfmt_som.c +++ b/fs/binfmt_som.c @@ -35,7 +35,7 @@ #include -static int load_som_binary(struct linux_binprm * bprm, struct pt_regs * regs); +static int load_som_binary(struct linux_binprm * bprm); static int load_som_library(struct file *); /* @@ -180,13 +180,14 @@ out: */ static int -load_som_binary(struct linux_binprm * bprm, struct pt_regs * regs) +load_som_binary(struct linux_binprm * bprm) { int retval; unsigned int size; unsigned long som_entry; struct som_hdr *som_ex; struct som_exec_auxhdr *hpuxhdr; + struct pt_regs *regs = current_pt_regs(); /* Get the exec-header */ som_ex = (struct som_hdr *) bprm->buf; diff --git a/fs/exec.c b/fs/exec.c index 2aee7ef1066..721a2992951 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1374,13 +1374,13 @@ int search_binary_handler(struct linux_binprm *bprm) for (try=0; try<2; try++) { read_lock(&binfmt_lock); list_for_each_entry(fmt, &formats, lh) { - int (*fn)(struct linux_binprm *, struct pt_regs *) = fmt->load_binary; + int (*fn)(struct linux_binprm *) = fmt->load_binary; if (!fn) continue; if (!try_module_get(fmt->module)) continue; read_unlock(&binfmt_lock); - retval = fn(bprm, current_pt_regs()); + retval = fn(bprm); /* * Restore the depth counter to its starting value * in this call, so we don't have to rely on every diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 1f6ce133b4c..2630c9b41a8 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -72,7 +72,7 @@ struct coredump_params { struct linux_binfmt { struct list_head lh; struct module *module; - int (*load_binary)(struct linux_binprm *, struct pt_regs * regs); + int (*load_binary)(struct linux_binprm *); int (*load_shlib)(struct file *); int (*core_dump)(struct coredump_params *cprm); unsigned long min_coredump; /* minimal dump size */ -- cgit v1.2.3-70-g09d2 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_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(-) (limited to 'fs/binfmt_script.c') diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index 4e6cce57d11..037a3e2b045 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -42,7 +42,6 @@ static int load_em86(struct linux_binprm *bprm) return -ENOEXEC; } - bprm->recursion_depth++; /* Well, the bang-shell is implicit... */ allow_write_access(bprm->file); fput(bprm->file); bprm->file = NULL; 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; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 8c954997e7f..1610a91637e 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -22,15 +22,13 @@ static int load_script(struct linux_binprm *bprm) 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; diff --git a/fs/exec.c b/fs/exec.c index 721a2992951..d5eb9e605ff 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1356,6 +1356,10 @@ int search_binary_handler(struct linux_binprm *bprm) struct linux_binfmt *fmt; pid_t old_pid, old_vpid; + /* This allows 4 levels of binfmt rewrites before failing hard. */ + if (depth > 5) + return -ELOOP; + retval = security_bprm_check(bprm); if (retval) return retval; @@ -1380,12 +1384,8 @@ int search_binary_handler(struct linux_binprm *bprm) if (!try_module_get(fmt->module)) continue; read_unlock(&binfmt_lock); + bprm->recursion_depth = depth + 1; retval = fn(bprm); - /* - * 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) { diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 2630c9b41a8..a4c2b565c83 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -54,8 +54,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 { siginfo_t *siginfo; -- cgit v1.2.3-70-g09d2 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 ++++- fs/binfmt_script.c | 4 +++- fs/exec.c | 15 +++++++++++++++ include/linux/binfmts.h | 1 + 4 files changed, 23 insertions(+), 2 deletions(-) (limited to 'fs/binfmt_script.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); diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 1610a91637e..5027a3e1492 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -80,7 +80,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 d8e1191cb11..237d5342786 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 a4c2b565c83..bdf3965f0a2 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -112,6 +112,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); -- cgit v1.2.3-70-g09d2