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

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

[Dwight Engen]

>From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
From: Dwight Engen <dwight.engen@oracle.com>
Date: Tue, 2 Jul 2013 09:52:54 -0400
Subject: [PATCH 7/7] enable building user namespace with xfs

Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
---
 init/Kconfig | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 247084b..a7bcd87 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1106,7 +1106,6 @@ config IPC_NS
 
 config USER_NS
 	bool "User namespace"
-	depends on UIDGID_CONVERTED
 	select UIDGID_STRICT_TYPE_CHECKS
 
 	default n
@@ -1140,20 +1139,8 @@ config NET_NS
 
 endif # NAMESPACES
 
-config UIDGID_CONVERTED
-	# True if all of the selected software conmponents are known
-	# to have uid_t and gid_t converted to kuid_t and kgid_t
-	# where appropriate and are otherwise safe to use with
-	# the user namespace.
-	bool
-	default y
-
-	# Filesystems
-	depends on XFS_FS = n
-
 config UIDGID_STRICT_TYPE_CHECKS
 	bool "Require conversions between uid/gids and their internal representation"
-	depends on UIDGID_CONVERTED
 	default n
 	help
 	 While the nececessary conversions are being added to all subsystems this option allows
-- 
1.8.1.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Ben Myers]

Hey Dwight,

On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> From: Dwight Engen <dwight.engen@oracle.com>
> Date: Tue, 2 Jul 2013 09:52:54 -0400
> Subject: [PATCH 7/7] enable building user namespace with xfs
> 
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>

Was there a patch running around to limit bulkstat to init_user_ns?  Any other
items that needed to be addressed before applying this patch?

Thanks,
	Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dave Chinner]

On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> Hey Dwight,
> 
> On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > From: Dwight Engen <dwight.engen@oracle.com>
> > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > Subject: [PATCH 7/7] enable building user namespace with xfs
> > 
> > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> 
> Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> items that needed to be addressed before applying this patch?

Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
executed in the init name space. Similarly, all the open-by-handle
interfaces have the same capable() checks so they can only be
executed int he init name space, too.

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.

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...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Gao feng]

On 07/30/2013 11:07 AM, Dwight Engen wrote:
>>From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> From: Dwight Engen <dwight.engen@oracle.com>
> Date: Tue, 2 Jul 2013 09:52:54 -0400
> Subject: [PATCH 7/7] enable building user namespace with xfs
> 
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> ---

looks good to me, thanks :)

Reviewed-by: Gao feng <gaofeng@cn.fujitsu.com>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Ben Myers]

Hey,

On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > From: Dwight Engen <dwight.engen@oracle.com>
> > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > 
> > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > 
> > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > items that needed to be addressed before applying this patch?
> 
> Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> executed in the init name space. Similarly, all the open-by-handle
> interfaces have the same capable() checks so they can only be
> executed int he init name space, too.

Gah.  I was under the impression that you could have a process with
CAP_SYS_ADMIN in a namespace other than init_user_ns.

> 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.
> 
> 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...

Looks like that should be added to patch 3.  Dwight, if you prefer to repost
only patch 3 that's fine.

-Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dwight Engen]

On Wed, 31 Jul 2013 08:25:23 -0500
Ben Myers <bpm@sgi.com> wrote:

> Hey,
> 
> On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17
> > > > >00:00:00 2001
> > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > 
> > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > 
> > > Was there a patch running around to limit bulkstat to
> > > init_user_ns?  Any other items that needed to be addressed before
> > > applying this patch?
> > 
> > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only
> > be executed in the init name space. Similarly, all the
> > open-by-handle interfaces have the same capable() checks so they
> > can only be executed int he init name space, too.
> 
> Gah.  I was under the impression that you could have a process with
> CAP_SYS_ADMIN in a namespace other than init_user_ns.

Hi Ben, a process can be CAP_SYS_ADMIN in its own userns, but the
check for that would be ns_capable(CAP_SYS_ADMIN) not
capable(CAP_SYS_ADMIN) which checks against init_user_ns. The checks
Dave was mentioning are all against init_user_ns so bulkstat
and open-by-handle effectively cannot be used from inside a
non-init_user_ns.

> > 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.
> > 
> > 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...
>
> Looks like that should be added to patch 3.  Dwight, if you prefer to
> repost only patch 3 that's fine.

I'll add this into patch 3 once we get it sorted out and then repost
with Gao's reviewed-by tags. Thanks.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dwight Engen]

On Wed, 31 Jul 2013 10:21:19 +1000
Dave Chinner <david@fromorbit.com> 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)

> Cheers,
> 
> Dave.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dave Chinner]

On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> Hey,
> 
> On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > 
> > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > 
> > > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > > items that needed to be addressed before applying this patch?
> > 
> > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> > executed in the init name space. Similarly, all the open-by-handle
> > interfaces have the same capable() checks so they can only be
> > executed int he init name space, too.
> 
> Gah.  I was under the impression that you could have a process with
> CAP_SYS_ADMIN in a namespace other than init_user_ns.

Ben, until about a week and a half ago I was also working under that
same understanding as you.  So don't feel bad about not knowing
about this basic, fundamental rule because it is completely
undocumented and it's not obvious to anyone reading the code until
someone points it out....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dave Chinner]

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@fromorbit.com> 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.

Right.

> 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.

Exactly.

> > 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....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Gao feng]

On 08/01/2013 07:43 AM, Dave Chinner wrote:
> 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@fromorbit.com> 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.
> 
> Right.
> 
>> 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.
> 
> Exactly.
> 
>>> 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....


Yes, in the implementation of userns, project ids can be modified and
project quota is allowed to setup in container safely, but compared
with removing xfs dependence, I hope this dependence to be removed firstly.

the mapping project IDs to user namespace work should be fixed after
we finish this removing dependence work. :)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Ben Myers]

Dave,

On Thu, Aug 01, 2013 at 09:28:52AM +1000, Dave Chinner wrote:
> On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > Hey,
> > 
> > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > 
> > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > 
> > > > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > > > items that needed to be addressed before applying this patch?
> > > 
> > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> > > executed in the init name space. Similarly, all the open-by-handle
> > > interfaces have the same capable() checks so they can only be
> > > executed int he init name space, too.
> > 
> > Gah.  I was under the impression that you could have a process with
> > CAP_SYS_ADMIN in a namespace other than init_user_ns.
> 
> Ben, until about a week and a half ago I was also working under that
> same understanding as you.

Well huh.  According to Dwight you can have a process with CAP_SYS_ADMIN in a
namespace other than init_user_ns.  Kinda cool, IMO.

-Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dwight Engen]

On Thu, 1 Aug 2013 10:06:43 -0500
Ben Myers <bpm@sgi.com> wrote:

> Dave,
> 
> On Thu, Aug 01, 2013 at 09:28:52AM +1000, Dave Chinner wrote:
> > On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > > Hey,
> > > 
> > > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17
> > > > > > >00:00:00 2001
> > > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > > 
> > > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > > 
> > > > > Was there a patch running around to limit bulkstat to
> > > > > init_user_ns?  Any other items that needed to be addressed
> > > > > before applying this patch?
> > > > 
> > > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can
> > > > only be executed in the init name space. Similarly, all the
> > > > open-by-handle interfaces have the same capable() checks so
> > > > they can only be executed int he init name space, too.
> > > 
> > > Gah.  I was under the impression that you could have a process
> > > with CAP_SYS_ADMIN in a namespace other than init_user_ns.
> > 
> > Ben, until about a week and a half ago I was also working under that
> > same understanding as you.
> 
> Well huh.  According to Dwight you can have a process with
> CAP_SYS_ADMIN in a namespace other than init_user_ns.  Kinda cool,
> IMO.

Yeah, see commit 3486740a and cap_capable(). Took me a bit to grok
that for loop, but I think the semantics are that if you are in a non
init_user_ns, you can start from current_user_ns() looking for a cap
and walk back the heirarchy (userns's can be nested) but it won't ever
check in init_user_ns, and so by definition you have no capabilities
in init_user_ns.

ie. if you are in a userns, you might be nsown_capable(CAP_SYS_ADMIN)
but you cannot ever be capable(CAP_SYS_ADMIN). Thats my understanding
anyway :)

> -Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Serge E. Hallyn]

Quoting Dwight Engen (dwight.engen@oracle.com):
> On Thu, 1 Aug 2013 10:06:43 -0500
> Ben Myers <bpm@sgi.com> wrote:
> 
> > Dave,
> > 
> > On Thu, Aug 01, 2013 at 09:28:52AM +1000, Dave Chinner wrote:
> > > On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > > > Hey,
> > > > 
> > > > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17
> > > > > > > >00:00:00 2001
> > > > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > > > 
> > > > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > > > 
> > > > > > Was there a patch running around to limit bulkstat to
> > > > > > init_user_ns?  Any other items that needed to be addressed
> > > > > > before applying this patch?
> > > > > 
> > > > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can
> > > > > only be executed in the init name space. Similarly, all the
> > > > > open-by-handle interfaces have the same capable() checks so
> > > > > they can only be executed int he init name space, too.
> > > > 
> > > > Gah.  I was under the impression that you could have a process
> > > > with CAP_SYS_ADMIN in a namespace other than init_user_ns.
> > > 
> > > Ben, until about a week and a half ago I was also working under that
> > > same understanding as you.
> > 
> > Well huh.  According to Dwight you can have a process with
> > CAP_SYS_ADMIN in a namespace other than init_user_ns.  Kinda cool,
> > IMO.
> 
> Yeah, see commit 3486740a and cap_capable(). Took me a bit to grok
> that for loop, but I think the semantics are that if you are in a non
> init_user_ns, you can start from current_user_ns() looking for a cap

To be precise, you're not looking for a cap in the parent ns.  You're
checking whether the subject is the creator of one of the object's
namespace's parents.

> and walk back the heirarchy (userns's can be nested) but it won't ever
> check in init_user_ns, and so by definition you have no capabilities
> in init_user_ns.

You also have no caps in any of your parent namespaces.  The loop is
checking over the *object*'s parents to see if the subject owns them.
I'ts not looping over the *subject*'s parents, as they do not grant
any privilege.  Only the subject's bottom-most credentials grant
privilege.

> ie. if you are in a userns, you might be nsown_capable(CAP_SYS_ADMIN)
> but you cannot ever be capable(CAP_SYS_ADMIN). Thats my understanding
> anyway :)

Yup.

-serge

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Serge E. Hallyn]

Quoting Dave Chinner (david@fromorbit.com):
> On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > Hey,
> > 
> > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > 
> > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > 
> > > > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > > > items that needed to be addressed before applying this patch?
> > > 
> > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> > > executed in the init name space. Similarly, all the open-by-handle
> > > interfaces have the same capable() checks so they can only be
> > > executed int he init name space, too.
> > 
> > Gah.  I was under the impression that you could have a process with
> > CAP_SYS_ADMIN in a namespace other than init_user_ns.
> 
> Ben, until about a week and a half ago I was also working under that
> same understanding as you.  So don't feel bad about not knowing
> about this basic, fundamental rule because it is completely
> undocumented and it's not obvious to anyone reading the code until
> someone points it out....

It's actually all documented in new manpages like namespaces(7) and
user_namespaces(7).  Unfortunately those don't seem to have been released yet.

Michael, I don't see any git development/experimental git branch at
kernel.org/pub/scm/docs/man-pages - do you have those pages up
somewhere?

thanks,
-serge

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Serge E. Hallyn]

Gah, forgot to actually add Michael to the cc:, sorry.

Quoting Serge E. Hallyn (serge@hallyn.com):
> Quoting Dave Chinner (david@fromorbit.com):
> > On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > > Hey,
> > > 
> > > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > > 
> > > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > > 
> > > > > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > > > > items that needed to be addressed before applying this patch?
> > > > 
> > > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> > > > executed in the init name space. Similarly, all the open-by-handle
> > > > interfaces have the same capable() checks so they can only be
> > > > executed int he init name space, too.
> > > 
> > > Gah.  I was under the impression that you could have a process with
> > > CAP_SYS_ADMIN in a namespace other than init_user_ns.
> > 
> > Ben, until about a week and a half ago I was also working under that
> > same understanding as you.  So don't feel bad about not knowing
> > about this basic, fundamental rule because it is completely
> > undocumented and it's not obvious to anyone reading the code until
> > someone points it out....
> 
> It's actually all documented in new manpages like namespaces(7) and
> user_namespaces(7).  Unfortunately those don't seem to have been released yet.
> 
> Michael, I don't see any git development/experimental git branch at
> kernel.org/pub/scm/docs/man-pages - do you have those pages up
> somewhere?
> 
> thanks,
> -serge
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

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

[Dave Chinner]

On Wed, Aug 07, 2013 at 02:59:30PM +0000, Serge E. Hallyn wrote:
> Quoting Dave Chinner (david@fromorbit.com):
> > On Wed, Jul 31, 2013 at 08:25:23AM -0500, Ben Myers wrote:
> > > Hey,
> > > 
> > > On Wed, Jul 31, 2013 at 10:21:19AM +1000, Dave Chinner wrote:
> > > > On Tue, Jul 30, 2013 at 06:40:21PM -0500, Ben Myers wrote:
> > > > > On Mon, Jul 29, 2013 at 11:07:09PM -0400, Dwight Engen wrote:
> > > > > > >From e6a9ee0cfa0ed40484f66bc1726dc19de36038b8 Mon Sep 17 00:00:00 2001
> > > > > > From: Dwight Engen <dwight.engen@oracle.com>
> > > > > > Date: Tue, 2 Jul 2013 09:52:54 -0400
> > > > > > Subject: [PATCH 7/7] enable building user namespace with xfs
> > > > > > 
> > > > > > Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> > > > > 
> > > > > Was there a patch running around to limit bulkstat to init_user_ns?  Any other
> > > > > items that needed to be addressed before applying this patch?
> > > > 
> > > > Bulkstat has a capable(CAP_SYS_ADMIN) check and therefore can only be
> > > > executed in the init name space. Similarly, all the open-by-handle
> > > > interfaces have the same capable() checks so they can only be
> > > > executed int he init name space, too.
> > > 
> > > Gah.  I was under the impression that you could have a process with
> > > CAP_SYS_ADMIN in a namespace other than init_user_ns.
> > 
> > Ben, until about a week and a half ago I was also working under that
> > same understanding as you.  So don't feel bad about not knowing
> > about this basic, fundamental rule because it is completely
> > undocumented and it's not obvious to anyone reading the code until
> > someone points it out....
> 
> It's actually all documented in new manpages like namespaces(7) and
> user_namespaces(7).  Unfortunately those don't seem to have been released yet.

User facing documentation goes in man pages.

My comments about the above point at the fact that there is no
developer facing documentation that tell us how to safely and
*securely* implement namespace support in different filesystems.
Information on the architecture, design and use of internal kernel
infrastructure for kernel developers should be in the Documentation/
subdirectory of the kernel tree.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs