From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S944127AbcJSPFx (ORCPT ); Wed, 19 Oct 2016 11:05:53 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:48762 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S944080AbcJSPFb (ORCPT ); Wed, 19 Oct 2016 11:05:31 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Amir Goldstein Cc: Jann Horn , Michal Hocko , linux-kernel , Linux Containers , Oleg Nesterov , Andy Lutomirski , linux-mm@kvack.org, linux-fsdevel References: <87twcbq696.fsf@x220.int.ebiederm.org> <20161018135031.GB13117@dhcp22.suse.cz> <8737jt903u.fsf@xmission.com> <20161018150507.GP14666@pc.thejh.net> <87twc9656s.fsf@xmission.com> <20161018191206.GA1210@laptop.thejh.net> <87r37dnz74.fsf@xmission.com> <87k2d5nytz.fsf_-_@xmission.com> Date: Wed, 19 Oct 2016 08:33:58 -0500 In-Reply-To: (Amir Goldstein's message of "Wed, 19 Oct 2016 09:13:01 +0300") Message-ID: <87mvi0mpix.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1bwr2L-0004Sd-A7;;;mid=<87mvi0mpix.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=75.170.125.99;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/eD6HszWLPZEzSEjOiy6OnUjlA3wG+Oi8= X-SA-Exim-Connect-IP: 75.170.125.99 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.7 XMSubLong Long Subject * 1.5 TR_Symld_Words too many words that have symbols inside * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.4804] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa04 1397; Body=1 Fuz1=1 Fuz2=1] * 0.1 XMSolicitRefs_0 Weightloss drug * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa04 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: **;Amir Goldstein X-Spam-Relay-Country: X-Spam-Timing: total 546 ms - load_scoreonly_sql: 0.07 (0.0%), signal_user_changed: 4.7 (0.9%), b_tie_ro: 3.3 (0.6%), parse: 1.28 (0.2%), extract_message_metadata: 3.9 (0.7%), get_uri_detail_list: 1.48 (0.3%), tests_pri_-1000: 3.7 (0.7%), tests_pri_-950: 0.92 (0.2%), tests_pri_-900: 0.76 (0.1%), tests_pri_-400: 21 (3.8%), check_bayes: 20 (3.6%), b_tokenize: 4.3 (0.8%), b_tok_get_all: 7 (1.2%), b_comp_prob: 1.85 (0.3%), b_tok_touch_all: 4.5 (0.8%), b_finish: 0.90 (0.2%), tests_pri_0: 496 (90.9%), check_dkim_signature: 0.90 (0.2%), check_dkim_adsp: 4.7 (0.9%), tests_pri_500: 3.9 (0.7%), rewrite_mail: 0.00 (0.0%) Subject: Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. 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: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Amir Goldstein writes: >> diff --git a/fs/exec.c b/fs/exec.c >> index 6fcfb3f7b137..f724ed94ba7a 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -1270,12 +1270,21 @@ EXPORT_SYMBOL(flush_old_exec); >> >> void would_dump(struct linux_binprm *bprm, struct file *file) >> { >> - if (inode_permission(file_inode(file), MAY_READ) < 0) >> + struct inode *inode = file_inode(file); >> + if (inode_permission(inode, MAY_READ) < 0) { >> + struct user_namespace *user_ns = current->mm->user_ns; >> bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP; >> + >> + /* May the user_ns root read the executable? */ >> + if (!kuid_has_mapping(user_ns, inode->i_uid) || >> + !kgid_has_mapping(user_ns, inode->i_gid)) { >> + bprm->interp_flags |= BINPRM_FLAGS_EXEC_INACCESSIBLE; >> + } > > This feels like it should belong inside > inode_permission(file_inode(file), MAY_EXEC) > which hopefully should be checked long before getting here?? It is the active ingredient in capable_wrt_inode_uidgid and is indeed inside of inode_permission. What I am testing for here is if I have a process with a full set of capabilities in current->mm->user_ns will the inode be readable. I can see an argument for calling prepare_creds stuffing the new cred full of capabilities. Calling override_cred. Calling inode_permission, restoring the credentials. But it seems very much like overkill and more error prone because of the more code involved. So I have done the simple thing that doesn't hide what is really going on. Eric From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: ebiederm@xmission.com (Eric W. Biederman) To: Amir Goldstein Cc: Jann Horn , Michal Hocko , linux-kernel , Linux Containers , Oleg Nesterov , Andy Lutomirski , linux-mm@kvack.org, linux-fsdevel References: <87twcbq696.fsf@x220.int.ebiederm.org> <20161018135031.GB13117@dhcp22.suse.cz> <8737jt903u.fsf@xmission.com> <20161018150507.GP14666@pc.thejh.net> <87twc9656s.fsf@xmission.com> <20161018191206.GA1210@laptop.thejh.net> <87r37dnz74.fsf@xmission.com> <87k2d5nytz.fsf_-_@xmission.com> Date: Wed, 19 Oct 2016 08:33:58 -0500 In-Reply-To: (Amir Goldstein's message of "Wed, 19 Oct 2016 09:13:01 +0300") Message-ID: <87mvi0mpix.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Sender: owner-linux-mm@kvack.org List-ID: Amir Goldstein writes: >> diff --git a/fs/exec.c b/fs/exec.c >> index 6fcfb3f7b137..f724ed94ba7a 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -1270,12 +1270,21 @@ EXPORT_SYMBOL(flush_old_exec); >> >> void would_dump(struct linux_binprm *bprm, struct file *file) >> { >> - if (inode_permission(file_inode(file), MAY_READ) < 0) >> + struct inode *inode = file_inode(file); >> + if (inode_permission(inode, MAY_READ) < 0) { >> + struct user_namespace *user_ns = current->mm->user_ns; >> bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP; >> + >> + /* May the user_ns root read the executable? */ >> + if (!kuid_has_mapping(user_ns, inode->i_uid) || >> + !kgid_has_mapping(user_ns, inode->i_gid)) { >> + bprm->interp_flags |= BINPRM_FLAGS_EXEC_INACCESSIBLE; >> + } > > This feels like it should belong inside > inode_permission(file_inode(file), MAY_EXEC) > which hopefully should be checked long before getting here?? It is the active ingredient in capable_wrt_inode_uidgid and is indeed inside of inode_permission. What I am testing for here is if I have a process with a full set of capabilities in current->mm->user_ns will the inode be readable. I can see an argument for calling prepare_creds stuffing the new cred full of capabilities. Calling override_cred. Calling inode_permission, restoring the credentials. But it seems very much like overkill and more error prone because of the more code involved. So I have done the simple thing that doesn't hide what is really going on. Eric -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f69.google.com (mail-pa0-f69.google.com [209.85.220.69]) by kanga.kvack.org (Postfix) with ESMTP id 10A546B0069 for ; Wed, 19 Oct 2016 09:36:08 -0400 (EDT) Received: by mail-pa0-f69.google.com with SMTP id os4so11961641pac.5 for ; Wed, 19 Oct 2016 06:36:08 -0700 (PDT) Received: from out02.mta.xmission.com (out02.mta.xmission.com. [166.70.13.232]) by mx.google.com with ESMTPS id e67si40623983pfg.132.2016.10.19.06.36.06 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Oct 2016 06:36:07 -0700 (PDT) From: ebiederm@xmission.com (Eric W. Biederman) References: <87twcbq696.fsf@x220.int.ebiederm.org> <20161018135031.GB13117@dhcp22.suse.cz> <8737jt903u.fsf@xmission.com> <20161018150507.GP14666@pc.thejh.net> <87twc9656s.fsf@xmission.com> <20161018191206.GA1210@laptop.thejh.net> <87r37dnz74.fsf@xmission.com> <87k2d5nytz.fsf_-_@xmission.com> Date: Wed, 19 Oct 2016 08:33:58 -0500 In-Reply-To: (Amir Goldstein's message of "Wed, 19 Oct 2016 09:13:01 +0300") Message-ID: <87mvi0mpix.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [REVIEW][PATCH] exec: Don't exec files the userns root can not read. Sender: owner-linux-mm@kvack.org List-ID: To: Amir Goldstein Cc: Jann Horn , Michal Hocko , linux-kernel , Linux Containers , Oleg Nesterov , Andy Lutomirski , linux-mm@kvack.org, linux-fsdevel Amir Goldstein writes: >> diff --git a/fs/exec.c b/fs/exec.c >> index 6fcfb3f7b137..f724ed94ba7a 100644 >> --- a/fs/exec.c >> +++ b/fs/exec.c >> @@ -1270,12 +1270,21 @@ EXPORT_SYMBOL(flush_old_exec); >> >> void would_dump(struct linux_binprm *bprm, struct file *file) >> { >> - if (inode_permission(file_inode(file), MAY_READ) < 0) >> + struct inode *inode = file_inode(file); >> + if (inode_permission(inode, MAY_READ) < 0) { >> + struct user_namespace *user_ns = current->mm->user_ns; >> bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP; >> + >> + /* May the user_ns root read the executable? */ >> + if (!kuid_has_mapping(user_ns, inode->i_uid) || >> + !kgid_has_mapping(user_ns, inode->i_gid)) { >> + bprm->interp_flags |= BINPRM_FLAGS_EXEC_INACCESSIBLE; >> + } > > This feels like it should belong inside > inode_permission(file_inode(file), MAY_EXEC) > which hopefully should be checked long before getting here?? It is the active ingredient in capable_wrt_inode_uidgid and is indeed inside of inode_permission. What I am testing for here is if I have a process with a full set of capabilities in current->mm->user_ns will the inode be readable. I can see an argument for calling prepare_creds stuffing the new cred full of capabilities. Calling override_cred. Calling inode_permission, restoring the credentials. But it seems very much like overkill and more error prone because of the more code involved. So I have done the simple thing that doesn't hide what is really going on. Eric -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org