| Message ID | 1531497949-1766-7-git-send-email-tyhicks@canonical.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | Make /sys/class/net per net namespace objects belong to container | expand |
On Fri, Jul 13, 2018 at 04:05:48PM +0000, Tyler Hicks wrote: > Make net_ns_get_ownership() reusable by networking code outside of core. > This is useful, for example, to allow bridge related sysfs files to be > owned by container root. > > Add a function comment since this is a potentially dangerous function to > use given the way that kobject_get_ownership() works by initializing uid > and gid before calling .get_ownership(). > > Signed-off-by: Tyler Hicks <tyhicks@canonical.com> > --- > include/net/net_namespace.h | 7 +++++++ > net/core/net-sysfs.c | 15 --------------- > net/core/net_namespace.c | 25 +++++++++++++++++++++++++ > 3 files changed, 32 insertions(+), 15 deletions(-) > > diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h > index a71264d75d7f..a257710527ce 100644 > --- a/include/net/net_namespace.h > +++ b/include/net/net_namespace.h > @@ -170,6 +170,8 @@ extern struct net init_net; > struct net *copy_net_ns(unsigned long flags, struct user_namespace *user_ns, > struct net *old_net); > > +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid); > + > void net_ns_barrier(void); > #else /* CONFIG_NET_NS */ > #include <linux/sched.h> > @@ -182,6 +184,11 @@ static inline struct net *copy_net_ns(unsigned long flags, > return old_net; > } > > +static inline void net_ns_get_ownership(const struct net *net, > + kuid_t *uid, kgid_t *gid) > +{ > +} > + > static inline void net_ns_barrier(void) {} > #endif /* CONFIG_NET_NS */ > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index 41d84c40fe51..a3ad8108d296 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -656,21 +656,6 @@ static const struct attribute_group wireless_group = { > #define net_class_groups NULL > #endif /* CONFIG_SYSFS */ > > -static void net_ns_get_ownership(const struct net *net, > - kuid_t *uid, kgid_t *gid) > -{ > - if (net) { > - kuid_t ns_root_uid = make_kuid(net->user_ns, 0); > - kgid_t ns_root_gid = make_kgid(net->user_ns, 0); > - > - if (uid_valid(ns_root_uid)) > - *uid = ns_root_uid; > - > - if (gid_valid(ns_root_gid)) > - *gid = ns_root_gid; > - } > -} > - > #ifdef CONFIG_SYSFS > #define to_rx_queue_attr(_attr) \ > container_of(_attr, struct rx_queue_attribute, attr) > diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c > index a11e03f920d3..5257875fa84d 100644 > --- a/net/core/net_namespace.c > +++ b/net/core/net_namespace.c > @@ -448,6 +448,31 @@ struct net *copy_net_ns(unsigned long flags, > return net; > } > > +/** > + * net_ns_get_ownership - get sysfs ownership data for @net > + * @net: network namespace in question (can be NULL) > + * @uid: kernel user ID for sysfs objects (must be GLOBAL_ROOT_UID) > + * @gid: kernel group ID for sysfs objects (must be GLOBAL_ROOT_GID) > + * > + * Returns the uid/gid pair of root in the user namespace associated with the > + * given network namespace. The caller must initialize @uid and @gid to > + * GLOBAL_ROOT_UID/GLOBAL_ROOT_GID before calling this function. If they must be so initialized why not just enforce this directly in the function? This way callers can rely on always getting back the correct permissions and the comment can be removed as well. Christian > + */ > +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid) > +{ > + if (net) { > + kuid_t ns_root_uid = make_kuid(net->user_ns, 0); > + kgid_t ns_root_gid = make_kgid(net->user_ns, 0); > + > + if (uid_valid(ns_root_uid)) > + *uid = ns_root_uid; > + > + if (gid_valid(ns_root_gid)) > + *gid = ns_root_gid; > + } > +} > +EXPORT_SYMBOL_GPL(net_ns_get_ownership); > + > static void unhash_nsid(struct net *net, struct net *last) > { > struct net *tmp; > -- > 2.7.4 > > _______________________________________________ > Containers mailing list > Containers@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/containers
On 07/19/2018 09:36 AM, Christian Brauner wrote: > On Fri, Jul 13, 2018 at 04:05:48PM +0000, Tyler Hicks wrote: >> Make net_ns_get_ownership() reusable by networking code outside of core. >> This is useful, for example, to allow bridge related sysfs files to be >> owned by container root. >> >> Add a function comment since this is a potentially dangerous function to >> use given the way that kobject_get_ownership() works by initializing uid >> and gid before calling .get_ownership(). >> >> Signed-off-by: Tyler Hicks <tyhicks@canonical.com> >> --- >> include/net/net_namespace.h | 7 +++++++ >> net/core/net-sysfs.c | 15 --------------- >> net/core/net_namespace.c | 25 +++++++++++++++++++++++++ >> 3 files changed, 32 insertions(+), 15 deletions(-) >> >> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h >> index a71264d75d7f..a257710527ce 100644 >> --- a/include/net/net_namespace.h >> +++ b/include/net/net_namespace.h >> @@ -170,6 +170,8 @@ extern struct net init_net; >> struct net *copy_net_ns(unsigned long flags, struct user_namespace *user_ns, >> struct net *old_net); >> >> +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid); >> + >> void net_ns_barrier(void); >> #else /* CONFIG_NET_NS */ >> #include <linux/sched.h> >> @@ -182,6 +184,11 @@ static inline struct net *copy_net_ns(unsigned long flags, >> return old_net; >> } >> >> +static inline void net_ns_get_ownership(const struct net *net, >> + kuid_t *uid, kgid_t *gid) >> +{ >> +} >> + >> static inline void net_ns_barrier(void) {} >> #endif /* CONFIG_NET_NS */ >> >> diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c >> index 41d84c40fe51..a3ad8108d296 100644 >> --- a/net/core/net-sysfs.c >> +++ b/net/core/net-sysfs.c >> @@ -656,21 +656,6 @@ static const struct attribute_group wireless_group = { >> #define net_class_groups NULL >> #endif /* CONFIG_SYSFS */ >> >> -static void net_ns_get_ownership(const struct net *net, >> - kuid_t *uid, kgid_t *gid) >> -{ >> - if (net) { >> - kuid_t ns_root_uid = make_kuid(net->user_ns, 0); >> - kgid_t ns_root_gid = make_kgid(net->user_ns, 0); >> - >> - if (uid_valid(ns_root_uid)) >> - *uid = ns_root_uid; >> - >> - if (gid_valid(ns_root_gid)) >> - *gid = ns_root_gid; >> - } >> -} >> - >> #ifdef CONFIG_SYSFS >> #define to_rx_queue_attr(_attr) \ >> container_of(_attr, struct rx_queue_attribute, attr) >> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c >> index a11e03f920d3..5257875fa84d 100644 >> --- a/net/core/net_namespace.c >> +++ b/net/core/net_namespace.c >> @@ -448,6 +448,31 @@ struct net *copy_net_ns(unsigned long flags, >> return net; >> } >> >> +/** >> + * net_ns_get_ownership - get sysfs ownership data for @net >> + * @net: network namespace in question (can be NULL) >> + * @uid: kernel user ID for sysfs objects (must be GLOBAL_ROOT_UID) >> + * @gid: kernel group ID for sysfs objects (must be GLOBAL_ROOT_GID) >> + * >> + * Returns the uid/gid pair of root in the user namespace associated with the >> + * given network namespace. The caller must initialize @uid and @gid to >> + * GLOBAL_ROOT_UID/GLOBAL_ROOT_GID before calling this function. > > If they must be so initialized why not just enforce this directly in the > function? This way callers can rely on always getting back the correct > permissions and the comment can be removed as well. I agree and made this change in v3 of the patch set that I just sent out. Thanks for the suggestion! Tyler > > Christian > >> + */ >> +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid) >> +{ >> + if (net) { >> + kuid_t ns_root_uid = make_kuid(net->user_ns, 0); >> + kgid_t ns_root_gid = make_kgid(net->user_ns, 0); >> + >> + if (uid_valid(ns_root_uid)) >> + *uid = ns_root_uid; >> + >> + if (gid_valid(ns_root_gid)) >> + *gid = ns_root_gid; >> + } >> +} >> +EXPORT_SYMBOL_GPL(net_ns_get_ownership); >> + >> static void unhash_nsid(struct net *net, struct net *last) >> { >> struct net *tmp; >> -- >> 2.7.4 >> >> _______________________________________________ >> Containers mailing list >> Containers@lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/containers
diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h index a71264d75d7f..a257710527ce 100644 --- a/include/net/net_namespace.h +++ b/include/net/net_namespace.h @@ -170,6 +170,8 @@ extern struct net init_net; struct net *copy_net_ns(unsigned long flags, struct user_namespace *user_ns, struct net *old_net); +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid); + void net_ns_barrier(void); #else /* CONFIG_NET_NS */ #include <linux/sched.h> @@ -182,6 +184,11 @@ static inline struct net *copy_net_ns(unsigned long flags, return old_net; } +static inline void net_ns_get_ownership(const struct net *net, + kuid_t *uid, kgid_t *gid) +{ +} + static inline void net_ns_barrier(void) {} #endif /* CONFIG_NET_NS */ diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index 41d84c40fe51..a3ad8108d296 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -656,21 +656,6 @@ static const struct attribute_group wireless_group = { #define net_class_groups NULL #endif /* CONFIG_SYSFS */ -static void net_ns_get_ownership(const struct net *net, - kuid_t *uid, kgid_t *gid) -{ - if (net) { - kuid_t ns_root_uid = make_kuid(net->user_ns, 0); - kgid_t ns_root_gid = make_kgid(net->user_ns, 0); - - if (uid_valid(ns_root_uid)) - *uid = ns_root_uid; - - if (gid_valid(ns_root_gid)) - *gid = ns_root_gid; - } -} - #ifdef CONFIG_SYSFS #define to_rx_queue_attr(_attr) \ container_of(_attr, struct rx_queue_attribute, attr) diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index a11e03f920d3..5257875fa84d 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -448,6 +448,31 @@ struct net *copy_net_ns(unsigned long flags, return net; } +/** + * net_ns_get_ownership - get sysfs ownership data for @net + * @net: network namespace in question (can be NULL) + * @uid: kernel user ID for sysfs objects (must be GLOBAL_ROOT_UID) + * @gid: kernel group ID for sysfs objects (must be GLOBAL_ROOT_GID) + * + * Returns the uid/gid pair of root in the user namespace associated with the + * given network namespace. The caller must initialize @uid and @gid to + * GLOBAL_ROOT_UID/GLOBAL_ROOT_GID before calling this function. + */ +void net_ns_get_ownership(const struct net *net, kuid_t *uid, kgid_t *gid) +{ + if (net) { + kuid_t ns_root_uid = make_kuid(net->user_ns, 0); + kgid_t ns_root_gid = make_kgid(net->user_ns, 0); + + if (uid_valid(ns_root_uid)) + *uid = ns_root_uid; + + if (gid_valid(ns_root_gid)) + *gid = ns_root_gid; + } +} +EXPORT_SYMBOL_GPL(net_ns_get_ownership); + static void unhash_nsid(struct net *net, struct net *last) { struct net *tmp;
Make net_ns_get_ownership() reusable by networking code outside of core. This is useful, for example, to allow bridge related sysfs files to be owned by container root. Add a function comment since this is a potentially dangerous function to use given the way that kobject_get_ownership() works by initializing uid and gid before calling .get_ownership(). Signed-off-by: Tyler Hicks <tyhicks@canonical.com> --- include/net/net_namespace.h | 7 +++++++ net/core/net-sysfs.c | 15 --------------- net/core/net_namespace.c | 25 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 15 deletions(-)