* [PATCH v7 7/7] enable building user namespace with xfs @ 2013-07-30 3:07 Dwight Engen 2013-07-30 23:40 ` Ben Myers 2013-07-31 7:20 ` Gao feng 0 siblings, 2 replies; 16+ messages in thread From: Dwight Engen @ 2013-07-30 3:07 UTC (permalink / raw) To: xfs >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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-30 3:07 [PATCH v7 7/7] enable building user namespace with xfs Dwight Engen @ 2013-07-30 23:40 ` Ben Myers 2013-07-31 0:21 ` Dave Chinner 2013-07-31 7:20 ` Gao feng 1 sibling, 1 reply; 16+ messages in thread From: Ben Myers @ 2013-07-30 23:40 UTC (permalink / raw) To: Dwight Engen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-30 23:40 ` Ben Myers @ 2013-07-31 0:21 ` Dave Chinner 2013-07-31 13:25 ` Ben Myers 2013-07-31 18:19 ` Dwight Engen 0 siblings, 2 replies; 16+ messages in thread From: Dave Chinner @ 2013-07-31 0:21 UTC (permalink / raw) To: Ben Myers; +Cc: Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 0:21 ` Dave Chinner @ 2013-07-31 13:25 ` Ben Myers 2013-07-31 17:09 ` Dwight Engen 2013-07-31 23:28 ` Dave Chinner 2013-07-31 18:19 ` Dwight Engen 1 sibling, 2 replies; 16+ messages in thread From: Ben Myers @ 2013-07-31 13:25 UTC (permalink / raw) To: Dwight Engen, Dave Chinner; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 13:25 ` Ben Myers @ 2013-07-31 17:09 ` Dwight Engen 2013-07-31 23:28 ` Dave Chinner 1 sibling, 0 replies; 16+ messages in thread From: Dwight Engen @ 2013-07-31 17:09 UTC (permalink / raw) To: Ben Myers; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 13:25 ` Ben Myers 2013-07-31 17:09 ` Dwight Engen @ 2013-07-31 23:28 ` Dave Chinner 2013-08-01 15:06 ` Ben Myers 2013-08-07 14:59 ` Serge E. Hallyn 1 sibling, 2 replies; 16+ messages in thread From: Dave Chinner @ 2013-07-31 23:28 UTC (permalink / raw) To: Ben Myers; +Cc: Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 23:28 ` Dave Chinner @ 2013-08-01 15:06 ` Ben Myers 2013-08-01 16:17 ` Dwight Engen 2013-08-07 14:59 ` Serge E. Hallyn 1 sibling, 1 reply; 16+ messages in thread From: Ben Myers @ 2013-08-01 15:06 UTC (permalink / raw) To: Dave Chinner; +Cc: Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-08-01 15:06 ` Ben Myers @ 2013-08-01 16:17 ` Dwight Engen 2013-08-06 15:11 ` Serge E. Hallyn 0 siblings, 1 reply; 16+ messages in thread From: Dwight Engen @ 2013-08-01 16:17 UTC (permalink / raw) To: Ben Myers; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-08-01 16:17 ` Dwight Engen @ 2013-08-06 15:11 ` Serge E. Hallyn 0 siblings, 0 replies; 16+ messages in thread From: Serge E. Hallyn @ 2013-08-06 15:11 UTC (permalink / raw) To: Dwight Engen; +Cc: Ben Myers, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 23:28 ` Dave Chinner 2013-08-01 15:06 ` Ben Myers @ 2013-08-07 14:59 ` Serge E. Hallyn 2013-08-07 15:01 ` Serge E. Hallyn 2013-08-11 23:57 ` ***** SUSPECTED SPAM ***** " Dave Chinner 1 sibling, 2 replies; 16+ messages in thread From: Serge E. Hallyn @ 2013-08-07 14:59 UTC (permalink / raw) To: Dave Chinner; +Cc: Ben Myers, Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-08-07 14:59 ` Serge E. Hallyn @ 2013-08-07 15:01 ` Serge E. Hallyn 2013-08-11 23:57 ` ***** SUSPECTED SPAM ***** " Dave Chinner 1 sibling, 0 replies; 16+ messages in thread From: Serge E. Hallyn @ 2013-08-07 15:01 UTC (permalink / raw) To: Serge E. Hallyn; +Cc: Dwight Engen, Ben Myers, Michael Kerrisk (man-pages), xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* ***** SUSPECTED SPAM ***** Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-08-07 14:59 ` Serge E. Hallyn 2013-08-07 15:01 ` Serge E. Hallyn @ 2013-08-11 23:57 ` Dave Chinner 1 sibling, 0 replies; 16+ messages in thread From: Dave Chinner @ 2013-08-11 23:57 UTC (permalink / raw) To: Serge E. Hallyn; +Cc: Ben Myers, Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 0:21 ` Dave Chinner 2013-07-31 13:25 ` Ben Myers @ 2013-07-31 18:19 ` Dwight Engen 2013-07-31 23:43 ` Dave Chinner 1 sibling, 1 reply; 16+ messages in thread From: Dwight Engen @ 2013-07-31 18:19 UTC (permalink / raw) To: Dave Chinner, Gao feng; +Cc: Ben Myers, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 18:19 ` Dwight Engen @ 2013-07-31 23:43 ` Dave Chinner 2013-08-01 0:54 ` Gao feng 0 siblings, 1 reply; 16+ messages in thread From: Dave Chinner @ 2013-07-31 23:43 UTC (permalink / raw) To: Dwight Engen; +Cc: Ben Myers, Gao feng, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-31 23:43 ` Dave Chinner @ 2013-08-01 0:54 ` Gao feng 0 siblings, 0 replies; 16+ messages in thread From: Gao feng @ 2013-08-01 0:54 UTC (permalink / raw) To: Dave Chinner; +Cc: Ben Myers, Dwight Engen, xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 7/7] enable building user namespace with xfs 2013-07-30 3:07 [PATCH v7 7/7] enable building user namespace with xfs Dwight Engen 2013-07-30 23:40 ` Ben Myers @ 2013-07-31 7:20 ` Gao feng 1 sibling, 0 replies; 16+ messages in thread From: Gao feng @ 2013-07-31 7:20 UTC (permalink / raw) To: Dwight Engen; +Cc: xfs 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-08-11 23:57 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-30 3:07 [PATCH v7 7/7] enable building user namespace with xfs Dwight Engen 2013-07-30 23:40 ` Ben Myers 2013-07-31 0:21 ` Dave Chinner 2013-07-31 13:25 ` Ben Myers 2013-07-31 17:09 ` Dwight Engen 2013-07-31 23:28 ` Dave Chinner 2013-08-01 15:06 ` Ben Myers 2013-08-01 16:17 ` Dwight Engen 2013-08-06 15:11 ` Serge E. Hallyn 2013-08-07 14:59 ` Serge E. Hallyn 2013-08-07 15:01 ` Serge E. Hallyn 2013-08-11 23:57 ` ***** SUSPECTED SPAM ***** " Dave Chinner 2013-07-31 18:19 ` Dwight Engen 2013-07-31 23:43 ` Dave Chinner 2013-08-01 0:54 ` Gao feng 2013-07-31 7:20 ` Gao feng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox