thr3ads.net - Linux Ethernet Bridging - [Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes [Jul 2018]

If this information is useful, please help other people find it:
Share via:

Tyler Hicks

2018-Jul-13 16:05 UTC

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

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 at 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.
+ */
+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

Christian Brauner

2018-Jul-19 14:36 UTC

head link

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

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 at 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 at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20180719/3080ea61/attachment.sig>

Tyler Hicks

2018-Jul-20 21:58 UTC

head link

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

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 at 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 at lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/containers

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/bridge/attachments/20180720/86058bd3/attachment.sig>

Linux Ethernet Bridging - Jul 2018 - [Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes

[Bridge] [PATCH net-next v2 6/7] net: Create reusable function for getting ownership info of sysfs inodes