From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v4 3/5] fuse: Restrict allow_other to uids already controlled by the user Date: Wed, 15 Oct 2014 07:58:59 -0700 Message-ID: <543E8BB3.6040701@amacapital.net> References: <1413296756-25071-1-git-send-email-seth.forshee@canonical.com> <1413296756-25071-4-git-send-email-seth.forshee@canonical.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: fuse-devel@lists.sourceforge.net, "Serge H. Hallyn" , LKML , "Eric W. Biederman" , linux-fsdevel To: Seth Forshee , Miklos Szeredi Return-path: In-Reply-To: <1413296756-25071-4-git-send-email-seth.forshee@canonical.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On 10/14/2014 07:25 AM, Seth Forshee wrote: > Unprivileged users are normally restricted from mounting with the > allow_other option by system policy, but this could be bypassed > for a mount done with user namespace root permissions. In such > cases allow_other should not allow users outside the user > namespace to access the mount as doing so would give the > unprivileged user the ability to manipulate processes it would > otherwise be unable to manipulate. What threat is this intended to protect against? I think that, if this is needed, tasks outside the userns or its descendents should be blocked, even if the user ids match. That is, I think you should check the namespace, not the uid and gid. --Andy > > Cc: Eric W. Biederman > Cc: Serge H. Hallyn > Signed-off-by: Seth Forshee > --- > fs/fuse/dir.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c > index 123db1e06c78..e3123bfbc711 100644 > --- a/fs/fuse/dir.c > +++ b/fs/fuse/dir.c > @@ -1089,12 +1089,20 @@ int fuse_reverse_inval_entry(struct super_block *sb, u64 parent_nodeid, > */ > int fuse_allow_current_process(struct fuse_conn *fc) > { > - const struct cred *cred; > + const struct cred *cred = current_cred(); > > - if (fc->flags & FUSE_ALLOW_OTHER) > - return 1; > + if (fc->flags & FUSE_ALLOW_OTHER) { > + if (kuid_has_mapping(fc->user_ns, cred->euid) && > + kuid_has_mapping(fc->user_ns, cred->suid) && > + kuid_has_mapping(fc->user_ns, cred->uid) && > + kgid_has_mapping(fc->user_ns, cred->egid) && > + kgid_has_mapping(fc->user_ns, cred->sgid) && > + kgid_has_mapping(fc->user_ns, cred->gid)) > + return 1; > + > + return 0; > + } > > - cred = current_cred(); > if (uid_eq(cred->euid, fc->user_id) && > uid_eq(cred->suid, fc->user_id) && > uid_eq(cred->uid, fc->user_id) && >