Re: [PATCH v3 2/3] fs: introduce uid/gid shifting bind mount

From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	containers@lists.linux-foundation.org,
	linux-unionfs@vger.kernel.org,
	David Howells <dhowells@redhat.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH v3 2/3] fs: introduce uid/gid shifting bind mount
Date: Tue, 18 Feb 2020 17:19:17 -0800	[thread overview]
Message-ID: <1582075157.31675.0.camel@HansenPartnership.com> (raw)
In-Reply-To: <20200218223313.GA15846@infradead.org>

On Tue, 2020-02-18 at 14:33 -0800, Christoph Hellwig wrote:
> On Mon, Feb 17, 2020 at 12:53:06PM -0800, James Bottomley wrote:

[...]
> > diff --git a/include/linux/cred.h b/include/linux/cred.h
> > index 18639c069263..d29638617844 100644
> > --- a/include/linux/cred.h
> > +++ b/include/linux/cred.h
> > @@ -59,6 +59,7 @@ extern struct group_info *groups_alloc(int);
> >  extern void groups_free(struct group_info *);
> >  
> >  extern int in_group_p(kgid_t);
> > +extern int in_group_p_shifted(kgid_t);
> 
> How do I know when to use in_group_p_shifted vs in_group_p?
> What about the various other fs callers?

So this is one I wondered about too.  The problem is that the shifted
credential (the one representing the fsuid/fsgid the filesystem will
see) still has cred->group_info representing the kuid/kgid which are
unshifted from the filesystem perspective.  The solution was to use
in_group_p_shifted when you're comparing a filesystem view fsgid and
use in_group_p when you're comparing a kernel kgid.

However, I'm now thinking that's way too complex and what should happen
is that I should shift every member of cred->group_info so that all
searches happen on the fs view, meaning the fs always uses in_group_p
like it does today and only the corner cases that compare a kgid need
shifting.

James