From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io0-f198.google.com (mail-io0-f198.google.com [209.85.223.198]) by kanga.kvack.org (Postfix) with ESMTP id 8173A6B0069 for ; Wed, 19 Oct 2016 13:07:44 -0400 (EDT) Received: by mail-io0-f198.google.com with SMTP id o141so45727663ioe.4 for ; Wed, 19 Oct 2016 10:07:44 -0700 (PDT) Received: from out01.mta.xmission.com (out01.mta.xmission.com. [166.70.13.231]) by mx.google.com with ESMTPS id qz8si34503284pab.25.2016.10.19.10.07.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 19 Oct 2016 10:07:43 -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> <87mvi0mpix.fsf@xmission.com> Date: Wed, 19 Oct 2016 12:04:45 -0500 In-Reply-To: <87mvi0mpix.fsf@xmission.com> (Eric W. Biederman's message of "Wed, 19 Oct 2016 08:33:58 -0500") Message-ID: <87lgxkjmmq.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: linux-kernel , Linux Containers , Oleg Nesterov , Andy Lutomirski , linux-mm@kvack.org, linux-fsdevel , Michal Hocko ebiederm@xmission.com (Eric W. Biederman) writes: > 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. At the same time I can see the addition of a helper function bool ns_inode(struct user_namespace *user_ns, struct inode *inode) { return kuid_has_mapping(user_ns, inode->i_uid) && kgid_has_mapping(user_ns, inode->i_gid); } That abstracts out the concept instead of open codes it. 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