From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752664Ab3FDWll (ORCPT ); Tue, 4 Jun 2013 18:41:41 -0400 Received: from 1wt.eu ([62.212.114.60]:35273 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752098Ab3FDWlg (ORCPT ); Tue, 4 Jun 2013 18:41:36 -0400 Message-Id: <20130604172130.969405668@1wt.eu> User-Agent: quilt/0.48-1 Date: Tue, 04 Jun 2013 19:21:46 +0200 From: Willy Tarreau To: linux-kernel@vger.kernel.org, stable@vger.kernel.org Cc: Kees Cook , halfdog , P J P , Alexander Viro , Andrew Morton , Linus Torvalds , Willy Tarreau Subject: [ 016/184] exec: use -ELOOP for max recursion depth In-Reply-To: <58df134a4b98edf5b0073e2e1e988fe6@local> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2.6.32-longterm review patch. If anyone has any objections, please let me know. ------------------ From: Kees Cook commit d740269867021faf4ce38a449353d2b986c34a67 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 Cc: halfdog Cc: P J P Cc: Alexander Viro Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds [dannf: backported to Debian's 2.6.32] Signed-off-by: Willy Tarreau --- 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(-) diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index 32fb00b..416dcae 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -43,7 +43,6 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs) 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 fb93997..258c5ca 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -116,10 +116,6 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs) 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); @@ -200,8 +196,6 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs) if (retval < 0) goto _error; - bprm->recursion_depth++; - retval = search_binary_handler (bprm, regs); if (retval < 0) goto _error; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 356568c..4fe6b8a 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -22,15 +22,13 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs) 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 f9f1b11..feb2435 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1285,6 +1285,10 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs) int try,retval; struct linux_binfmt *fmt; + /* This allows 4 levels of binfmt rewrites before failing hard. */ + if (depth > 5) + return -ELOOP; + retval = security_bprm_check(bprm); if (retval) return retval; @@ -1306,12 +1310,8 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs) 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) diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index d06c3a4..9ffffec 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -71,8 +71,6 @@ extern struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos, #define BINPRM_FLAGS_EXECFD_BIT 1 #define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT) -#define BINPRM_MAX_RECURSION 4 - /* * This structure defines the functions that are used to load the binary formats that * linux accepts. -- 1.7.12.2.21.g234cd45.dirty