Re: [PATCH 1/2] fs: add inode helpers for fsuid and fsgid

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Wed, 2017-02-15 at 15:29 +1300, Eric W. Biederman wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> 
> > On Tue, 2017-02-14 at 20:46 +1300, Eric W. Biederman wrote:
> > > James Bottomley <James.Bottomley@HansenPartnership.com> writes:
> > > 
> > > > Now that we have two different views of filesystem ids (the 
> > > > filesystem view and the kernel view), we have a problem in that
> > > > current_fsuid/fsgid() return the kernel view but are sometimes
> > > > used 
> > > > in filesystem code where the filesystem view shoud be used. 
> > > >  This
> > > > patch introduces helpers to produce the filesystem view of
> > > > current 
> > > > fsuid and fsgid.
> > > 
> > > If I am reading this right what we are seeing is that xfs
> > > explicitly
> > > opted out of type safety with predictable results.   Accidentally
> > > confusing kuids and uids, which is potentially security issue.
> > > 
> > > All of that said where are you getting sb->s_user_ns !=
> > > &init_user_ns
> > > for an xfs filesystem?
> 
> James please answer this question:
> 
>  Where are you getting sb->s_user_ns != &init_user_ns for an xfs
> filesystem?

I'm playing with a patch that allows host admin to set up an
unprivileged container for a guest to use.  One of the extensions is to
allow anything possessing capability(CAP_SYS_ADMIN) to make s_user_ns
follow mnt_ns->user_ns for new mounts (as an option).  The idea was to
see if root could set up an id shifted container with just the current
s_user_ns infrastructure.

> None of this matters if sb->s_user_ns == &init_user_ns.
> 
> This is signification because only xfs keeps any in-core data 
> structure in it's on-disk encoding.  So this problem is xfs specific.
>    So understanding how you are getting xfs to have sb->s_user_ns !=
> &init_user_ns is important for discussing which direction we go with
> helper functions here.
> 
> xfs with sb->s_user_ns == &init_user_ns is perfectly fine and as such 
> no fixes are needed.

So what you're saying is that unless the unprivileged container could
mount the filesystem itself (i.e. only those possessing the
FS_USERNS_MOUNT flag) the filesystems are going to be full of problems
like this.  I suppose whether it's worthwhile trying to fix them all
depends on whether the ability of the administrator to set up an id
shifted container is useful or not.

James