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

Open Source and information security mailing list archives

Message-ID: <20180719143616.GA29715@mailbox.org>
Date:   Thu, 19 Jul 2018 16:36:16 +0200
From:   Christian Brauner <christian@...uner.io>
To:     Tyler Hicks <tyhicks@...onical.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Tejun Heo <tj@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Stephen Hemminger <stephen@...workplumber.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        netdev@...r.kernel.org,
        Linux Containers <containers@...ts.linux-foundation.org>,
        bridge@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        "Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [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@...onical.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@...ts.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/containers

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives