All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
@ 2016-08-16 22:33 Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid Dmitry Torokhov
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

There are objects in /sys hierarchy (/sys/class/net/) that logically belong
to a namespace/container. Unfortunately all sysfs objects start their life
belonging to global root, and while we could change ownership manually,
keeping tracks of all objects that come and go is cumbersome. It would
be better if kernel created them using correct uid/gid from the beginning.

This series changes kernfs to allow creating object's with arbitrary
uid/gid, adds get_ownership() callback to ktype structure so subsystems
could supply their own logic (likely tied to namespace support) for
determining ownership of kobjects, and adjusts sysfs code to make use of
this information. Lastly net-sysfs is adjusted to make sure that objects in
net namespace are owned by the root user from the owning user namespace.

Note that we do not adjust ownership of objects moved into a new namespace
(as when moving a network device into a container) as userspace can easily
do it.

Thanks!

Dmitry Torokhov (5):
  kernfs: allow creating kernfs objects with arbitrary uid/gid
  sysfs, kobject: allow creating kobject belonging to arbitrary users
  kobject: kset_create_and_add() - fetch ownership info from parent
  driver core: set up ownership of class devices in sysfs
  net-sysfs: make sure objects belong to contrainer's owner

 drivers/base/core.c         |  9 +++++++++
 fs/kernfs/dir.c             | 29 ++++++++++++++++++++++++++---
 fs/kernfs/file.c            |  8 ++++++--
 fs/kernfs/inode.c           |  2 +-
 fs/kernfs/kernfs-internal.h |  2 ++
 fs/kernfs/symlink.c         | 11 ++++++++++-
 fs/sysfs/dir.c              |  7 ++++++-
 fs/sysfs/file.c             | 33 +++++++++++++++++++++------------
 fs/sysfs/group.c            | 23 +++++++++++++++++------
 fs/sysfs/sysfs.h            |  3 ++-
 include/linux/device.h      |  5 +++++
 include/linux/kernfs.h      | 28 +++++++++++++++++++---------
 include/linux/kobject.h     |  4 ++++
 kernel/cgroup.c             |  4 +++-
 lib/kobject.c               | 28 +++++++++++++++++++++++++++-
 net/core/net-sysfs.c        | 44 +++++++++++++++++++++++++++++++++++++++++++-
 16 files changed, 201 insertions(+), 39 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
@ 2016-08-16 22:33 ` Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 2/5] sysfs, kobject: allow creating kobject belonging to arbitrary users Dmitry Torokhov
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

This change allows creating kernfs files and directories with arbitrary
uid/gid instead of always using GLOBAL_ROOT_UID/GID by extending
kernfs_create_dir_ns() and kernfs_create_file_ns() with uid/gid arguments.
The "simple" kernfs_create_file() and kernfs_create_dir() are left alone
and always create objects belonging to the global root.

When creating symlinks ownership (uid/gid) is taken from the target kernfs
object.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 fs/kernfs/dir.c             | 29 ++++++++++++++++++++++++++---
 fs/kernfs/file.c            |  8 ++++++--
 fs/kernfs/inode.c           |  2 +-
 fs/kernfs/kernfs-internal.h |  2 ++
 fs/kernfs/symlink.c         | 11 ++++++++++-
 fs/sysfs/dir.c              |  4 +++-
 fs/sysfs/file.c             |  5 +++--
 include/linux/kernfs.h      | 28 +++++++++++++++++++---------
 kernel/cgroup.c             |  4 +++-
 9 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 8a65240..67c276a 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -657,6 +657,7 @@ struct kernfs_node *kernfs_node_from_dentry(struct dentry *dentry)
 
 static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 					     const char *name, umode_t mode,
+					     kuid_t uid, kgid_t gid,
 					     unsigned flags)
 {
 	struct kernfs_node *kn;
@@ -683,8 +684,22 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 	kn->mode = mode;
 	kn->flags = flags;
 
+	if (!uid_eq(uid, GLOBAL_ROOT_UID) || !gid_eq(gid, GLOBAL_ROOT_GID)) {
+		struct iattr iattr = {
+			.ia_valid = ATTR_UID | ATTR_GID,
+			.ia_uid = uid,
+			.ia_gid = gid,
+		};
+
+		ret = __kernfs_setattr(kn, &iattr);
+		if (ret < 0)
+			goto err_out3;
+	}
+
 	return kn;
 
+ err_out3:
+	ida_simple_remove(&root->ino_ida, kn->ino);
  err_out2:
 	kmem_cache_free(kernfs_node_cache, kn);
  err_out1:
@@ -694,11 +709,13 @@ static struct kernfs_node *__kernfs_new_node(struct kernfs_root *root,
 
 struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
 				    const char *name, umode_t mode,
+				    kuid_t uid, kgid_t gid,
 				    unsigned flags)
 {
 	struct kernfs_node *kn;
 
-	kn = __kernfs_new_node(kernfs_root(parent), name, mode, flags);
+	kn = __kernfs_new_node(kernfs_root(parent),
+			       name, mode, uid, gid, flags);
 	if (kn) {
 		kernfs_get(parent);
 		kn->parent = parent;
@@ -919,6 +936,7 @@ struct kernfs_root *kernfs_create_root(struct kernfs_syscall_ops *scops,
 	INIT_LIST_HEAD(&root->supers);
 
 	kn = __kernfs_new_node(root, "", S_IFDIR | S_IRUGO | S_IXUGO,
+			       GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
 			       KERNFS_DIR);
 	if (!kn) {
 		ida_destroy(&root->ino_ida);
@@ -957,6 +975,8 @@ void kernfs_destroy_root(struct kernfs_root *root)
  * @parent: parent in which to create a new directory
  * @name: name of the new directory
  * @mode: mode of the new directory
+ * @uid: uid of the new directory
+ * @gid: gid of the new directory
  * @priv: opaque data associated with the new directory
  * @ns: optional namespace tag of the directory
  *
@@ -964,13 +984,15 @@ void kernfs_destroy_root(struct kernfs_root *root)
  */
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
 					 const char *name, umode_t mode,
+					 kuid_t uid, kgid_t gid,
 					 void *priv, const void *ns)
 {
 	struct kernfs_node *kn;
 	int rc;
 
 	/* allocate */
-	kn = kernfs_new_node(parent, name, mode | S_IFDIR, KERNFS_DIR);
+	kn = kernfs_new_node(parent, name, mode | S_IFDIR,
+			     uid, gid, KERNFS_DIR);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
@@ -1001,7 +1023,8 @@ struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent,
 	int rc;
 
 	/* allocate */
-	kn = kernfs_new_node(parent, name, S_IRUGO|S_IXUGO|S_IFDIR, KERNFS_DIR);
+	kn = kernfs_new_node(parent, name, S_IRUGO|S_IXUGO|S_IFDIR,
+			     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, KERNFS_DIR);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index e157400..736a168 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -904,6 +904,8 @@ const struct file_operations kernfs_file_fops = {
  * @parent: directory to create the file in
  * @name: name of the file
  * @mode: mode of the file
+ * @uid: uid of the file
+ * @gid: gid of the file
  * @size: size of the file
  * @ops: kernfs operations for the file
  * @priv: private data for the file
@@ -914,7 +916,8 @@ const struct file_operations kernfs_file_fops = {
  */
 struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
 					 const char *name,
-					 umode_t mode, loff_t size,
+					 umode_t mode, kuid_t uid, kgid_t gid,
+					 loff_t size,
 					 const struct kernfs_ops *ops,
 					 void *priv, const void *ns,
 					 struct lock_class_key *key)
@@ -925,7 +928,8 @@ struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
 
 	flags = KERNFS_FILE;
 
-	kn = kernfs_new_node(parent, name, (mode & S_IALLUGO) | S_IFREG, flags);
+	kn = kernfs_new_node(parent, name, (mode & S_IALLUGO) | S_IFREG,
+			     uid, gid, flags);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 63b925d..3d3f152 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -66,7 +66,7 @@ out_unlock:
 	return ret;
 }
 
-static int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr)
+int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr)
 {
 	struct kernfs_iattrs *attrs;
 	struct iattr *iattrs;
diff --git a/fs/kernfs/kernfs-internal.h b/fs/kernfs/kernfs-internal.h
index 3715923..80f4b5a 100644
--- a/fs/kernfs/kernfs-internal.h
+++ b/fs/kernfs/kernfs-internal.h
@@ -88,6 +88,7 @@ int kernfs_iop_removexattr(struct dentry *dentry, const char *name);
 ssize_t kernfs_iop_getxattr(struct dentry *dentry, struct inode *inode,
 			    const char *name, void *buf, size_t size);
 ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size);
+int __kernfs_setattr(struct kernfs_node *kn, const struct iattr *iattr);
 
 /*
  * dir.c
@@ -102,6 +103,7 @@ void kernfs_put_active(struct kernfs_node *kn);
 int kernfs_add_one(struct kernfs_node *kn);
 struct kernfs_node *kernfs_new_node(struct kernfs_node *parent,
 				    const char *name, umode_t mode,
+				    kuid_t uid, kgid_t gid,
 				    unsigned flags);
 
 /*
diff --git a/fs/kernfs/symlink.c b/fs/kernfs/symlink.c
index 117b8b3..e0847ce 100644
--- a/fs/kernfs/symlink.c
+++ b/fs/kernfs/symlink.c
@@ -21,6 +21,7 @@
  * @target: target node for the symlink to point to
  *
  * Returns the created node on success, ERR_PTR() value on error.
+ * Ownership of the link matches ownership of the target.
  */
 struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
 				       const char *name,
@@ -28,8 +29,16 @@ struct kernfs_node *kernfs_create_link(struct kernfs_node *parent,
 {
 	struct kernfs_node *kn;
 	int error;
+	kuid_t uid = GLOBAL_ROOT_UID;
+	kgid_t gid = GLOBAL_ROOT_GID;
 
-	kn = kernfs_new_node(parent, name, S_IFLNK|S_IRWXUGO, KERNFS_LINK);
+	if (target->iattr) {
+		uid = target->iattr->ia_iattr.ia_uid;
+		gid = target->iattr->ia_iattr.ia_gid;
+	}
+
+	kn = kernfs_new_node(parent, name, S_IFLNK|S_IRWXUGO, uid, gid,
+			     KERNFS_LINK);
 	if (!kn)
 		return ERR_PTR(-ENOMEM);
 
diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index 94374e4..d6a2b76 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -53,7 +53,9 @@ int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
 		return -ENOENT;
 
 	kn = kernfs_create_dir_ns(parent, kobject_name(kobj),
-				  S_IRWXU | S_IRUGO | S_IXUGO, kobj, ns);
+				  S_IRWXU | S_IRUGO | S_IXUGO,
+				  GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				  kobj, ns);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
 			sysfs_warn_dup(parent, kobject_name(kobj));
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index f35523d..0241830 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -296,8 +296,9 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 	if (!attr->ignore_lockdep)
 		key = attr->key ?: (struct lock_class_key *)&attr->skey;
 #endif
-	kn = __kernfs_create_file(parent, attr->name, mode & 0777, size, ops,
-				  (void *)attr, ns, key);
+	kn = __kernfs_create_file(parent, attr->name,
+				  mode & 0777, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				  size, ops, (void *)attr, ns, key);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
 			sysfs_warn_dup(parent, attr->name);
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 96356ef..0a3687d 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -15,6 +15,7 @@
 #include <linux/lockdep.h>
 #include <linux/rbtree.h>
 #include <linux/atomic.h>
+#include <linux/uidgid.h>
 #include <linux/wait.h>
 
 struct file;
@@ -295,12 +296,14 @@ void kernfs_destroy_root(struct kernfs_root *root);
 
 struct kernfs_node *kernfs_create_dir_ns(struct kernfs_node *parent,
 					 const char *name, umode_t mode,
+					 kuid_t uid, kgid_t gid,
 					 void *priv, const void *ns);
 struct kernfs_node *kernfs_create_empty_dir(struct kernfs_node *parent,
 					    const char *name);
 struct kernfs_node *__kernfs_create_file(struct kernfs_node *parent,
-					 const char *name,
-					 umode_t mode, loff_t size,
+					 const char *name, umode_t mode,
+					 kuid_t uid, kgid_t gid,
+					 loff_t size,
 					 const struct kernfs_ops *ops,
 					 void *priv, const void *ns,
 					 struct lock_class_key *key);
@@ -385,12 +388,14 @@ static inline void kernfs_destroy_root(struct kernfs_root *root) { }
 
 static inline struct kernfs_node *
 kernfs_create_dir_ns(struct kernfs_node *parent, const char *name,
-		     umode_t mode, void *priv, const void *ns)
+		     umode_t mode, kuid_t uid, kgid_t gid,
+		     void *priv, const void *ns)
 { return ERR_PTR(-ENOSYS); }
 
 static inline struct kernfs_node *
 __kernfs_create_file(struct kernfs_node *parent, const char *name,
-		     umode_t mode, loff_t size, const struct kernfs_ops *ops,
+		     umode_t mode, kuid_t uid, kgid_t gid,
+		     loff_t size, const struct kernfs_ops *ops,
 		     void *priv, const void *ns, struct lock_class_key *key)
 { return ERR_PTR(-ENOSYS); }
 
@@ -452,12 +457,15 @@ static inline struct kernfs_node *
 kernfs_create_dir(struct kernfs_node *parent, const char *name, umode_t mode,
 		  void *priv)
 {
-	return kernfs_create_dir_ns(parent, name, mode, priv, NULL);
+	return kernfs_create_dir_ns(parent, name, mode,
+				    GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				    priv, NULL);
 }
 
 static inline struct kernfs_node *
 kernfs_create_file_ns(struct kernfs_node *parent, const char *name,
-		      umode_t mode, loff_t size, const struct kernfs_ops *ops,
+		      umode_t mode, kuid_t uid, kgid_t gid,
+		      loff_t size, const struct kernfs_ops *ops,
 		      void *priv, const void *ns)
 {
 	struct lock_class_key *key = NULL;
@@ -465,15 +473,17 @@ kernfs_create_file_ns(struct kernfs_node *parent, const char *name,
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	key = (struct lock_class_key *)&ops->lockdep_key;
 #endif
-	return __kernfs_create_file(parent, name, mode, size, ops, priv, ns,
-				    key);
+	return __kernfs_create_file(parent, name, mode, uid, gid,
+				    size, ops, priv, ns, key);
 }
 
 static inline struct kernfs_node *
 kernfs_create_file(struct kernfs_node *parent, const char *name, umode_t mode,
 		   loff_t size, const struct kernfs_ops *ops, void *priv)
 {
-	return kernfs_create_file_ns(parent, name, mode, size, ops, priv, NULL);
+	return kernfs_create_file_ns(parent, name, mode,
+				     GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				     size, ops, priv, NULL);
 }
 
 static inline int kernfs_remove_by_name(struct kernfs_node *parent,
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 75c0ff0..86ec412 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -3634,7 +3634,9 @@ static int cgroup_add_file(struct cgroup_subsys_state *css, struct cgroup *cgrp,
 	key = &cft->lockdep_key;
 #endif
 	kn = __kernfs_create_file(cgrp->kn, cgroup_file_name(cgrp, cft, name),
-				  cgroup_file_mode(cft), 0, cft->kf_ops, cft,
+				  cgroup_file_mode(cft),
+				  GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				  0, cft->kf_ops, cft,
 				  NULL, key);
 	if (IS_ERR(kn))
 		return PTR_ERR(kn);
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/5] sysfs, kobject: allow creating kobject belonging to arbitrary users
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid Dmitry Torokhov
@ 2016-08-16 22:33 ` Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 3/5] kobject: kset_create_and_add() - fetch ownership info from parent Dmitry Torokhov
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

Normally kobjects and their sysfs representation belong to global root,
however it is not necessarily the case for objects in separate namespaces.
For example, objects in separate network namespace logically belong to the
container's root and not global root.

This change lays groundwork for allowing network namespace objects
ownership to be transferred to container's root user by defining
get_ownership() callback in ktype structure and using it in sysfs code to
retrieve desired uid/gid when creating sysfs objects for given kobject.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 fs/sysfs/dir.c          |  7 +++++--
 fs/sysfs/file.c         | 32 ++++++++++++++++++++------------
 fs/sysfs/group.c        | 23 +++++++++++++++++------
 fs/sysfs/sysfs.h        |  3 ++-
 include/linux/kobject.h |  4 ++++
 lib/kobject.c           | 19 +++++++++++++++++++
 6 files changed, 67 insertions(+), 21 deletions(-)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index d6a2b76..7ef6524 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -41,6 +41,8 @@ void sysfs_warn_dup(struct kernfs_node *parent, const char *name)
 int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
 {
 	struct kernfs_node *parent, *kn;
+	kuid_t uid;
+	kgid_t gid;
 
 	BUG_ON(!kobj);
 
@@ -52,9 +54,10 @@ int sysfs_create_dir_ns(struct kobject *kobj, const void *ns)
 	if (!parent)
 		return -ENOENT;
 
+	kobject_get_ownership(kobj, &uid, &gid);
+
 	kn = kernfs_create_dir_ns(parent, kobject_name(kobj),
-				  S_IRWXU | S_IRUGO | S_IXUGO,
-				  GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+				  S_IRWXU | S_IRUGO | S_IXUGO, uid, gid,
 				  kobj, ns);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 0241830..8411e7c 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -239,7 +239,7 @@ static const struct kernfs_ops sysfs_bin_kfops_mmap = {
 
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 			   const struct attribute *attr, bool is_bin,
-			   umode_t mode, const void *ns)
+			   umode_t mode, kuid_t uid, kgid_t gid, const void *ns)
 {
 	struct lock_class_key *key = NULL;
 	const struct kernfs_ops *ops;
@@ -296,8 +296,8 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 	if (!attr->ignore_lockdep)
 		key = attr->key ?: (struct lock_class_key *)&attr->skey;
 #endif
-	kn = __kernfs_create_file(parent, attr->name,
-				  mode & 0777, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+
+	kn = __kernfs_create_file(parent, attr->name, mode & 0777, uid, gid,
 				  size, ops, (void *)attr, ns, key);
 	if (IS_ERR(kn)) {
 		if (PTR_ERR(kn) == -EEXIST)
@@ -307,12 +307,6 @@ int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 	return 0;
 }
 
-int sysfs_add_file(struct kernfs_node *parent, const struct attribute *attr,
-		   bool is_bin)
-{
-	return sysfs_add_file_mode_ns(parent, attr, is_bin, attr->mode, NULL);
-}
-
 /**
  * sysfs_create_file_ns - create an attribute file for an object with custom ns
  * @kobj: object we're creating for
@@ -322,9 +316,14 @@ int sysfs_add_file(struct kernfs_node *parent, const struct attribute *attr,
 int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr,
 			 const void *ns)
 {
+	kuid_t uid;
+	kgid_t gid;
+
 	BUG_ON(!kobj || !kobj->sd || !attr);
 
-	return sysfs_add_file_mode_ns(kobj->sd, attr, false, attr->mode, ns);
+	kobject_get_ownership(kobj, &uid, &gid);
+	return sysfs_add_file_mode_ns(kobj->sd, attr, false, attr->mode,
+				      uid, gid, ns);
 
 }
 EXPORT_SYMBOL_GPL(sysfs_create_file_ns);
@@ -353,6 +352,8 @@ int sysfs_add_file_to_group(struct kobject *kobj,
 		const struct attribute *attr, const char *group)
 {
 	struct kernfs_node *parent;
+	kuid_t uid;
+	kgid_t gid;
 	int error;
 
 	if (group) {
@@ -365,7 +366,9 @@ int sysfs_add_file_to_group(struct kobject *kobj,
 	if (!parent)
 		return -ENOENT;
 
-	error = sysfs_add_file(parent, attr, false);
+	kobject_get_ownership(kobj, &uid, &gid);
+	error = sysfs_add_file_mode_ns(kobj->sd, attr, false,
+				       attr->mode, uid, gid, NULL);
 	kernfs_put(parent);
 
 	return error;
@@ -481,9 +484,14 @@ EXPORT_SYMBOL_GPL(sysfs_remove_file_from_group);
 int sysfs_create_bin_file(struct kobject *kobj,
 			  const struct bin_attribute *attr)
 {
+	kuid_t uid;
+	kgid_t gid;
+
 	BUG_ON(!kobj || !kobj->sd || !attr);
 
-	return sysfs_add_file(kobj->sd, &attr->attr, true);
+	kobject_get_ownership(kobj, &uid, &gid);
+	return sysfs_add_file_mode_ns(kobj->sd, &attr->attr, true,
+				      attr->attr.mode, uid, gid, NULL);
 }
 EXPORT_SYMBOL_GPL(sysfs_create_bin_file);
 
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 5e98639..0d4798c 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -34,6 +34,7 @@ static void remove_files(struct kernfs_node *parent,
 }
 
 static int create_files(struct kernfs_node *parent, struct kobject *kobj,
+			kuid_t uid, kgid_t gid,
 			const struct attribute_group *grp, int update)
 {
 	struct attribute *const *attr;
@@ -63,7 +64,7 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 
 			mode &= SYSFS_PREALLOC | 0664;
 			error = sysfs_add_file_mode_ns(parent, *attr, false,
-						       mode, NULL);
+						       mode, uid, gid, NULL);
 			if (unlikely(error))
 				break;
 		}
@@ -93,7 +94,8 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 			mode &= SYSFS_PREALLOC | 0664;
 			error = sysfs_add_file_mode_ns(parent,
 					&(*bin_attr)->attr, true,
-					mode, NULL);
+					mode,
+					uid, gid, NULL);
 			if (error)
 				break;
 		}
@@ -109,6 +111,8 @@ static int internal_create_group(struct kobject *kobj, int update,
 				 const struct attribute_group *grp)
 {
 	struct kernfs_node *kn;
+	kuid_t uid;
+	kgid_t gid;
 	int error;
 
 	BUG_ON(!kobj || (!update && !kobj->sd));
@@ -121,9 +125,11 @@ static int internal_create_group(struct kobject *kobj, int update,
 			kobj->name, grp->name ?: "");
 		return -EINVAL;
 	}
+	kobject_get_ownership(kobj, &uid, &gid);
 	if (grp->name) {
-		kn = kernfs_create_dir(kobj->sd, grp->name,
-				       S_IRWXU | S_IRUGO | S_IXUGO, kobj);
+		kn = kernfs_create_dir_ns(kobj->sd, grp->name,
+					  S_IRWXU | S_IRUGO | S_IXUGO,
+					  uid, gid, kobj, NULL);
 		if (IS_ERR(kn)) {
 			if (PTR_ERR(kn) == -EEXIST)
 				sysfs_warn_dup(kobj->sd, grp->name);
@@ -132,7 +138,7 @@ static int internal_create_group(struct kobject *kobj, int update,
 	} else
 		kn = kobj->sd;
 	kernfs_get(kn);
-	error = create_files(kn, kobj, grp, update);
+	error = create_files(kn, kobj, uid, gid, grp, update);
 	if (error) {
 		if (grp->name)
 			kernfs_remove(kn);
@@ -284,6 +290,8 @@ int sysfs_merge_group(struct kobject *kobj,
 		       const struct attribute_group *grp)
 {
 	struct kernfs_node *parent;
+	kuid_t uid;
+	kgid_t gid;
 	int error = 0;
 	struct attribute *const *attr;
 	int i;
@@ -292,8 +300,11 @@ int sysfs_merge_group(struct kobject *kobj,
 	if (!parent)
 		return -ENOENT;
 
+	kobject_get_ownership(kobj, &uid, &gid);
+
 	for ((i = 0, attr = grp->attrs); *attr && !error; (++i, ++attr))
-		error = sysfs_add_file(parent, *attr, false);
+		error = sysfs_add_file_mode_ns(parent, *attr, false,
+					       (*attr)->mode, uid, gid, NULL);
 	if (error) {
 		while (--i >= 0)
 			kernfs_remove_by_name(parent, (*--attr)->name);
diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h
index 0e2f1cc..9a38568 100644
--- a/fs/sysfs/sysfs.h
+++ b/fs/sysfs/sysfs.h
@@ -32,7 +32,8 @@ int sysfs_add_file(struct kernfs_node *parent,
 		   const struct attribute *attr, bool is_bin);
 int sysfs_add_file_mode_ns(struct kernfs_node *parent,
 			   const struct attribute *attr, bool is_bin,
-			   umode_t amode, const void *ns);
+			   umode_t amode, kuid_t uid, kgid_t gid,
+			   const void *ns);
 
 /*
  * symlink.c
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index e628459..3c8a689 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -27,6 +27,7 @@
 #include <linux/wait.h>
 #include <linux/atomic.h>
 #include <linux/workqueue.h>
+#include <linux/uidgid.h>
 
 #define UEVENT_HELPER_PATH_LEN		256
 #define UEVENT_NUM_ENVP			32	/* number of env pointers */
@@ -111,6 +112,8 @@ extern struct kobject *kobject_get(struct kobject *kobj);
 extern void kobject_put(struct kobject *kobj);
 
 extern const void *kobject_namespace(struct kobject *kobj);
+extern void kobject_get_ownership(struct kobject *kobj,
+				  kuid_t *uid, kgid_t *gid);
 extern char *kobject_get_path(struct kobject *kobj, gfp_t flag);
 
 struct kobj_type {
@@ -119,6 +122,7 @@ struct kobj_type {
 	struct attribute **default_attrs;
 	const struct kobj_ns_type_operations *(*child_ns_type)(struct kobject *kobj);
 	const void *(*namespace)(struct kobject *kobj);
+	void (*get_ownership)(struct kobject *kobj, kuid_t *uid, kgid_t *gid);
 };
 
 struct kobj_uevent_env {
diff --git a/lib/kobject.c b/lib/kobject.c
index 445dcae..7d516d9 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -37,6 +37,25 @@ const void *kobject_namespace(struct kobject *kobj)
 	return kobj->ktype->namespace(kobj);
 }
 
+/**
+ * kobject_get_ownership - get sysfs ownership data for @kobj
+ * @kobj: kobject in question
+ * @uid: kernel user ID for sysfs objects
+ * @gid: kernel group ID for sysfs objects
+ *
+ * Returns initial uid/gid pair that should be used when creating sysfs
+ * representation of given kobject. Normally used to adjust ownership of
+ * objects in a container.
+ */
+void kobject_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid)
+{
+	*uid = GLOBAL_ROOT_UID;
+	*gid = GLOBAL_ROOT_GID;
+
+	if (kobj->ktype->get_ownership)
+		kobj->ktype->get_ownership(kobj, uid, gid);
+}
+
 /*
  * populate_dir - populate directory with attributes.
  * @kobj: object we're working on.
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/5] kobject: kset_create_and_add() - fetch ownership info from parent
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 2/5] sysfs, kobject: allow creating kobject belonging to arbitrary users Dmitry Torokhov
@ 2016-08-16 22:33 ` Dmitry Torokhov
  2016-08-16 22:33 ` [PATCH 4/5] driver core: set up ownership of class devices in sysfs Dmitry Torokhov
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

This change implements get_ownership() for ksets created with
kset_create_and_add() call by fetching ownership data from parent kobject.
This is done mostly for benefit of "queues" attribute of net devices so
that corresponding directory belongs to container's root instead of global
root for network devices in a container.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 lib/kobject.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/kobject.c b/lib/kobject.c
index 7d516d9..f81ac7c 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -890,9 +890,16 @@ static void kset_release(struct kobject *kobj)
 	kfree(kset);
 }
 
+void kset_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid)
+{
+	if (kobj->parent)
+		kobject_get_ownership(kobj->parent, uid, gid);
+}
+
 static struct kobj_type kset_ktype = {
 	.sysfs_ops	= &kobj_sysfs_ops,
-	.release = kset_release,
+	.release	= kset_release,
+	.get_ownership	= kset_get_ownership,
 };
 
 /**
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/5] driver core: set up ownership of class devices in sysfs
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2016-08-16 22:33 ` [PATCH 3/5] kobject: kset_create_and_add() - fetch ownership info from parent Dmitry Torokhov
@ 2016-08-16 22:33 ` Dmitry Torokhov
  2016-08-20  2:26   ` Stephen Hemminger
  2016-08-16 22:33 ` [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner Dmitry Torokhov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

Plumb in get_ownership() callback for devices belonging to a class so that
they can be created with uid/gid different from global root. This will
allow network devices in a container to belong to container's root and not
global root.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/base/core.c    | 9 +++++++++
 include/linux/device.h | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 0a8bdad..0f95454 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -263,10 +263,19 @@ static const void *device_namespace(struct kobject *kobj)
 	return ns;
 }
 
+static void device_get_ownership(struct kobject *kobj, kuid_t *uid, kgid_t *gid)
+{
+	struct device *dev = kobj_to_dev(kobj);
+
+	if (dev->class && dev->class->get_ownership)
+		dev->class->get_ownership(dev, uid, gid);
+}
+
 static struct kobj_type device_ktype = {
 	.release	= device_release,
 	.sysfs_ops	= &dev_sysfs_ops,
 	.namespace	= device_namespace,
+	.get_ownership	= device_get_ownership,
 };
 
 
diff --git a/include/linux/device.h b/include/linux/device.h
index 38f0281..da20b12 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -374,6 +374,9 @@ int subsys_virtual_register(struct bus_type *subsys,
  * @resume:	Used to bring the device from the sleep mode.
  * @ns_type:	Callbacks so sysfs can detemine namespaces.
  * @namespace:	Namespace of the device belongs to this class.
+ * @get_ownership: Allows class to specify uid/gid of the sysfs directories
+ *		for the devices belonging to the class. Usually tied to
+ *		device's namespace.
  * @pm:		The default device power management operations of this class.
  * @p:		The private data of the driver core, no one other than the
  *		driver core can touch this.
@@ -404,6 +407,8 @@ struct class {
 	const struct kobj_ns_type_operations *ns_type;
 	const void *(*namespace)(struct device *dev);
 
+	void (*get_ownership)(struct device *dev, kuid_t *uid, kgid_t *gid);
+
 	const struct dev_pm_ops *pm;
 
 	struct subsys_private *p;
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2016-08-16 22:33 ` [PATCH 4/5] driver core: set up ownership of class devices in sysfs Dmitry Torokhov
@ 2016-08-16 22:33 ` Dmitry Torokhov
  2016-08-22  6:03   ` kbuild test robot
  2016-08-19 23:59 ` [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container David Miller
  2016-08-22  6:41 ` David Miller
  6 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-16 22:33 UTC (permalink / raw)
  To: Tejun Heo, Eric W. Biederman, David S. Miller; +Cc: linux-kernel, netdev

When creating various objects in /sys/class/net/... make sure that they
belong to container's owner instead of global root (if they belong to a
container/namespace).

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 net/core/net-sysfs.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7a0b616..ef997bb 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -882,11 +882,35 @@ static const void *rx_queue_namespace(struct kobject *kobj)
 	return ns;
 }
 
+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;
+	}
+}
+
+static void rx_queue_get_ownership(struct kobject *kobj,
+				   kuid_t *uid, kgid_t *gid)
+{
+	const struct net *net = rx_queue_namespace(kobj);
+
+	net_ns_get_ownership(net, uid, gid);
+}
+
 static struct kobj_type rx_queue_ktype = {
 	.sysfs_ops = &rx_queue_sysfs_ops,
 	.release = rx_queue_release,
 	.default_attrs = rx_queue_default_attrs,
-	.namespace = rx_queue_namespace
+	.namespace = rx_queue_namespace,
+	.get_ownership = rx_queue_get_ownership,
 };
 
 static int rx_queue_add_kobject(struct net_device *dev, int index)
@@ -1274,11 +1298,20 @@ static const void *netdev_queue_namespace(struct kobject *kobj)
 	return ns;
 }
 
+static void netdev_queue_get_ownership(struct kobject *kobj,
+				       kuid_t *uid, kgid_t *gid)
+{
+	const struct net *net = netdev_queue_namespace(kobj);
+
+	net_ns_get_ownership(net, uid, gid);
+}
+
 static struct kobj_type netdev_queue_ktype = {
 	.sysfs_ops = &netdev_queue_sysfs_ops,
 	.release = netdev_queue_release,
 	.default_attrs = netdev_queue_default_attrs,
 	.namespace = netdev_queue_namespace,
+	.get_ownership = netdev_queue_get_ownership,
 };
 
 static int netdev_queue_add_kobject(struct net_device *dev, int index)
@@ -1463,6 +1496,14 @@ static const void *net_namespace(struct device *d)
 	return dev_net(dev);
 }
 
+static void net_get_ownership(struct device *d, kuid_t *uid, kgid_t *gid)
+{
+	struct net_device *dev = to_net_dev(d);
+	const struct net *net = dev_net(dev);
+
+	net_ns_get_ownership(net, uid, gid);
+}
+
 static struct class net_class = {
 	.name = "net",
 	.dev_release = netdev_release,
@@ -1470,6 +1511,7 @@ static struct class net_class = {
 	.dev_uevent = netdev_uevent,
 	.ns_type = &net_ns_type_operations,
 	.namespace = net_namespace,
+	.get_ownership = net_get_ownership,
 };
 
 #ifdef CONFIG_OF_NET
-- 
2.8.0.rc3.226.g39d4020

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2016-08-16 22:33 ` [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner Dmitry Torokhov
@ 2016-08-19 23:59 ` David Miller
  2016-08-29 12:38   ` Eric W. Biederman
  2016-08-22  6:41 ` David Miller
  6 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2016-08-19 23:59 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: tj, ebiederm, linux-kernel, netdev

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Tue, 16 Aug 2016 15:33:10 -0700

> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
> to a namespace/container. Unfortunately all sysfs objects start their life
> belonging to global root, and while we could change ownership manually,
> keeping tracks of all objects that come and go is cumbersome. It would
> be better if kernel created them using correct uid/gid from the beginning.
> 
> This series changes kernfs to allow creating object's with arbitrary
> uid/gid, adds get_ownership() callback to ktype structure so subsystems
> could supply their own logic (likely tied to namespace support) for
> determining ownership of kobjects, and adjusts sysfs code to make use of
> this information. Lastly net-sysfs is adjusted to make sure that objects in
> net namespace are owned by the root user from the owning user namespace.
> 
> Note that we do not adjust ownership of objects moved into a new namespace
> (as when moving a network device into a container) as userspace can easily
> do it.

I need some domain experts to review this series please.

Thank you.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/5] driver core: set up ownership of class devices in sysfs
  2016-08-16 22:33 ` [PATCH 4/5] driver core: set up ownership of class devices in sysfs Dmitry Torokhov
@ 2016-08-20  2:26   ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2016-08-20  2:26 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Tejun Heo, Eric W. Biederman, David S. Miller, linux-kernel, netdev

On Tue, 16 Aug 2016 15:33:14 -0700
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

>  static struct kobj_type device_ktype = {
>  	.release	= device_release,
>  	.sysfs_ops	= &dev_sysfs_ops,
>  	.namespace	= device_namespace,
> +	.get_ownership	= device_get_ownership,
>  };
>  

OT - I wonder if kobj_type could be ro_after_init?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner
  2016-08-16 22:33 ` [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner Dmitry Torokhov
@ 2016-08-22  6:03   ` kbuild test robot
  0 siblings, 0 replies; 14+ messages in thread
From: kbuild test robot @ 2016-08-22  6:03 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: kbuild-all, Tejun Heo, Eric W. Biederman, David S. Miller,
	linux-kernel, netdev

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

Hi Dmitry,

[auto build test ERROR on driver-core/driver-core-testing]
[also build test ERROR on v4.8-rc3 next-20160819]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Torokhov/Make-sys-class-net-per-net-namespace-objects-belong-to-container/20160817-064309
config: m68k-m5208evb_defconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 4.9.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m68k 

All errors (new ones prefixed by >>):

   net/core/net-sysfs.c: In function 'net_get_ownership':
>> net/core/net-sysfs.c:1517:2: error: implicit declaration of function 'net_ns_get_ownership' [-Werror=implicit-function-declaration]
     net_ns_get_ownership(net, uid, gid);
     ^
   cc1: some warnings being treated as errors

vim +/net_ns_get_ownership +1517 net/core/net-sysfs.c

  1511	
  1512	static void net_get_ownership(struct device *d, kuid_t *uid, kgid_t *gid)
  1513	{
  1514		struct net_device *dev = to_net_dev(d);
  1515		const struct net *net = dev_net(dev);
  1516	
> 1517		net_ns_get_ownership(net, uid, gid);
  1518	}
  1519	
  1520	static struct class net_class = {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 7657 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
                   ` (5 preceding siblings ...)
  2016-08-19 23:59 ` [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container David Miller
@ 2016-08-22  6:41 ` David Miller
  2016-08-22 16:24   ` Dmitry Torokhov
  6 siblings, 1 reply; 14+ messages in thread
From: David Miller @ 2016-08-22  6:41 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: tj, ebiederm, linux-kernel, netdev

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: Tue, 16 Aug 2016 15:33:10 -0700

> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
> to a namespace/container. Unfortunately all sysfs objects start their life
> belonging to global root, and while we could change ownership manually,
> keeping tracks of all objects that come and go is cumbersome. It would
> be better if kernel created them using correct uid/gid from the beginning.
> 
> This series changes kernfs to allow creating object's with arbitrary
> uid/gid, adds get_ownership() callback to ktype structure so subsystems
> could supply their own logic (likely tied to namespace support) for
> determining ownership of kobjects, and adjusts sysfs code to make use of
> this information. Lastly net-sysfs is adjusted to make sure that objects in
> net namespace are owned by the root user from the owning user namespace.
> 
> Note that we do not adjust ownership of objects moved into a new namespace
> (as when moving a network device into a container) as userspace can easily
> do it.

As shown by the kbuild robot, this fails to build when CONFIG_SYSFS
it disabled.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-08-22  6:41 ` David Miller
@ 2016-08-22 16:24   ` Dmitry Torokhov
  0 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2016-08-22 16:24 UTC (permalink / raw)
  To: David Miller; +Cc: tj, ebiederm, linux-kernel, netdev

On Sun, Aug 21, 2016 at 11:41:39PM -0700, David Miller wrote:
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Tue, 16 Aug 2016 15:33:10 -0700
> 
> > There are objects in /sys hierarchy (/sys/class/net/) that logically belong
> > to a namespace/container. Unfortunately all sysfs objects start their life
> > belonging to global root, and while we could change ownership manually,
> > keeping tracks of all objects that come and go is cumbersome. It would
> > be better if kernel created them using correct uid/gid from the beginning.
> > 
> > This series changes kernfs to allow creating object's with arbitrary
> > uid/gid, adds get_ownership() callback to ktype structure so subsystems
> > could supply their own logic (likely tied to namespace support) for
> > determining ownership of kobjects, and adjusts sysfs code to make use of
> > this information. Lastly net-sysfs is adjusted to make sure that objects in
> > net namespace are owned by the root user from the owning user namespace.
> > 
> > Note that we do not adjust ownership of objects moved into a new namespace
> > (as when moving a network device into a container) as userspace can easily
> > do it.
> 
> As shown by the kbuild robot, this fails to build when CONFIG_SYSFS
> it disabled.

Sorry about that, I haven't noticed that all netdev queue stuff is
protected by #ifdef CONFIG_SYSFS. I'll move the definition out and
resubmit in a few days (maybe Eric and Tejun will have some comments for
be by then as well...).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-08-19 23:59 ` [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container David Miller
@ 2016-08-29 12:38   ` Eric W. Biederman
  2016-09-15  3:24     ` Dmitry Torokhov
  0 siblings, 1 reply; 14+ messages in thread
From: Eric W. Biederman @ 2016-08-29 12:38 UTC (permalink / raw)
  To: David Miller; +Cc: dmitry.torokhov, tj, linux-kernel, netdev

David Miller <davem@davemloft.net> writes:

> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Date: Tue, 16 Aug 2016 15:33:10 -0700
>
>> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
>> to a namespace/container. Unfortunately all sysfs objects start their life
>> belonging to global root, and while we could change ownership manually,
>> keeping tracks of all objects that come and go is cumbersome. It would
>> be better if kernel created them using correct uid/gid from the beginning.
>> 
>> This series changes kernfs to allow creating object's with arbitrary
>> uid/gid, adds get_ownership() callback to ktype structure so subsystems
>> could supply their own logic (likely tied to namespace support) for
>> determining ownership of kobjects, and adjusts sysfs code to make use of
>> this information. Lastly net-sysfs is adjusted to make sure that objects in
>> net namespace are owned by the root user from the owning user namespace.
>> 
>> Note that we do not adjust ownership of objects moved into a new namespace
>> (as when moving a network device into a container) as userspace can easily
>> do it.
>
> I need some domain experts to review this series please.

I just came back from vacation and I will aim to take a look shortly.

The big picture idea seems sensible.  Having a better ownship of sysfs
files that are part of a network namespace.  I will have to look at the
details to see if the implementation is similarly sensible.

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-08-29 12:38   ` Eric W. Biederman
@ 2016-09-15  3:24     ` Dmitry Torokhov
  2016-09-15 23:26       ` Eric W. Biederman
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2016-09-15  3:24 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, Tejun Heo, lkml, netdev

On Mon, Aug 29, 2016 at 5:38 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> David Miller <davem@davemloft.net> writes:
>
>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Date: Tue, 16 Aug 2016 15:33:10 -0700
>>
>>> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
>>> to a namespace/container. Unfortunately all sysfs objects start their life
>>> belonging to global root, and while we could change ownership manually,
>>> keeping tracks of all objects that come and go is cumbersome. It would
>>> be better if kernel created them using correct uid/gid from the beginning.
>>>
>>> This series changes kernfs to allow creating object's with arbitrary
>>> uid/gid, adds get_ownership() callback to ktype structure so subsystems
>>> could supply their own logic (likely tied to namespace support) for
>>> determining ownership of kobjects, and adjusts sysfs code to make use of
>>> this information. Lastly net-sysfs is adjusted to make sure that objects in
>>> net namespace are owned by the root user from the owning user namespace.
>>>
>>> Note that we do not adjust ownership of objects moved into a new namespace
>>> (as when moving a network device into a container) as userspace can easily
>>> do it.
>>
>> I need some domain experts to review this series please.
>
> I just came back from vacation and I will aim to take a look shortly.
>
> The big picture idea seems sensible.  Having a better ownship of sysfs
> files that are part of a network namespace.  I will have to look at the
> details to see if the implementation is similarly sensible.

Eric,

Did you find anything objectionable in the series or should I fix up
the !CONFIG_SYSFS error in networking patch and resubmit?

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container
  2016-09-15  3:24     ` Dmitry Torokhov
@ 2016-09-15 23:26       ` Eric W. Biederman
  0 siblings, 0 replies; 14+ messages in thread
From: Eric W. Biederman @ 2016-09-15 23:26 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: David Miller, Tejun Heo, lkml, netdev

Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:

> On Mon, Aug 29, 2016 at 5:38 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
>> David Miller <davem@davemloft.net> writes:
>>
>>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> Date: Tue, 16 Aug 2016 15:33:10 -0700
>>>
>>>> There are objects in /sys hierarchy (/sys/class/net/) that logically belong
>>>> to a namespace/container. Unfortunately all sysfs objects start their life
>>>> belonging to global root, and while we could change ownership manually,
>>>> keeping tracks of all objects that come and go is cumbersome. It would
>>>> be better if kernel created them using correct uid/gid from the beginning.
>>>>
>>>> This series changes kernfs to allow creating object's with arbitrary
>>>> uid/gid, adds get_ownership() callback to ktype structure so subsystems
>>>> could supply their own logic (likely tied to namespace support) for
>>>> determining ownership of kobjects, and adjusts sysfs code to make use of
>>>> this information. Lastly net-sysfs is adjusted to make sure that objects in
>>>> net namespace are owned by the root user from the owning user namespace.
>>>>
>>>> Note that we do not adjust ownership of objects moved into a new namespace
>>>> (as when moving a network device into a container) as userspace can easily
>>>> do it.
>>>
>>> I need some domain experts to review this series please.
>>
>> I just came back from vacation and I will aim to take a look shortly.
>>
>> The big picture idea seems sensible.  Having a better ownship of sysfs
>> files that are part of a network namespace.  I will have to look at the
>> details to see if the implementation is similarly sensible.
>
> Eric,
>
> Did you find anything objectionable in the series or should I fix up
> the !CONFIG_SYSFS error in networking patch and resubmit?

Thank you for the ping, I put this patchset down and forgot to look
back.

The notion of a get_ownership call seems sensible.

At some level I am not a fan of setting the uids and gids on the sysfs
nodes as that requires allocation of an additional data structure and it
will increase the code of sysfs nodes.   Certainly I don't think we
should incur that cost if we are not using user namespaces.  sysfs nodes
can be expensive data wise because we sometimes have so many of them.
So skipping the setattr when uid == GLOBAL_ROOT_UID and gid ==
GLOBAL_ROOT_GID seems very desirable.  Perhaps that is just an
optimization in setattr, but it should be somewhere.

I would very much prefer it if we can find a way not to touch all of the
layers, in the stack.  As I recall it is the code in drivers/base/core.c
that creates the attributes.  So my gut feel says we want to export a
sysfs_setattr modeled after sysfs_chmod from sysfs.h and then just have
the driver core level perform the setattr calls for non-default uids and
gids.

Symlinks we don't need to worry about changing their ownership they are
globally read, write, execute.

As long as the chattr happens before the uevent is triggered the code
should be essentially race free in dealing with userspace.

I think that will lead to a simpler more comprehensible and more
maintainable implementation.  Hooking in where or near where the
namespace bits hook in seems excessively complicated (although there may
be a good reason for it) that I am forgetting.

Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-09-15 23:39 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-16 22:33 [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container Dmitry Torokhov
2016-08-16 22:33 ` [PATCH 1/5] kernfs: allow creating kernfs objects with arbitrary uid/gid Dmitry Torokhov
2016-08-16 22:33 ` [PATCH 2/5] sysfs, kobject: allow creating kobject belonging to arbitrary users Dmitry Torokhov
2016-08-16 22:33 ` [PATCH 3/5] kobject: kset_create_and_add() - fetch ownership info from parent Dmitry Torokhov
2016-08-16 22:33 ` [PATCH 4/5] driver core: set up ownership of class devices in sysfs Dmitry Torokhov
2016-08-20  2:26   ` Stephen Hemminger
2016-08-16 22:33 ` [PATCH 5/5] net-sysfs: make sure objects belong to contrainer's owner Dmitry Torokhov
2016-08-22  6:03   ` kbuild test robot
2016-08-19 23:59 ` [PATCH 0/5] Make /sys/class/net per net namespace objects belong to container David Miller
2016-08-29 12:38   ` Eric W. Biederman
2016-09-15  3:24     ` Dmitry Torokhov
2016-09-15 23:26       ` Eric W. Biederman
2016-08-22  6:41 ` David Miller
2016-08-22 16:24   ` Dmitry Torokhov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.