From patchwork Tue May 19 00:29:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556573 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 77CC6618 for ; Tue, 19 May 2020 00:33:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 691132075F for ; Tue, 19 May 2020 00:33:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727083AbgESAdY (ORCPT ); Mon, 18 May 2020 20:33:24 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:59200 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726953AbgESAdX (ORCPT ); Mon, 18 May 2020 20:33:23 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqCA-0002eO-8w; Mon, 18 May 2020 18:33:22 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqC8-0004Hu-W1; Mon, 18 May 2020 18:33:22 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:29:41 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <871rng22dm.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqC8-0004Hu-W1;;;mid=<871rng22dm.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/5Ib6hqHkieMX7PPoHgPDToeUME0OO9Kw= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa04.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa04 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 867 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 12 (1.4%), b_tie_ro: 10 (1.2%), parse: 1.44 (0.2%), extract_message_metadata: 13 (1.4%), get_uri_detail_list: 1.38 (0.2%), tests_pri_-1000: 14 (1.6%), tests_pri_-950: 1.18 (0.1%), tests_pri_-900: 0.99 (0.1%), tests_pri_-90: 160 (18.4%), check_bayes: 158 (18.2%), b_tokenize: 7 (0.8%), b_tok_get_all: 4.9 (0.6%), b_comp_prob: 2.1 (0.2%), b_tok_touch_all: 140 (16.2%), b_finish: 1.02 (0.1%), tests_pri_0: 653 (75.3%), check_dkim_signature: 0.56 (0.1%), check_dkim_adsp: 2.4 (0.3%), poll_dns_idle: 0.61 (0.1%), tests_pri_10: 2.0 (0.2%), tests_pri_500: 7 (0.8%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 1/8] exec: Teach prepare_exec_creds how exec treats uids & gids X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: It is almost possible to use the result of prepare_exec_creds with no modifications during exec. Update prepare_exec_creds to initialize the suid and the fsuid to the euid, and the sgid and the fsgid to the egid. This is all that is needed to handle the common case of exec when nothing special like a setuid exec is happening. That this preserves the existing behavior of exec can be verified by examing bprm_fill_uid and cap_bprm_set_creds. This change makes it clear that the later parts of exec that update bprm->cred are just need to handle special cases such as setuid exec and change of domains. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- kernel/cred.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/kernel/cred.c b/kernel/cred.c index 71a792616917..421b1149c651 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -315,6 +315,9 @@ struct cred *prepare_exec_creds(void) new->process_keyring = NULL; #endif + new->suid = new->fsuid = new->euid; + new->sgid = new->fsgid = new->egid; + return new; } From patchwork Tue May 19 00:30:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556579 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 255C290 for ; Tue, 19 May 2020 00:33:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0CF8A2075F for ; Tue, 19 May 2020 00:33:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727882AbgESAdy (ORCPT ); Mon, 18 May 2020 20:33:54 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:37174 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726953AbgESAdx (ORCPT ); Mon, 18 May 2020 20:33:53 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqCe-0000Pe-A9; Mon, 18 May 2020 18:33:52 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqCc-0004K8-Ny; Mon, 18 May 2020 18:33:52 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:30:10 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87v9kszrzh.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqCc-0004K8-Ny;;;mid=<87v9kszrzh.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+ecPFCymisO0mdQ0r9+TKZOTSbzdPHvMY= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: *** X-Spam-Status: No, score=3.2 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,LotsOfNums_01,T_TooManySym_01,XMGappySubj_01, XMGappySubj_02,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 1.0 XMGappySubj_02 Gappier still * 0.5 XMGappySubj_01 Very gappy subject * 1.2 LotsOfNums_01 BODY: Lots of long strings of numbers * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ***; X-Spam-Relay-Country: X-Spam-Timing: total 1152 ms - load_scoreonly_sql: 0.09 (0.0%), signal_user_changed: 12 (1.0%), b_tie_ro: 10 (0.9%), parse: 2.1 (0.2%), extract_message_metadata: 22 (1.9%), get_uri_detail_list: 9 (0.8%), tests_pri_-1000: 14 (1.2%), tests_pri_-950: 1.41 (0.1%), tests_pri_-900: 1.14 (0.1%), tests_pri_-90: 141 (12.3%), check_bayes: 139 (12.1%), b_tokenize: 29 (2.5%), b_tok_get_all: 17 (1.4%), b_comp_prob: 6 (0.5%), b_tok_touch_all: 83 (7.2%), b_finish: 0.95 (0.1%), tests_pri_0: 941 (81.7%), check_dkim_signature: 0.88 (0.1%), check_dkim_adsp: 2.4 (0.2%), poll_dns_idle: 0.59 (0.1%), tests_pri_10: 3.2 (0.3%), tests_pri_500: 9 (0.8%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 2/8] exec: Factor security_bprm_creds_for_exec out of security_bprm_set_creds X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Today security_bprm_set_creds has several implementations: apparmor_bprm_set_creds, cap_bprm_set_creds, selinux_bprm_set_creds, smack_bprm_set_creds, and tomoyo_bprm_set_creds. Except for cap_bprm_set_creds they all test bprm->called_set_creds and return immediately if it is true. The function cap_bprm_set_creds ignores bprm->calld_sed_creds entirely. Create a new LSM hook security_bprm_creds_for_exec that is called just before prepare_binprm in __do_execve_file, resulting in a LSM hook that is called exactly once for the entire of exec. Modify the bits of security_bprm_set_creds that only want to be called once per exec into security_bprm_creds_for_exec, leaving only cap_bprm_set_creds behind. Remove bprm->called_set_creds all of it's former users have been moved to security_bprm_creds_for_exec. Add or upate comments a appropriate to bring them up to date and to reflect this change. Signed-off-by: "Eric W. Biederman" Acked-by: Casey Schaufler Reviewed-by: Kees Cook --- fs/exec.c | 6 +++- include/linux/binfmts.h | 18 +++-------- include/linux/lsm_hook_defs.h | 1 + include/linux/lsm_hooks.h | 50 +++++++++++++++++------------- include/linux/security.h | 6 ++++ security/apparmor/domain.c | 7 ++--- security/apparmor/include/domain.h | 2 +- security/apparmor/lsm.c | 2 +- security/security.c | 5 +++ security/selinux/hooks.c | 8 ++--- security/smack/smack_lsm.c | 9 ++---- security/tomoyo/tomoyo.c | 12 ++----- 12 files changed, 63 insertions(+), 63 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 14b786158aa9..9e70da47f8d9 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1640,7 +1640,6 @@ int prepare_binprm(struct linux_binprm *bprm) retval = security_bprm_set_creds(bprm); if (retval) return retval; - bprm->called_set_creds = 1; memset(bprm->buf, 0, BINPRM_BUF_SIZE); return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos); @@ -1855,6 +1854,11 @@ static int __do_execve_file(int fd, struct filename *filename, if (retval < 0) goto out; + /* Set the unchanging part of bprm->cred */ + retval = security_bprm_creds_for_exec(bprm); + if (retval) + goto out; + retval = prepare_binprm(bprm); if (retval < 0) goto out; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 1b48e2154766..d1217fcdedea 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -27,22 +27,14 @@ struct linux_binprm { unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int /* - * True after the bprm_set_creds hook has been called once - * (multiple calls can be made via prepare_binprm() for - * binfmt_script/misc). - */ - called_set_creds:1, - /* - * True if most recent call to the commoncaps bprm_set_creds - * hook (due to multiple prepare_binprm() calls from the - * binfmt_script/misc handlers) resulted in elevated - * privileges. + * True if most recent call to cap_bprm_set_creds + * resulted in elevated privileges. */ cap_elevated:1, /* - * Set by bprm_set_creds hook to indicate a privilege-gaining - * exec has happened. Used to sanitize execution environment - * and to set AT_SECURE auxv for glibc. + * Set by bprm_creds_for_exec hook to indicate a + * privilege-gaining exec has happened. Used to set + * AT_SECURE auxv for glibc. */ secureexec:1, /* diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 9cd4455528e5..aab0695f41df 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -49,6 +49,7 @@ LSM_HOOK(int, 0, syslog, int type) LSM_HOOK(int, 0, settime, const struct timespec64 *ts, const struct timezone *tz) LSM_HOOK(int, 0, vm_enough_memory, struct mm_struct *mm, long pages) +LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_set_creds, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, struct linux_binprm *bprm) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 988ca0df7824..c719af37df20 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -34,40 +34,46 @@ * * Security hooks for program execution operations. * + * @bprm_creds_for_exec: + * If the setup in prepare_exec_creds did not setup @bprm->cred->security + * properly for executing @bprm->file, update the LSM's portion of + * @bprm->cred->security to be what commit_creds needs to install for the + * new program. This hook may also optionally check permissions + * (e.g. for transitions between security domains). + * The hook must set @bprm->secureexec to 1 if AT_SECURE should be set to + * request libc enable secure mode. + * @bprm contains the linux_binprm structure. + * Return 0 if the hook is successful and permission is granted. * @bprm_set_creds: - * Save security information in the bprm->security field, typically based - * on information about the bprm->file, for later use by the apply_creds - * hook. This hook may also optionally check permissions (e.g. for + * Assuming that the relevant bits of @bprm->cred->security have been + * previously set, examine @bprm->file and regenerate them. This is + * so that the credentials derived from the interpreter the code is + * actually going to run are used rather than credentials derived + * from a script. This done because the interpreter binary needs to + * reopen script, and may end up opening something completely different. + * This hook may also optionally check permissions (e.g. for * transitions between security domains). - * This hook may be called multiple times during a single execve, e.g. for - * interpreters. The hook can tell whether it has already been called by - * checking to see if @bprm->security is non-NULL. If so, then the hook - * may decide either to retain the security information saved earlier or - * to replace it. The hook must set @bprm->secureexec to 1 if a "secure - * exec" has happened as a result of this hook call. The flag is used to - * indicate the need for a sanitized execution environment, and is also - * passed in the ELF auxiliary table on the initial stack to indicate - * whether libc should enable secure mode. + * The hook must set @bprm->cap_elevated to 1 if AT_SECURE should be set to + * request libc enable secure mode. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. * @bprm_check_security: * This hook mediates the point when a search for a binary handler will - * begin. It allows a check the @bprm->security value which is set in the - * preceding set_creds call. The primary difference from set_creds is - * that the argv list and envp list are reliably available in @bprm. This - * hook may be called multiple times during a single execve; and in each - * pass set_creds is called first. + * begin. It allows a check against the @bprm->cred->security value + * which was set in the preceding creds_for_exec call. The argv list and + * envp list are reliably available in @bprm. This hook may be called + * multiple times during a single execve. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. * @bprm_committing_creds: * Prepare to install the new security attributes of a process being * transformed by an execve operation, based on the old credentials * pointed to by @current->cred and the information set in @bprm->cred by - * the bprm_set_creds hook. @bprm points to the linux_binprm structure. - * This hook is a good place to perform state changes on the process such - * as closing open file descriptors to which access will no longer be - * granted when the attributes are changed. This is called immediately - * before commit_creds(). + * the bprm_creds_for_exec hook. @bprm points to the linux_binprm + * structure. This hook is a good place to perform state changes on the + * process such as closing open file descriptors to which access will no + * longer be granted when the attributes are changed. This is called + * immediately before commit_creds(). * @bprm_committed_creds: * Tidy up after the installation of the new security attributes of a * process being transformed by an execve operation. The new credentials diff --git a/include/linux/security.h b/include/linux/security.h index a8d9310472df..1bd7a6582775 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -276,6 +276,7 @@ int security_quota_on(struct dentry *dentry); int security_syslog(int type); int security_settime64(const struct timespec64 *ts, const struct timezone *tz); int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); +int security_bprm_creds_for_exec(struct linux_binprm *bprm); int security_bprm_set_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); void security_bprm_committing_creds(struct linux_binprm *bprm); @@ -569,6 +570,11 @@ static inline int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) return __vm_enough_memory(mm, pages, cap_vm_enough_memory(mm, pages)); } +static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + return 0; +} + static inline int security_bprm_set_creds(struct linux_binprm *bprm) { return cap_bprm_set_creds(bprm); diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 6ceb74e0f789..0b870a647488 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -854,14 +854,14 @@ static struct aa_label *handle_onexec(struct aa_label *label, } /** - * apparmor_bprm_set_creds - set the new creds on the bprm struct + * apparmor_bprm_creds_for_exec - Update the new creds on the bprm struct * @bprm: binprm for the exec (NOT NULL) * * Returns: %0 or error on failure * * TODO: once the other paths are done see if we can't refactor into a fn */ -int apparmor_bprm_set_creds(struct linux_binprm *bprm) +int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm) { struct aa_task_ctx *ctx; struct aa_label *label, *new = NULL; @@ -875,9 +875,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) file_inode(bprm->file)->i_mode }; - if (bprm->called_set_creds) - return 0; - ctx = task_ctx(current); AA_BUG(!cred_label(bprm->cred)); AA_BUG(!ctx); diff --git a/security/apparmor/include/domain.h b/security/apparmor/include/domain.h index 21b875fe2d37..d14928fe1c6f 100644 --- a/security/apparmor/include/domain.h +++ b/security/apparmor/include/domain.h @@ -30,7 +30,7 @@ struct aa_domain { struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex, const char **name); -int apparmor_bprm_set_creds(struct linux_binprm *bprm); +int apparmor_bprm_creds_for_exec(struct linux_binprm *bprm); void aa_free_domain_entries(struct aa_domain *domain); int aa_change_hat(const char *hats[], int count, u64 token, int flags); diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index b621ad74f54a..3623ab08279d 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -1232,7 +1232,7 @@ static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(cred_prepare, apparmor_cred_prepare), LSM_HOOK_INIT(cred_transfer, apparmor_cred_transfer), - LSM_HOOK_INIT(bprm_set_creds, apparmor_bprm_set_creds), + LSM_HOOK_INIT(bprm_creds_for_exec, apparmor_bprm_creds_for_exec), LSM_HOOK_INIT(bprm_committing_creds, apparmor_bprm_committing_creds), LSM_HOOK_INIT(bprm_committed_creds, apparmor_bprm_committed_creds), diff --git a/security/security.c b/security/security.c index 7fed24b9d57e..4ee76a729f73 100644 --- a/security/security.c +++ b/security/security.c @@ -823,6 +823,11 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages) return __vm_enough_memory(mm, pages, cap_sys_admin); } +int security_bprm_creds_for_exec(struct linux_binprm *bprm) +{ + return call_int_hook(bprm_creds_for_exec, 0, bprm); +} + int security_bprm_set_creds(struct linux_binprm *bprm) { return call_int_hook(bprm_set_creds, 0, bprm); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 0b4e32161b77..718345dd76bb 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -2286,7 +2286,7 @@ static int check_nnp_nosuid(const struct linux_binprm *bprm, return -EACCES; } -static int selinux_bprm_set_creds(struct linux_binprm *bprm) +static int selinux_bprm_creds_for_exec(struct linux_binprm *bprm) { const struct task_security_struct *old_tsec; struct task_security_struct *new_tsec; @@ -2297,8 +2297,6 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm) /* SELinux context only depends on initial program or script and not * the script interpreter */ - if (bprm->called_set_creds) - return 0; old_tsec = selinux_cred(current_cred()); new_tsec = selinux_cred(bprm->cred); @@ -6385,7 +6383,7 @@ static int selinux_setprocattr(const char *name, void *value, size_t size) /* Permission checking based on the specified context is performed during the actual operation (execve, open/mkdir/...), when we know the full context of the - operation. See selinux_bprm_set_creds for the execve + operation. See selinux_bprm_creds_for_exec for the execve checks and may_create for the file creation checks. The operation will then fail if the context is not permitted. */ tsec = selinux_cred(new); @@ -6914,7 +6912,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(netlink_send, selinux_netlink_send), - LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds), + LSM_HOOK_INIT(bprm_creds_for_exec, selinux_bprm_creds_for_exec), LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds), LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 8c61d175e195..0ac8f4518d07 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -891,12 +891,12 @@ static int smack_sb_statfs(struct dentry *dentry) */ /** - * smack_bprm_set_creds - set creds for exec + * smack_bprm_creds_for_exec - Update bprm->cred if needed for exec * @bprm: the exec information * * Returns 0 if it gets a blob, -EPERM if exec forbidden and -ENOMEM otherwise */ -static int smack_bprm_set_creds(struct linux_binprm *bprm) +static int smack_bprm_creds_for_exec(struct linux_binprm *bprm) { struct inode *inode = file_inode(bprm->file); struct task_smack *bsp = smack_cred(bprm->cred); @@ -904,9 +904,6 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm) struct superblock_smack *sbsp; int rc; - if (bprm->called_set_creds) - return 0; - isp = smack_inode(inode); if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task) return 0; @@ -4598,7 +4595,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(sb_statfs, smack_sb_statfs), LSM_HOOK_INIT(sb_set_mnt_opts, smack_set_mnt_opts), - LSM_HOOK_INIT(bprm_set_creds, smack_bprm_set_creds), + LSM_HOOK_INIT(bprm_creds_for_exec, smack_bprm_creds_for_exec), LSM_HOOK_INIT(inode_alloc_security, smack_inode_alloc_security), LSM_HOOK_INIT(inode_init_security, smack_inode_init_security), diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c index 716c92ec941a..f9adddc42ac8 100644 --- a/security/tomoyo/tomoyo.c +++ b/security/tomoyo/tomoyo.c @@ -63,20 +63,14 @@ static void tomoyo_bprm_committed_creds(struct linux_binprm *bprm) #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER /** - * tomoyo_bprm_set_creds - Target for security_bprm_set_creds(). + * tomoyo_bprm_for_exec - Target for security_bprm_creds_for_exec(). * * @bprm: Pointer to "struct linux_binprm". * * Returns 0. */ -static int tomoyo_bprm_set_creds(struct linux_binprm *bprm) +static int tomoyo_bprm_creds_for_exec(struct linux_binprm *bprm) { - /* - * Do only if this function is called for the first time of an execve - * operation. - */ - if (bprm->called_set_creds) - return 0; /* * Load policy if /sbin/tomoyo-init exists and /sbin/init is requested * for the first time. @@ -539,7 +533,7 @@ static struct security_hook_list tomoyo_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(task_alloc, tomoyo_task_alloc), LSM_HOOK_INIT(task_free, tomoyo_task_free), #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER - LSM_HOOK_INIT(bprm_set_creds, tomoyo_bprm_set_creds), + LSM_HOOK_INIT(bprm_creds_for_exec, tomoyo_bprm_creds_for_exec), #endif LSM_HOOK_INIT(bprm_check_security, tomoyo_bprm_check_security), LSM_HOOK_INIT(file_fcntl, tomoyo_file_fcntl), From patchwork Tue May 19 00:31:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556583 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B71E390 for ; Tue, 19 May 2020 00:35:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A3F612075F for ; Tue, 19 May 2020 00:35:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728027AbgESAe6 (ORCPT ); Mon, 18 May 2020 20:34:58 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:37346 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726573AbgESAe5 (ORCPT ); Mon, 18 May 2020 20:34:57 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqDf-0000Sk-R7; Mon, 18 May 2020 18:34:55 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqDe-0003VW-L7; Mon, 18 May 2020 18:34:55 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:31:14 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87o8qkzrxp.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqDe-0003VW-L7;;;mid=<87o8qkzrxp.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX18gTzpc5ZKgxRTyLbmfRdW+xc8Sy4jTNSc= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: ** X-Spam-Status: No, score=2.2 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,LotsOfNums_01,T_TooManySym_01,XMGappySubj_01, XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 0.5 XMGappySubj_01 Very gappy subject * 1.2 LotsOfNums_01 BODY: Lots of long strings of numbers * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **; X-Spam-Relay-Country: X-Spam-Timing: total 723 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 11 (1.6%), b_tie_ro: 10 (1.4%), parse: 1.17 (0.2%), extract_message_metadata: 14 (2.0%), get_uri_detail_list: 4.2 (0.6%), tests_pri_-1000: 13 (1.9%), tests_pri_-950: 1.34 (0.2%), tests_pri_-900: 1.06 (0.1%), tests_pri_-90: 88 (12.2%), check_bayes: 87 (12.0%), b_tokenize: 17 (2.4%), b_tok_get_all: 14 (1.9%), b_comp_prob: 4.1 (0.6%), b_tok_touch_all: 48 (6.6%), b_finish: 0.88 (0.1%), tests_pri_0: 572 (79.1%), check_dkim_signature: 0.80 (0.1%), check_dkim_adsp: 2.3 (0.3%), poll_dns_idle: 0.62 (0.1%), tests_pri_10: 2.6 (0.4%), tests_pri_500: 14 (1.9%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 3/8] exec: Convert security_bprm_set_creds into security_bprm_repopulate_creds X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Rename bprm->cap_elevated to bprm->active_secureexec and initialize it in prepare_binprm instead of in cap_bprm_set_creds. Initializing bprm->active_secureexec in prepare_binprm allows multiple implementations of security_bprm_repopulate_creds to play nicely with each other. Rename security_bprm_set_creds to security_bprm_reopulate_creds to emphasize that this path recomputes part of bprm->cred. This recomputation avoids the time of check vs time of use problems that are inherent in unix #! interpreters. In short two renames and a move in the location of initializing bprm->active_secureexec. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- fs/exec.c | 8 ++++---- include/linux/binfmts.h | 4 ++-- include/linux/lsm_hook_defs.h | 2 +- include/linux/lsm_hooks.h | 4 ++-- include/linux/security.h | 8 ++++---- security/commoncap.c | 9 ++++----- security/security.c | 4 ++-- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 9e70da47f8d9..8e3b93d51d31 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1366,7 +1366,7 @@ int begin_new_exec(struct linux_binprm * bprm) * the final state of setuid/setgid/fscaps can be merged into the * secureexec flag. */ - bprm->secureexec |= bprm->cap_elevated; + bprm->secureexec |= bprm->active_secureexec; if (bprm->secureexec) { /* Make sure parent cannot signal privileged process. */ @@ -1634,10 +1634,10 @@ int prepare_binprm(struct linux_binprm *bprm) int retval; loff_t pos = 0; + /* Recompute parts of bprm->cred based on bprm->file */ + bprm->active_secureexec = 0; bprm_fill_uid(bprm); - - /* fill in binprm security blob */ - retval = security_bprm_set_creds(bprm); + retval = security_bprm_repopulate_creds(bprm); if (retval) return retval; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index d1217fcdedea..8605ab4a0f89 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -27,10 +27,10 @@ struct linux_binprm { unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int /* - * True if most recent call to cap_bprm_set_creds + * True if most recent call to security_bprm_set_creds * resulted in elevated privileges. */ - cap_elevated:1, + active_secureexec:1, /* * Set by bprm_creds_for_exec hook to indicate a * privilege-gaining exec has happened. Used to set diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index aab0695f41df..1e295ba12c0d 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -50,7 +50,7 @@ LSM_HOOK(int, 0, settime, const struct timespec64 *ts, const struct timezone *tz) LSM_HOOK(int, 0, vm_enough_memory, struct mm_struct *mm, long pages) LSM_HOOK(int, 0, bprm_creds_for_exec, struct linux_binprm *bprm) -LSM_HOOK(int, 0, bprm_set_creds, struct linux_binprm *bprm) +LSM_HOOK(int, 0, bprm_repopulate_creds, struct linux_binprm *bprm) LSM_HOOK(int, 0, bprm_check_security, struct linux_binprm *bprm) LSM_HOOK(void, LSM_RET_VOID, bprm_committing_creds, struct linux_binprm *bprm) LSM_HOOK(void, LSM_RET_VOID, bprm_committed_creds, struct linux_binprm *bprm) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index c719af37df20..d618ecc4d660 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -44,7 +44,7 @@ * request libc enable secure mode. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. - * @bprm_set_creds: + * @bprm_repopulate_creds: * Assuming that the relevant bits of @bprm->cred->security have been * previously set, examine @bprm->file and regenerate them. This is * so that the credentials derived from the interpreter the code is @@ -53,7 +53,7 @@ * reopen script, and may end up opening something completely different. * This hook may also optionally check permissions (e.g. for * transitions between security domains). - * The hook must set @bprm->cap_elevated to 1 if AT_SECURE should be set to + * The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to * request libc enable secure mode. * @bprm contains the linux_binprm structure. * Return 0 if the hook is successful and permission is granted. diff --git a/include/linux/security.h b/include/linux/security.h index 1bd7a6582775..d23f078eb589 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -140,7 +140,7 @@ extern int cap_capset(struct cred *new, const struct cred *old, const kernel_cap_t *effective, const kernel_cap_t *inheritable, const kernel_cap_t *permitted); -extern int cap_bprm_set_creds(struct linux_binprm *bprm); +extern int cap_bprm_repopulate_creds(struct linux_binprm *bprm); extern int cap_inode_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags); extern int cap_inode_removexattr(struct dentry *dentry, const char *name); @@ -277,7 +277,7 @@ int security_syslog(int type); int security_settime64(const struct timespec64 *ts, const struct timezone *tz); int security_vm_enough_memory_mm(struct mm_struct *mm, long pages); int security_bprm_creds_for_exec(struct linux_binprm *bprm); -int security_bprm_set_creds(struct linux_binprm *bprm); +int security_bprm_repopulate_creds(struct linux_binprm *bprm); int security_bprm_check(struct linux_binprm *bprm); void security_bprm_committing_creds(struct linux_binprm *bprm); void security_bprm_committed_creds(struct linux_binprm *bprm); @@ -575,9 +575,9 @@ static inline int security_bprm_creds_for_exec(struct linux_binprm *bprm) return 0; } -static inline int security_bprm_set_creds(struct linux_binprm *bprm) +static inline int security_bprm_repopulate_creds(struct linux_binprm *bprm) { - return cap_bprm_set_creds(bprm); + return cap_bprm_repopluate_creds(bprm); } static inline int security_bprm_check(struct linux_binprm *bprm) diff --git a/security/commoncap.c b/security/commoncap.c index f4ee0ae106b2..045b5b80ea40 100644 --- a/security/commoncap.c +++ b/security/commoncap.c @@ -797,14 +797,14 @@ static inline bool nonroot_raised_pE(struct cred *new, const struct cred *old, } /** - * cap_bprm_set_creds - Set up the proposed credentials for execve(). + * cap_bprm_repopulate_creds - Set up the proposed credentials for execve(). * @bprm: The execution parameters, including the proposed creds * * Set up the proposed credentials for a new execution context being * constructed by execve(). The proposed creds in @bprm->cred is altered, * which won't take effect immediately. Returns 0 if successful, -ve on error. */ -int cap_bprm_set_creds(struct linux_binprm *bprm) +int cap_bprm_repopulate_creds(struct linux_binprm *bprm) { const struct cred *old = current_cred(); struct cred *new = bprm->cred; @@ -884,12 +884,11 @@ int cap_bprm_set_creds(struct linux_binprm *bprm) return -EPERM; /* Check for privilege-elevated exec. */ - bprm->cap_elevated = 0; if (is_setid || (!__is_real(root_uid, new) && (effective || __cap_grew(permitted, ambient, new)))) - bprm->cap_elevated = 1; + bprm->active_secureexec = 1; return 0; } @@ -1346,7 +1345,7 @@ static struct security_hook_list capability_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(ptrace_traceme, cap_ptrace_traceme), LSM_HOOK_INIT(capget, cap_capget), LSM_HOOK_INIT(capset, cap_capset), - LSM_HOOK_INIT(bprm_set_creds, cap_bprm_set_creds), + LSM_HOOK_INIT(bprm_repopulate_creds, cap_bprm_repopulate_creds), LSM_HOOK_INIT(inode_need_killpriv, cap_inode_need_killpriv), LSM_HOOK_INIT(inode_killpriv, cap_inode_killpriv), LSM_HOOK_INIT(inode_getsecurity, cap_inode_getsecurity), diff --git a/security/security.c b/security/security.c index 4ee76a729f73..b890b7e2a765 100644 --- a/security/security.c +++ b/security/security.c @@ -828,9 +828,9 @@ int security_bprm_creds_for_exec(struct linux_binprm *bprm) return call_int_hook(bprm_creds_for_exec, 0, bprm); } -int security_bprm_set_creds(struct linux_binprm *bprm) +int security_bprm_repopulate_creds(struct linux_binprm *bprm) { - return call_int_hook(bprm_set_creds, 0, bprm); + return call_int_hook(bprm_repopulate_creds, 0, bprm); } int security_bprm_check(struct linux_binprm *bprm) From patchwork Tue May 19 00:31:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556585 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E8CDD1391 for ; Tue, 19 May 2020 00:35:34 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D5D5B20657 for ; Tue, 19 May 2020 00:35:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726985AbgESAfe (ORCPT ); Mon, 18 May 2020 20:35:34 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:37512 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726494AbgESAfd (ORCPT ); Mon, 18 May 2020 20:35:33 -0400 Received: from in01.mta.xmission.com ([166.70.13.51]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqEG-0000Vb-7M; Mon, 18 May 2020 18:35:32 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqEF-0003a2-Dh; Mon, 18 May 2020 18:35:32 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:31:51 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87imgszrwo.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqEF-0003a2-Dh;;;mid=<87imgszrwo.fsf_-_@x220.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+zSDC+oPj2CKUq9mJvOsL3qCDQjL7cYcM= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa02.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TM2_M_HEADER_IN_MSG,T_TooManySym_01,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Virus: No X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa02 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa02 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 403 ms - load_scoreonly_sql: 0.02 (0.0%), signal_user_changed: 3.8 (0.9%), b_tie_ro: 2.6 (0.6%), parse: 1.17 (0.3%), extract_message_metadata: 11 (2.8%), get_uri_detail_list: 2.3 (0.6%), tests_pri_-1000: 11 (2.7%), tests_pri_-950: 1.03 (0.3%), tests_pri_-900: 0.82 (0.2%), tests_pri_-90: 64 (15.9%), check_bayes: 63 (15.6%), b_tokenize: 7 (1.8%), b_tok_get_all: 6 (1.4%), b_comp_prob: 1.78 (0.4%), b_tok_touch_all: 46 (11.4%), b_finish: 0.63 (0.2%), tests_pri_0: 298 (73.9%), check_dkim_signature: 0.39 (0.1%), check_dkim_adsp: 2.6 (0.7%), poll_dns_idle: 1.29 (0.3%), tests_pri_10: 2.4 (0.6%), tests_pri_500: 7 (1.6%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 4/8] exec: Allow load_misc_binary to call prepare_binfmt unconditionally X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Add a flag preserve_creds that binfmt_misc can set to prevent credentials from being updated. This allows binfmt_misc to always call prepare_binfmt. Allowing the credential computation logic to be consolidated. Not replacing the credentials with the interpreters credentials is safe because because an open file descriptor to the executable is passed to the interpreter. As the interpreter does not need to reopen the executable it is guaranteed to see the same file that exec sees. Ref: c407c033de84 ("[PATCH] binfmt_misc: improve calculation of interpreter's credentials") Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- fs/binfmt_misc.c | 15 +++------------ fs/exec.c | 19 ++++++++++++------- include/linux/binfmts.h | 2 ++ 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index cdb45829354d..264829745d6f 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -218,19 +218,10 @@ static int load_misc_binary(struct linux_binprm *bprm) goto error; bprm->file = interp_file; - if (fmt->flags & MISC_FMT_CREDENTIALS) { - loff_t pos = 0; - - /* - * No need to call prepare_binprm(), it's already been - * done. bprm->buf is stale, update from interp_file. - */ - memset(bprm->buf, 0, BINPRM_BUF_SIZE); - retval = kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, - &pos); - } else - retval = prepare_binprm(bprm); + if (fmt->flags & MISC_FMT_CREDENTIALS) + bprm->preserve_creds = 1; + retval = prepare_binprm(bprm); if (retval < 0) goto error; diff --git a/fs/exec.c b/fs/exec.c index 8e3b93d51d31..028e0e323af5 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1631,15 +1631,20 @@ static void bprm_fill_uid(struct linux_binprm *bprm) */ int prepare_binprm(struct linux_binprm *bprm) { - int retval; loff_t pos = 0; - /* Recompute parts of bprm->cred based on bprm->file */ - bprm->active_secureexec = 0; - bprm_fill_uid(bprm); - retval = security_bprm_repopulate_creds(bprm); - if (retval) - return retval; + /* Can the interpreter get to the executable without races? */ + if (!bprm->preserve_creds) { + int retval; + + /* Recompute parts of bprm->cred based on bprm->file */ + bprm->active_secureexec = 0; + bprm_fill_uid(bprm); + retval = security_bprm_repopulate_creds(bprm); + if (retval) + return retval; + } + bprm->preserve_creds = 0; memset(bprm->buf, 0, BINPRM_BUF_SIZE); return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos); diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 8605ab4a0f89..dbb5614d62a2 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -26,6 +26,8 @@ struct linux_binprm { unsigned long p; /* current top of mem */ unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int + /* It is safe to use the creds of a script (see binfmt_misc) */ + preserve_creds:1, /* * True if most recent call to security_bprm_set_creds * resulted in elevated privileges. From patchwork Tue May 19 00:32:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556591 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 60DD71391 for ; Tue, 19 May 2020 00:36:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 534472072C for ; Tue, 19 May 2020 00:36:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726958AbgESAgA (ORCPT ); Mon, 18 May 2020 20:36:00 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:59490 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726957AbgESAgA (ORCPT ); Mon, 18 May 2020 20:36:00 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqEh-0002js-2Y; Mon, 18 May 2020 18:35:59 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqEg-0004V7-2k; Mon, 18 May 2020 18:35:58 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:32:18 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87d070zrvx.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqEg-0004V7-2k;;;mid=<87d070zrvx.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/9roiBo9Zi8ekR3M2JucjX1nO39mpsBoU= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa08.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa08 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa08 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 417 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 16 (3.8%), b_tie_ro: 14 (3.3%), parse: 1.59 (0.4%), extract_message_metadata: 14 (3.3%), get_uri_detail_list: 2.4 (0.6%), tests_pri_-1000: 14 (3.4%), tests_pri_-950: 1.36 (0.3%), tests_pri_-900: 1.25 (0.3%), tests_pri_-90: 63 (15.1%), check_bayes: 61 (14.5%), b_tokenize: 10 (2.4%), b_tok_get_all: 10 (2.5%), b_comp_prob: 3.0 (0.7%), b_tok_touch_all: 33 (7.8%), b_finish: 1.35 (0.3%), tests_pri_0: 290 (69.6%), check_dkim_signature: 0.56 (0.1%), check_dkim_adsp: 3.0 (0.7%), poll_dns_idle: 1.22 (0.3%), tests_pri_10: 2.6 (0.6%), tests_pri_500: 9 (2.1%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 5/8] exec: Move the call of prepare_binprm into search_binary_handler X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The code in prepare_binary_handler needs to be run every time search_binary_handler is called so move the call into search_binary_handler itself to make the code simpler and easier to understand. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook Reviewed-by: James Morris --- arch/alpha/kernel/binfmt_loader.c | 3 --- fs/binfmt_em86.c | 4 ---- fs/binfmt_misc.c | 4 ---- fs/binfmt_script.c | 3 --- fs/exec.c | 12 +++++------- include/linux/binfmts.h | 1 - 6 files changed, 5 insertions(+), 22 deletions(-) diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c index a8d0d6e06526..d712ba51d15a 100644 --- a/arch/alpha/kernel/binfmt_loader.c +++ b/arch/alpha/kernel/binfmt_loader.c @@ -35,9 +35,6 @@ static int load_binary(struct linux_binprm *bprm) bprm->file = file; bprm->loader = loader; - retval = prepare_binprm(bprm); - if (retval < 0) - return retval; return search_binary_handler(bprm); } diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index 466497860c62..cedde2341ade 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -91,10 +91,6 @@ static int load_em86(struct linux_binprm *bprm) bprm->file = file; - retval = prepare_binprm(bprm); - if (retval < 0) - return retval; - return search_binary_handler(bprm); } diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 264829745d6f..50a73afdf9b7 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -221,10 +221,6 @@ static int load_misc_binary(struct linux_binprm *bprm) if (fmt->flags & MISC_FMT_CREDENTIALS) bprm->preserve_creds = 1; - retval = prepare_binprm(bprm); - if (retval < 0) - goto error; - retval = search_binary_handler(bprm); if (retval < 0) goto error; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index e9e6a6f4a35f..8d718d8fd0fe 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -143,9 +143,6 @@ static int load_script(struct linux_binprm *bprm) return PTR_ERR(file); bprm->file = file; - retval = prepare_binprm(bprm); - if (retval < 0) - return retval; return search_binary_handler(bprm); } diff --git a/fs/exec.c b/fs/exec.c index 028e0e323af5..5fc458460e44 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1629,7 +1629,7 @@ static void bprm_fill_uid(struct linux_binprm *bprm) * * This may be called multiple times for binary chains (scripts for example). */ -int prepare_binprm(struct linux_binprm *bprm) +static int prepare_binprm(struct linux_binprm *bprm) { loff_t pos = 0; @@ -1650,8 +1650,6 @@ int prepare_binprm(struct linux_binprm *bprm) return kernel_read(bprm->file, bprm->buf, BINPRM_BUF_SIZE, &pos); } -EXPORT_SYMBOL(prepare_binprm); - /* * Arguments are '\0' separated strings found at the location bprm->p * points to; chop off the first by relocating brpm->p to right after @@ -1707,6 +1705,10 @@ int search_binary_handler(struct linux_binprm *bprm) if (bprm->recursion_depth > 5) return -ELOOP; + retval = prepare_binprm(bprm); + if (retval < 0) + return retval; + retval = security_bprm_check(bprm); if (retval) return retval; @@ -1864,10 +1866,6 @@ static int __do_execve_file(int fd, struct filename *filename, if (retval) goto out; - retval = prepare_binprm(bprm); - if (retval < 0) - goto out; - retval = copy_strings_kernel(1, &bprm->filename, bprm); if (retval < 0) goto out; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index dbb5614d62a2..8c7779d6bf19 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -116,7 +116,6 @@ static inline void insert_binfmt(struct linux_binfmt *fmt) extern void unregister_binfmt(struct linux_binfmt *); -extern int prepare_binprm(struct linux_binprm *); extern int __must_check remove_arg_zero(struct linux_binprm *); extern int search_binary_handler(struct linux_binprm *); extern int begin_new_exec(struct linux_binprm * bprm); From patchwork Tue May 19 00:33:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556593 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1EE221391 for ; Tue, 19 May 2020 00:37:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0A9BF20657 for ; Tue, 19 May 2020 00:37:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726721AbgESAhE (ORCPT ); Mon, 18 May 2020 20:37:04 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:45682 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726442AbgESAhE (ORCPT ); Mon, 18 May 2020 20:37:04 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out03.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqFi-0003xR-52; Mon, 18 May 2020 18:37:02 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqFh-0004a3-3C; Mon, 18 May 2020 18:37:01 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:33:21 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <874ksczru6.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqFh-0004a3-3C;;;mid=<874ksczru6.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1973r7WyKjyu1LuSucdEGRqDGPfyMVGYww= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,T_TooManySym_02,T_TooManySym_03, T_TooManySym_04,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.4986] * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_03 6+ unique symbols in subject * 0.0 T_TooManySym_04 7+ unique symbols in subject * 0.0 T_TooManySym_01 4+ unique symbols in subject * 0.0 T_TooManySym_02 5+ unique symbols in subject X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 576 ms - load_scoreonly_sql: 0.05 (0.0%), signal_user_changed: 11 (1.8%), b_tie_ro: 9 (1.6%), parse: 1.04 (0.2%), extract_message_metadata: 13 (2.2%), get_uri_detail_list: 3.1 (0.5%), tests_pri_-1000: 13 (2.3%), tests_pri_-950: 1.27 (0.2%), tests_pri_-900: 1.02 (0.2%), tests_pri_-90: 70 (12.2%), check_bayes: 69 (12.0%), b_tokenize: 13 (2.3%), b_tok_get_all: 11 (1.9%), b_comp_prob: 3.2 (0.6%), b_tok_touch_all: 38 (6.6%), b_finish: 0.92 (0.2%), tests_pri_0: 447 (77.6%), check_dkim_signature: 0.66 (0.1%), check_dkim_adsp: 2.7 (0.5%), poll_dns_idle: 0.69 (0.1%), tests_pri_10: 3.4 (0.6%), tests_pri_500: 13 (2.2%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 6/8] exec/binfmt_script: Don't modify bprm->buf and then return -ENOEXEC X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: The return code -ENOEXEC serves to tell search_binary_handler that it should continue searching for the binfmt to handle a given file. This makes return -ENOEXEC with a bprm->buf that is needed to continue the search problematic. The current binfmt_script manages to escape problems as it closes and clears bprm->file before return -ENOEXEC with bprm->buf modified. This prevents search_binary_handler from looping as it explicitly handles a NULL bprm->file. I plan on moving all of the bprm->file managment into fs/exec.c and out of the binary handlers so this will become a problem. Move closing bprm->file and the test for BINPRM_PATH_INACCESSIBLE down below the last return of -ENOEXEC. Introduce i_sep and i_end to track the end of the first argument and the end of the parameters respectively. Using those, constification of all char * pointers, and the helpers next_terminator and next_non_spacetab guarantee the parameter parsing will not modify bprm->buf. Only modify bprm->buf to terminate the strings i_arg and i_name with '\0' for passing to copy_strings_kernel. When replacing loops with next_non_spacetab and next_terminator care has been take that the logic of the parsing code (short of replacing characters by '\0') remains the same. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- fs/binfmt_script.c | 80 ++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 8d718d8fd0fe..85e0ef86eb11 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -16,14 +16,14 @@ #include static inline bool spacetab(char c) { return c == ' ' || c == '\t'; } -static inline char *next_non_spacetab(char *first, const char *last) +static inline const char *next_non_spacetab(const char *first, const char *last) { for (; first <= last; first++) if (!spacetab(*first)) return first; return NULL; } -static inline char *next_terminator(char *first, const char *last) +static inline const char *next_terminator(const char *first, const char *last) { for (; first <= last; first++) if (spacetab(*first) || !*first) @@ -33,8 +33,7 @@ static inline char *next_terminator(char *first, const char *last) static int load_script(struct linux_binprm *bprm) { - const char *i_arg, *i_name; - char *cp, *buf_end; + const char *i_name, *i_sep, *i_arg, *i_end, *buf_end; struct file *file; int retval; @@ -42,20 +41,6 @@ static int load_script(struct linux_binprm *bprm) if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!')) return -ENOEXEC; - /* - * If the script filename will be inaccessible after exec, typically - * because it is a "/dev/fd//.." path against an O_CLOEXEC fd, give - * up now (on the assumption that the interpreter will want to load - * this file). - */ - if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) - return -ENOENT; - - /* Release since we are not mapping a binary into memory. */ - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - /* * This section handles parsing the #! line into separate * interpreter path and argument strings. We must be careful @@ -71,39 +56,48 @@ static int load_script(struct linux_binprm *bprm) * parse them on its own. */ buf_end = bprm->buf + sizeof(bprm->buf) - 1; - cp = strnchr(bprm->buf, sizeof(bprm->buf), '\n'); - if (!cp) { - cp = next_non_spacetab(bprm->buf + 2, buf_end); - if (!cp) + i_end = strnchr(bprm->buf, sizeof(bprm->buf), '\n'); + if (!i_end) { + i_end = next_non_spacetab(bprm->buf + 2, buf_end); + if (!i_end) return -ENOEXEC; /* Entire buf is spaces/tabs */ /* * If there is no later space/tab/NUL we must assume the * interpreter path is truncated. */ - if (!next_terminator(cp, buf_end)) + if (!next_terminator(i_end, buf_end)) return -ENOEXEC; - cp = buf_end; + i_end = buf_end; } - /* NUL-terminate the buffer and any trailing spaces/tabs. */ - *cp = '\0'; - while (cp > bprm->buf) { - cp--; - if ((*cp == ' ') || (*cp == '\t')) - *cp = '\0'; - else - break; - } - for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++); - if (*cp == '\0') + /* Trim any trailing spaces/tabs from i_end */ + while (spacetab(i_end[-1])) + i_end--; + + /* Skip over leading spaces/tabs */ + i_name = next_non_spacetab(bprm->buf+2, i_end); + if (!i_name || (i_name == i_end)) return -ENOEXEC; /* No interpreter name found */ - i_name = cp; + + /* Is there an optional argument? */ i_arg = NULL; - for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++) - /* nothing */ ; - while ((*cp == ' ') || (*cp == '\t')) - *cp++ = '\0'; - if (*cp) - i_arg = cp; + i_sep = next_terminator(i_name, i_end); + if (i_sep && (*i_sep != '\0')) + i_arg = next_non_spacetab(i_sep, i_end); + + /* + * If the script filename will be inaccessible after exec, typically + * because it is a "/dev/fd//.." path against an O_CLOEXEC fd, give + * up now (on the assumption that the interpreter will want to load + * this file). + */ + if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) + return -ENOENT; + + /* Release since we are not mapping a binary into memory. */ + allow_write_access(bprm->file); + fput(bprm->file); + bprm->file = NULL; + /* * OK, we've parsed out the interpreter name and * (optional) argument. @@ -121,7 +115,9 @@ static int load_script(struct linux_binprm *bprm) if (retval < 0) return retval; bprm->argc++; + *((char *)i_end) = '\0'; if (i_arg) { + *((char *)i_sep) = '\0'; retval = copy_strings_kernel(1, &i_arg, bprm); if (retval < 0) return retval; From patchwork Tue May 19 00:33:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556599 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 42ABA618 for ; Tue, 19 May 2020 00:37:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 30F182070A for ; Tue, 19 May 2020 00:37:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727007AbgESAh2 (ORCPT ); Mon, 18 May 2020 20:37:28 -0400 Received: from out01.mta.xmission.com ([166.70.13.231]:37812 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726442AbgESAh2 (ORCPT ); Mon, 18 May 2020 20:37:28 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out01.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqG7-0000am-3z; Mon, 18 May 2020 18:37:27 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqG5-0004ce-WA; Mon, 18 May 2020 18:37:26 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:33:46 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87y2poyd91.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqG5-0004ce-WA;;;mid=<87y2poyd91.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+hBvVAJEz5VTuL9F3pCoTSZ4Vfzxiz3sQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: X-Spam-Status: No, score=-0.2 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 638 ms - load_scoreonly_sql: 0.03 (0.0%), signal_user_changed: 11 (1.7%), b_tie_ro: 10 (1.5%), parse: 1.23 (0.2%), extract_message_metadata: 14 (2.3%), get_uri_detail_list: 4.1 (0.6%), tests_pri_-1000: 13 (2.1%), tests_pri_-950: 1.26 (0.2%), tests_pri_-900: 1.03 (0.2%), tests_pri_-90: 75 (11.7%), check_bayes: 73 (11.5%), b_tokenize: 16 (2.5%), b_tok_get_all: 12 (1.9%), b_comp_prob: 3.3 (0.5%), b_tok_touch_all: 39 (6.0%), b_finish: 0.82 (0.1%), tests_pri_0: 508 (79.6%), check_dkim_signature: 0.68 (0.1%), check_dkim_adsp: 2.5 (0.4%), poll_dns_idle: 0.79 (0.1%), tests_pri_10: 2.1 (0.3%), tests_pri_500: 7 (1.2%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 7/8] exec: Generic execfd support X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Most of the support for passing the file descriptor of an executable to an interpreter already lives in the generic code and in binfmt_elf. Rework the fields in binfmt_elf that deal with executable file descriptor passing to make executable file descriptor passing a first class concept. Move the fd_install from binfmt_misc into begin_new_exec after the new creds have been installed. This means that accessing the file through /proc//fd/N is able to see the creds for the new executable before allowing access to the new executables files. Performing the install of the executables file descriptor after the point of no return also means that nothing special needs to be done on error. The exiting of the process will close all of it's open files. Move the would_dump from binfmt_misc into begin_new_exec right after would_dump is called on the bprm->file. This makes it obvious this case exists and that no nesting of bprm->file is currently supported. In binfmt_misc the movement of fd_install into generic code means that it's special error exit path is no longer needed. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- fs/binfmt_elf.c | 4 ++-- fs/binfmt_elf_fdpic.c | 4 ++-- fs/binfmt_misc.c | 40 ++++++++-------------------------------- fs/exec.c | 15 +++++++++++++++ include/linux/binfmts.h | 10 +++++----- 5 files changed, 32 insertions(+), 41 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 396d5c2e6b5e..441c85f04dfd 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -273,8 +273,8 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, NEW_AUX_ENT(AT_BASE_PLATFORM, (elf_addr_t)(unsigned long)u_base_platform); } - if (bprm->interp_flags & BINPRM_FLAGS_EXECFD) { - NEW_AUX_ENT(AT_EXECFD, bprm->interp_data); + if (bprm->have_execfd) { + NEW_AUX_ENT(AT_EXECFD, bprm->execfd); } #undef NEW_AUX_ENT /* AT_NULL is zero; clear the rest too */ diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c index 896e3ca9bf85..2d5e9eb12075 100644 --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -628,10 +628,10 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm, (elf_addr_t) (unsigned long) u_base_platform); } - if (bprm->interp_flags & BINPRM_FLAGS_EXECFD) { + if (bprm->have_execfd) { nr = 0; csp -= 2 * sizeof(unsigned long); - NEW_AUX_ENT(AT_EXECFD, bprm->interp_data); + NEW_AUX_ENT(AT_EXECFD, bprm->execfd); } nr = 0; diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index 50a73afdf9b7..ad2866f28f0c 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -134,7 +134,6 @@ static int load_misc_binary(struct linux_binprm *bprm) Node *fmt; struct file *interp_file = NULL; int retval; - int fd_binary = -1; retval = -ENOEXEC; if (!enabled) @@ -161,29 +160,12 @@ static int load_misc_binary(struct linux_binprm *bprm) } if (fmt->flags & MISC_FMT_OPEN_BINARY) { - - /* if the binary should be opened on behalf of the - * interpreter than keep it open and assign descriptor - * to it - */ - fd_binary = get_unused_fd_flags(0); - if (fd_binary < 0) { - retval = fd_binary; - goto ret; - } - fd_install(fd_binary, bprm->file); - - /* if the binary is not readable than enforce mm->dumpable=0 - regardless of the interpreter's permissions */ - would_dump(bprm, bprm->file); + /* Pass the open binary to the interpreter */ + bprm->have_execfd = 1; + bprm->executable = bprm->file; allow_write_access(bprm->file); bprm->file = NULL; - - /* mark the bprm that fd should be passed to interp */ - bprm->interp_flags |= BINPRM_FLAGS_EXECFD; - bprm->interp_data = fd_binary; - } else { allow_write_access(bprm->file); fput(bprm->file); @@ -192,19 +174,19 @@ static int load_misc_binary(struct linux_binprm *bprm) /* make argv[1] be the path to the binary */ retval = copy_strings_kernel(1, &bprm->interp, bprm); if (retval < 0) - goto error; + goto ret; bprm->argc++; /* add the interp as argv[0] */ retval = copy_strings_kernel(1, &fmt->interpreter, bprm); if (retval < 0) - goto error; + goto ret; bprm->argc++; /* Update interp in case binfmt_script needs it. */ retval = bprm_change_interp(fmt->interpreter, bprm); if (retval < 0) - goto error; + goto ret; if (fmt->flags & MISC_FMT_OPEN_FILE) { interp_file = file_clone_open(fmt->interp_file); @@ -215,7 +197,7 @@ static int load_misc_binary(struct linux_binprm *bprm) } retval = PTR_ERR(interp_file); if (IS_ERR(interp_file)) - goto error; + goto ret; bprm->file = interp_file; if (fmt->flags & MISC_FMT_CREDENTIALS) @@ -223,17 +205,11 @@ static int load_misc_binary(struct linux_binprm *bprm) retval = search_binary_handler(bprm); if (retval < 0) - goto error; + goto ret; ret: dput(fmt->dentry); return retval; -error: - if (fd_binary > 0) - ksys_close(fd_binary); - bprm->interp_flags = 0; - bprm->interp_data = 0; - goto ret; } /* Command parsers */ diff --git a/fs/exec.c b/fs/exec.c index 5fc458460e44..ca91393893ea 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1323,7 +1323,10 @@ int begin_new_exec(struct linux_binprm * bprm) */ set_mm_exe_file(bprm->mm, bprm->file); + /* If the binary is not readable than enforce mm->dumpable=0 */ would_dump(bprm, bprm->file); + if (bprm->have_execfd) + would_dump(bprm, bprm->executable); /* * Release all of the old mmap stuff @@ -1427,6 +1430,16 @@ int begin_new_exec(struct linux_binprm * bprm) * credentials; any time after this it may be unlocked. */ security_bprm_committed_creds(bprm); + + /* Pass the opened binary to the interpreter. */ + if (bprm->have_execfd) { + retval = get_unused_fd_flags(0); + if (retval < 0) + goto out_unlock; + fd_install(retval, bprm->executable); + bprm->executable = NULL; + bprm->execfd = retval; + } return 0; out_unlock: @@ -1516,6 +1529,8 @@ static void free_bprm(struct linux_binprm *bprm) allow_write_access(bprm->file); fput(bprm->file); } + if (bprm->executable) + fput(bprm->executable); /* If a binfmt changed the interp, free it. */ if (bprm->interp != bprm->filename) kfree(bprm->interp); diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 8c7779d6bf19..653508b25815 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -26,6 +26,9 @@ struct linux_binprm { unsigned long p; /* current top of mem */ unsigned long argmin; /* rlimit marker for copy_strings() */ unsigned int + /* Should an execfd be passed to userspace? */ + have_execfd:1, + /* It is safe to use the creds of a script (see binfmt_misc) */ preserve_creds:1, /* @@ -48,6 +51,7 @@ struct linux_binprm { unsigned int taso:1; #endif unsigned int recursion_depth; /* only for search_binary_handler() */ + struct file * executable; /* Executable to pass to the interpreter */ struct file * file; struct cred *cred; /* new credentials */ int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */ @@ -58,7 +62,7 @@ struct linux_binprm { of the time same as filename, but could be different for binfmt_{misc,script} */ unsigned interp_flags; - unsigned interp_data; + int execfd; /* File descriptor of the executable */ unsigned long loader, exec; struct rlimit rlim_stack; /* Saved RLIMIT_STACK used during exec. */ @@ -69,10 +73,6 @@ struct linux_binprm { #define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0 #define BINPRM_FLAGS_ENFORCE_NONDUMP (1 << BINPRM_FLAGS_ENFORCE_NONDUMP_BIT) -/* fd of the binary should be passed to the interpreter */ -#define BINPRM_FLAGS_EXECFD_BIT 1 -#define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT) - /* filename of the binary will be inaccessible after exec */ #define BINPRM_FLAGS_PATH_INACCESSIBLE_BIT 2 #define BINPRM_FLAGS_PATH_INACCESSIBLE (1 << BINPRM_FLAGS_PATH_INACCESSIBLE_BIT) From patchwork Tue May 19 00:34:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Eric W. Biederman" X-Patchwork-Id: 11556601 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B8BA41391 for ; Tue, 19 May 2020 00:38:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A22E22075F for ; Tue, 19 May 2020 00:38:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726442AbgESAiD (ORCPT ); Mon, 18 May 2020 20:38:03 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:59706 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726713AbgESAiC (ORCPT ); Mon, 18 May 2020 20:38:02 -0400 Received: from in02.mta.xmission.com ([166.70.13.52]) by out02.mta.xmission.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1jaqGe-0002w7-Jc; Mon, 18 May 2020 18:38:00 -0600 Received: from ip68-227-160-95.om.om.cox.net ([68.227.160.95] helo=x220.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.87) (envelope-from ) id 1jaqGd-0004lP-G5; Mon, 18 May 2020 18:38:00 -0600 From: ebiederm@xmission.com (Eric W. Biederman) To: Cc: Linus Torvalds , Oleg Nesterov , Jann Horn , Kees Cook , Greg Ungerer , Rob Landley , Bernd Edlinger , , Al Viro , Alexey Dobriyan , Andrew Morton , Casey Schaufler , linux-security-module@vger.kernel.org, James Morris , "Serge E. Hallyn" , Andy Lutomirski References: <87h7wujhmz.fsf@x220.int.ebiederm.org> <87sgga6ze4.fsf@x220.int.ebiederm.org> <87v9l4zyla.fsf_-_@x220.int.ebiederm.org> <877dx822er.fsf_-_@x220.int.ebiederm.org> Date: Mon, 18 May 2020 19:34:19 -0500 In-Reply-To: <877dx822er.fsf_-_@x220.int.ebiederm.org> (Eric W. Biederman's message of "Mon, 18 May 2020 19:29:00 -0500") Message-ID: <87sgfwyd84.fsf_-_@x220.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-XM-SPF: eid=1jaqGd-0004lP-G5;;;mid=<87sgfwyd84.fsf_-_@x220.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19NGjIG6pRsk4dc9RIXJzoLeOwe3nHlUBU= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on sa06.xmission.com X-Spam-Level: X-Spam-Status: No, score=0.5 required=8.0 tests=ALL_TRUSTED,BAYES_50, DCC_CHECK_NEGATIVE,T_TooManySym_01,XMSubLong autolearn=disabled version=3.4.2 X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * 0.7 XMSubLong Long Subject * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 0; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: ; sa06 0; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ; X-Spam-Relay-Country: X-Spam-Timing: total 697 ms - load_scoreonly_sql: 0.08 (0.0%), signal_user_changed: 12 (1.7%), b_tie_ro: 10 (1.5%), parse: 1.81 (0.3%), extract_message_metadata: 20 (2.8%), get_uri_detail_list: 5 (0.7%), tests_pri_-1000: 16 (2.2%), tests_pri_-950: 1.57 (0.2%), tests_pri_-900: 1.30 (0.2%), tests_pri_-90: 80 (11.5%), check_bayes: 78 (11.2%), b_tokenize: 18 (2.6%), b_tok_get_all: 12 (1.8%), b_comp_prob: 3.9 (0.6%), b_tok_touch_all: 40 (5.7%), b_finish: 0.93 (0.1%), tests_pri_0: 550 (78.9%), check_dkim_signature: 0.77 (0.1%), check_dkim_adsp: 2.7 (0.4%), poll_dns_idle: 0.84 (0.1%), tests_pri_10: 3.0 (0.4%), tests_pri_500: 7 (1.0%), rewrite_mail: 0.00 (0.0%) Subject: [PATCH v2 8/8] exec: Remove recursion from search_binary_handler X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: Recursion in kernel code is generally a bad idea as it can overflow the kernel stack. Recursion in exec also hides that the code is looping and that the loop changes bprm->file. Instead of recursing in search_binary_handler have the methods that would recurse set bprm->interpreter and return 0. Modify exec_binprm to loop when bprm->interpreter is set. Consolidate all of the reassignments of bprm->file in that loop to make it clear what is going on. The structure of the new loop in exec_binprm is that all errors return immediately, while successful completion (ret == 0 && !bprm->interpreter) just breaks out of the loop and runs what exec_bprm has always run upon successful completion. Fail if the an interpreter is being call after execfd has been set. The code has never properly handled an interpreter being called with execfd being set and with reassignments of bprm->file and the assignment of bprm->executable in generic code it has finally become possible to test and fail when if this problematic condition happens. With the reassignments of bprm->file and the assignment of bprm->executable moved into the generic code add a test to see if bprm->executable is being reassigned. In search_binary_handler remove the test for !bprm->file. With all reassignments of bprm->file moved to exec_binprm bprm->file can never be NULL in search_binary_handler. Signed-off-by: "Eric W. Biederman" Reviewed-by: Kees Cook --- arch/alpha/kernel/binfmt_loader.c | 8 ++--- fs/binfmt_em86.c | 9 ++---- fs/binfmt_misc.c | 18 ++--------- fs/binfmt_script.c | 9 ++---- fs/exec.c | 51 ++++++++++++++++++++----------- include/linux/binfmts.h | 3 +- 6 files changed, 43 insertions(+), 55 deletions(-) diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c index d712ba51d15a..e4be7a543ecf 100644 --- a/arch/alpha/kernel/binfmt_loader.c +++ b/arch/alpha/kernel/binfmt_loader.c @@ -19,10 +19,6 @@ static int load_binary(struct linux_binprm *bprm) if (bprm->loader) return -ENOEXEC; - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - loader = bprm->vma->vm_end - sizeof(void *); file = open_exec("/sbin/loader"); @@ -33,9 +29,9 @@ static int load_binary(struct linux_binprm *bprm) /* Remember if the application is TASO. */ bprm->taso = eh->ah.entry < 0x100000000UL; - bprm->file = file; + bprm->interpreter = file; bprm->loader = loader; - return search_binary_handler(bprm); + return 0; } static struct linux_binfmt loader_format = { diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c index cedde2341ade..995883693cb2 100644 --- a/fs/binfmt_em86.c +++ b/fs/binfmt_em86.c @@ -48,10 +48,6 @@ static int load_em86(struct linux_binprm *bprm) if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) return -ENOENT; - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - /* Unlike in the script case, we don't have to do any hairy * parsing to find our interpreter... it's hardcoded! */ @@ -89,9 +85,8 @@ static int load_em86(struct linux_binprm *bprm) if (IS_ERR(file)) return PTR_ERR(file); - bprm->file = file; - - return search_binary_handler(bprm); + bprm->interpreter = file; + return 0; } static struct linux_binfmt em86_format = { diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c index ad2866f28f0c..53968ea07b57 100644 --- a/fs/binfmt_misc.c +++ b/fs/binfmt_misc.c @@ -159,18 +159,9 @@ static int load_misc_binary(struct linux_binprm *bprm) goto ret; } - if (fmt->flags & MISC_FMT_OPEN_BINARY) { - /* Pass the open binary to the interpreter */ + if (fmt->flags & MISC_FMT_OPEN_BINARY) bprm->have_execfd = 1; - bprm->executable = bprm->file; - allow_write_access(bprm->file); - bprm->file = NULL; - } else { - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - } /* make argv[1] be the path to the binary */ retval = copy_strings_kernel(1, &bprm->interp, bprm); if (retval < 0) @@ -199,14 +190,11 @@ static int load_misc_binary(struct linux_binprm *bprm) if (IS_ERR(interp_file)) goto ret; - bprm->file = interp_file; + bprm->interpreter = interp_file; if (fmt->flags & MISC_FMT_CREDENTIALS) bprm->preserve_creds = 1; - retval = search_binary_handler(bprm); - if (retval < 0) - goto ret; - + retval = 0; ret: dput(fmt->dentry); return retval; diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c index 85e0ef86eb11..0e8b953d12cf 100644 --- a/fs/binfmt_script.c +++ b/fs/binfmt_script.c @@ -93,11 +93,6 @@ static int load_script(struct linux_binprm *bprm) if (bprm->interp_flags & BINPRM_FLAGS_PATH_INACCESSIBLE) return -ENOENT; - /* Release since we are not mapping a binary into memory. */ - allow_write_access(bprm->file); - fput(bprm->file); - bprm->file = NULL; - /* * OK, we've parsed out the interpreter name and * (optional) argument. @@ -138,8 +133,8 @@ static int load_script(struct linux_binprm *bprm) if (IS_ERR(file)) return PTR_ERR(file); - bprm->file = file; - return search_binary_handler(bprm); + bprm->interpreter = file; + return 0; } static struct linux_binfmt script_format = { diff --git a/fs/exec.c b/fs/exec.c index ca91393893ea..47d831e5efde 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1710,16 +1710,12 @@ EXPORT_SYMBOL(remove_arg_zero); /* * cycle the list of binary formats handler, until one recognizes the image */ -int search_binary_handler(struct linux_binprm *bprm) +static int search_binary_handler(struct linux_binprm *bprm) { bool need_retry = IS_ENABLED(CONFIG_MODULES); struct linux_binfmt *fmt; int retval; - /* This allows 4 levels of binfmt rewrites before failing hard. */ - if (bprm->recursion_depth > 5) - return -ELOOP; - retval = prepare_binprm(bprm); if (retval < 0) return retval; @@ -1736,14 +1732,11 @@ int search_binary_handler(struct linux_binprm *bprm) continue; read_unlock(&binfmt_lock); - bprm->recursion_depth++; retval = fmt->load_binary(bprm); - bprm->recursion_depth--; read_lock(&binfmt_lock); put_binfmt(fmt); - if (bprm->point_of_no_return || !bprm->file || - (retval != -ENOEXEC)) { + if (bprm->point_of_no_return || (retval != -ENOEXEC)) { read_unlock(&binfmt_lock); return retval; } @@ -1762,12 +1755,11 @@ int search_binary_handler(struct linux_binprm *bprm) return retval; } -EXPORT_SYMBOL(search_binary_handler); static int exec_binprm(struct linux_binprm *bprm) { pid_t old_pid, old_vpid; - int ret; + int ret, depth; /* Need to fetch pid before load_binary changes it */ old_pid = current->pid; @@ -1775,15 +1767,38 @@ static int exec_binprm(struct linux_binprm *bprm) old_vpid = task_pid_nr_ns(current, task_active_pid_ns(current->parent)); rcu_read_unlock(); - ret = search_binary_handler(bprm); - if (ret >= 0) { - audit_bprm(bprm); - trace_sched_process_exec(current, old_pid, bprm); - ptrace_event(PTRACE_EVENT_EXEC, old_vpid); - proc_exec_connector(current); + /* This allows 4 levels of binfmt rewrites before failing hard. */ + for (depth = 0;; depth++) { + struct file *exec; + if (depth > 5) + return -ELOOP; + + ret = search_binary_handler(bprm); + if (ret < 0) + return ret; + if (!bprm->interpreter) + break; + + exec = bprm->file; + bprm->file = bprm->interpreter; + bprm->interpreter = NULL; + + allow_write_access(exec); + if (unlikely(bprm->have_execfd)) { + if (bprm->executable) { + fput(exec); + return -ENOEXEC; + } + bprm->executable = exec; + } else + fput(exec); } - return ret; + audit_bprm(bprm); + trace_sched_process_exec(current, old_pid, bprm); + ptrace_event(PTRACE_EVENT_EXEC, old_vpid); + proc_exec_connector(current); + return 0; } /* diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 653508b25815..7fc05929c967 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -50,8 +50,8 @@ struct linux_binprm { #ifdef __alpha__ unsigned int taso:1; #endif - unsigned int recursion_depth; /* only for search_binary_handler() */ struct file * executable; /* Executable to pass to the interpreter */ + struct file * interpreter; struct file * file; struct cred *cred; /* new credentials */ int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */ @@ -117,7 +117,6 @@ static inline void insert_binfmt(struct linux_binfmt *fmt) extern void unregister_binfmt(struct linux_binfmt *); extern int __must_check remove_arg_zero(struct linux_binprm *); -extern int search_binary_handler(struct linux_binprm *); extern int begin_new_exec(struct linux_binprm * bprm); extern void setup_new_exec(struct linux_binprm * bprm); extern void finalize_exec(struct linux_binprm *bprm);