[Top] [All Lists]

Re: [PATCH v7 7/7] enable building user namespace with xfs

To: Dwight Engen <dwight.engen@xxxxxxxxxx>
Subject: Re: [PATCH v7 7/7] enable building user namespace with xfs
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 1 Aug 2013 09:43:57 +1000
Cc: Gao feng <gaofeng@xxxxxxxxxxxxxx>, Ben Myers <bpm@xxxxxxx>, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130731141931.2e1c77d4@xxxxxxxxxx>
References: <20130729230709.11033b2d@xxxxxxxxxx> <20130730234021.GR3111@xxxxxxx> <20130731002119.GR13468@dastard> <20130731141931.2e1c77d4@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Jul 31, 2013 at 02:19:31PM -0400, Dwight Engen wrote:
> On Wed, 31 Jul 2013 10:21:19 +1000
> Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> [...]
> > The only thing I think we still need to address is whether
> > xfs_ioc_setattr() should allow users within a namespace to change
> > the project ID of a file they otherwise own. That function is
> > currently changed to use a inode_owner_or_capable() check and so if
> > the uids match inside the namespace the modification is allowed.
> Right, so before this change the caller has to own the file or be
> CAP_FOWNER in init_user_ns. Changing to using inode_owner_or_capable()
> means the caller has to own the file (and the inode's uid must be valid
> in current_user_ns) or be CAP_FOWNER in current_user_ns. I don't
> see how this lets the user the user do something inside the userns that
> they can't do outside.


> Basically I think we want to treat projids as a non namespace aware
> identifier, but it sounds like maybe we don't want them manipulated
> from userns context at all.


> > However, right now for project IDs I think we have decided to limit
> > manipulations to the init user namespace and not expose project IDs
> > inside user namespaces at all. Hence I think that xfs_ioc_setattr()
> > needs a further check for this...
> If we don't want to allow setting the projid from a userns, the check
> I would propose inside the if (mask & FSX_EXTSIZE) block is:
>   if (current_user_ns() != &init_user_ns)
> Is the appropriate error return for this EINVAL? (thats what a similar
> check in kernel/taskstats.c returns)

Yeah, it looks like the very few cases where an error is returned
for such a check it is an EINVAL case. Don't forget a comment
explaining this.

We probably also fix getxstate/getxquota implementations to reject
project quota requests as well. It might be best to do this with an
additional patch.

As it is, we're going to need to revisit the project quota support
in fs/quota/quota.c because that maps project IDs to/from user
namespaces. This was done based on the overriding assertion of the
userns implementor that project quota IDs must be mapped via
namespaces, but as we are getting down to it the details (i.e.
project IDs only visible in the init user ns) we can see
that this is incorrect and we're going to need to do quite a bit of
surgery there to fix this up....


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>