OpenVZ Forum


Home » Mailing lists » Devel » [PATCH] cgroups: implement device whitelist (v4)
[PATCH] cgroups: implement device whitelist (v4) [message #28417] Mon, 17 March 2008 18:07 Go to next message
serue is currently offline  serue
Messages: 750
Registered: February 2006
Senior Member
Implement a cgroup to track and enforce open and mknod restrictions on device
files.  A device cgroup associates a device access whitelist with each
cgroup.  A whitelist entry has 4 fields.  'type' is a (all), c (char), or
b (block).  'all' means it applies to all types and all major and minor
numbers.  Major and minor are either an integer or * for all.
Access is a composition of r (read), w (write), and m (mknod).

The root device cgroup starts with rwm to 'all'.  A child devcg gets
a copy of the parent.  Admins can then remove devices from the
whitelist or add new entries.  A child cgroup can never receive a
device access which is denied its parent.  However when a device
access is removed from a parent it will not also be removed from the
child(ren).

An entry is added using devices.allow, and removed using
devices.deny.  For instance

	echo 'c 1:3 mr' > /cgroups/1/devices.allow

allows cgroup 1 to read and mknod the device usually known as
/dev/null.  Doing

	echo a > /cgroups/1/devices.deny

will remove the default 'a *:* mrw' entry.

CAP_SYS_ADMIN is needed to change permissions or move another task
to a new cgroup.  A cgroup may not be granted more permissions than
the cgroup's parent has.  Any task can move itself between cgroups.
This won't be sufficient, but we can decide the best way to
adequately restrict movement later.

The parsing of devices.allow/deny needs to be cleaned up a bit and
Documented.  I'd like to get an idea whether this approach is otherwise
acceptable.

Changelog:
	Mar 17 2008: Place specific device cgroup hooks next to
		security_inode_{mknod,permission} rather than using
		the security hooks.
		Also remove most of the controls over tasks moving
		between cgroups and playing with the allow and deny
		permissions.
		Switch to major:minor format.
		Rename devcg to 'devices' to conform to cgroup naming.
	Mar 13 2008: move the dev_cgroup support into
		capability hooks instead of having it
		as a separate security module.
		Support root_plug with devcg.
		Note that due to this change, devcg will
		not be enforcing if the dummy module is
		loaded, or if selinux is loaded without
		capabilities.
	Mar 12 2008: allow dev_cgroup lsm to be used when
		SECURITY=n, and allow stacking with SELinux
		and Smack.  Don't work too hard in Kconfig
		to prevent a warning when smack+devcg are
		both compiled in, worry about that later.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
---
 fs/namei.c                    |    9 +
 include/linux/cgroup_subsys.h |    6 +
 include/linux/device_cgroup.h |   12 +
 init/Kconfig                  |    7 +
 security/Makefile             |    1 +
 security/device_cgroup.c      |  597 +++++++++++++++++++++++++++++++++++++++++
 6 files changed, 632 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/device_cgroup.h
 create mode 100644 security/device_cgroup.c

diff --git a/fs/namei.c b/fs/namei.c
index dfb3cb8..6caed32 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -30,6 +30,7 @@
 #include <linux/capability.h>
 #include <linux/file.h>
 #include <linux/fcntl.h>
+#include <linux/device_cgroup.h>
 #include <asm/namei.h>
 #include <asm/uaccess.h>
 
@@ -281,6 +282,10 @@ int permission(struct inode *inode, int mask, struct nameidata *nd)
 	if (retval)
 		return retval;
 
+	retval = devcgroup_inode_permission(inode, mask);
+	if (retval)
+		return retval;
+
 	return security_inode_permission(inode, mask, nd);
 }
 
@@ -2028,6 +2033,10 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
 	if (!dir->i_op || !dir->i_op->mknod)
 		return -EPERM;
 
+	error = devcgroup_inode_mknod(mode, dev);
+	if (error)
+		return error;
+
 	error = security_inode_mknod(dir, dentry, mode, dev);
 	if (error)
 		return error;
diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
index 1ddebfc..e287745 100644
--- a/include/linux/cgroup_subsys.h
+++ b/include/linux/cgroup_subsys.h
@@ -42,3 +42,9 @@ SUBSYS(mem_cgroup)
 #endif
 
 /* */
+
+#ifdef CONFIG_CGROUP_DEVICE
+SUBSYS(devices)
+#endif
+
+/* */
diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h
new file mode 100644
index 0000000..0b0d9c3
--- /dev/null
+++ b/include/linux/device_cgroup.h
@@ -0,0 +1,12 @@
+#include <linux/module.h>
+#include <linux/fs.h>
+
+#ifdef CONFIG_CGROUP_DEVICE
+extern int devcgroup_inode_permission(struct inode *inode, int mask);
+extern int devcgroup_inode_mknod(int mode, dev_t dev);
+#else
+static inline int devcgroup_inode_permission(struct inode *inode, int mask)
+{ return 0; }
+static inline int devcgroup_inode_mknod(int mode, dev_t dev)
+{ return 0; }
+#endif
diff --git a/init/Kconfig b/init/Kconfig
index 009f2d8..30868cd 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -298,6 +298,13 @@ config CGROUP_NS
           for instance virtual servers and checkpoint/restart
           jobs.
 
+config CGROUP_DEVICE
+	bool "Device controller for cgroups"
+	depends on CGROUPS && EXPERIMENTAL
+	help
+	  Provides a cgroup implementing whitelists for devices which
+	  a process in the cgroup can mknod or open.
+
 config CPUSETS
 	bool "Cpuset support"
 	depends on SMP && CGROUPS
diff --git a/security/Makefile b/security/Makefile
index 9e8b025..7ef1107 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -18,3 +18,4 @@ obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
 obj-$(CONFIG_SECURITY_SMACK)		+= commoncap.o smack/built-in.o
 obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
 obj-$(CONFIG_SECURITY_ROOTPLUG)		+= commoncap.o root_plug.o
+obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
diff --git a/security/device_cgroup.c b/security/device_cgroup.c
new file mode 100644
index 0000000..33d8fd8
--- /dev/null
+++ b/security/device_cgroup.c
@@ -0,0 +1,597 @@
+/*
+ * dev_cgroup.c - device cgroup subsystem
+ *
+ * Copyright 2007 IBM Corp
+ */
+
+#include <linux/device_cgroup.h>
+#include <linux/cgroup.h>
+#include <linux/ctype.h>
+#include <linux/list.h>
+#include <asm/uaccess.h>
+
+#define ACC_MKNOD 1
+#define ACC_READ  2
+#define ACC_WRITE 4
+#define ACC_MASK (ACC_MKNOD | ACC_READ | ACC_WRITE)
+
+#define DEV_BLOCK 1
+#define DEV_CHAR  2
+#define DEV_ALL   4  /* this represents all devices */
+
+/*
+ * whitelist locking rules:
+ * cgroup_lock() cannot be taken under cgroup->lock.
+ * cgroup->lock can be taken with or without cgroup_lock().
+ *
+ * modifications always require cgroup_lock
+ * modifications to a list which is visible require the
+ *   cgroup->lock *and* cgroup_lock()
+ * walking the list requires cgroup->lock or cgroup_lock().
+ *
+ * reasoning: dev_whitelist_copy() needs to kmalloc, so needs
+ *   a mutex, which the cgroup_lock() is.  Since modifying
+ *   a visible list requires both locks, either lock can be
+ *   taken for walking the list.  Since the wh->spinlock is taken
+ *   for modifying a public-accessible list, the spinlock is
+ *   sufficient for just walking the list.
+ */
+
+struct dev_whitelist_item {
+	u32 major, minor;
+	short type;
+	short access;
+	struct list_head list;
+};
+
+struct dev_cgroup {
+	struct cgroup_subsys_state css;
+	struct list_head whitelist;
+	spinlock_t lock;
+};
+
+static inline struct dev_cgroup *cgroup_to_devcgroup(
+		struct cgroup *cgroup)
+{
+	return container_of(cgroup_subsys_state(cgroup, devices_subsys_id),
+			    struct dev_cgroup, css);
+}
+
+
+struct cgroup_subsys devices_subsys;
+
+static int devcgroup_can_attach(struct cgroup_subsys *ss,
+		struct cgroup *new_cgroup, struct task_struct *task)
+{
+
+	if (current != task && !capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+	return 0;
+}
+
+/*
+ * called under cgroup_lock()
+ */
+int dev_whitelist_copy(struct list_head *dest, struct list_head *orig)
+{
+	struct dev_whitelist_item *wh, *tmp, *new;
+
+	list_for_each_entry(wh, orig, list) {
+		new = kmalloc(sizeof(*wh), GFP_KERNEL);
+		if (!new)
+			goto free_and_exit;
+		new->major = wh->major;
+		new->minor = wh->minor;
+		new->type = wh->type;
+		new->access = wh->access;
+		list_add_tail(&new->list, dest);
+	}
+
+	return 0;
+
+free_and_exit:
+	list_for_each_entry_safe(wh, tmp, dest, list) {
+		list_del(&wh->list);
+		kfree(wh);
+	}
+	return -ENOMEM;
+}
+
+/* Stupid prototype - don't bother combining existing entries */
+/*
+ * called under cgroup_lock()
+ * since the list is visible to other tasks, we need the spinlock also
+ */
+int dev_whitelist_add(struct dev_cgroup *dev_cgroup,
+			struct dev_whitelist_item *wh)
+{
+	struct dev_whitelist_item *whcopy;
+
+	whcopy = kmalloc(sizeof(*whcopy), GFP_KERNEL);
+	if (!whcopy)
+		return -ENOMEM;
+
+	memcpy(whcopy, wh, sizeof(*whcopy));
+	spin_lock(&dev_cgroup->lock);
+	list_add_tail(&whcopy->list, &dev_cgroup->whitelist);
+	spin_unlock(&dev_cgroup->lock);
+	return 0;
+}
+
+/*
+ * called under cgroup_lock()
+ * since the list is visible to other tasks, we need the spinlock also
+ */
+void dev_whitelist_rm(struct dev_cgroup *dev_cgroup,
+			struct dev_whitelist_item *wh)
+{
+	struct dev_whitelist_item *walk, *tmp;
+
+	spin_lock(&dev_cgroup->lock);
+	list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) {
+		if (walk->type == DEV_ALL)
+			goto remove;
+		if (walk->type != wh->type)
+			continue;
+		if (walk->major != ~0 && walk->major != wh->major)
+			continue;
+		if (walk->minor != ~0 && walk->minor != wh->minor)
+			continue;
+
+remove:
+		walk->access &= ~wh->access;
+		if (!walk->access) {
+			list_del(&walk->list);
+			kfree(walk);
+		}
+	}
+	spin_unlock(&dev_cgroup->lock);
+}
+
+/*
+ * called from kernel/cgroup.c with cgroup_lock() held.
+ */
+static struct cgroup_subsys_state *devcgroup_create(struct cgroup_subsys *ss,
+						struct cgroup *cgroup)
+{
+	struct dev_cgroup *dev_cgroup, *parent_dev_cgroup;
+
...

Re: [PATCH] cgroups: implement device whitelist (v4) [message #28419 is a reply to message #28417] Tue, 18 March 2008 04:17 Go to previous messageGo to next message
James Morris is currently offline  James Morris
Messages: 10
Registered: March 2006
Junior Member
On Mon, 17 Mar 2008, Serge E. Hallyn wrote:

> Implement a cgroup to track and enforce open and mknod restrictions on device
> files.  A device cgroup associates a device access whitelist with each
> cgroup.  A whitelist entry has 4 fields.  'type' is a (all), c (char), or
> b (block).  'all' means it applies to all types and all major and minor
> numbers.  Major and minor are either an integer or * for all.
> Access is a composition of r (read), w (write), and m (mknod).

Acked-by: James Morris <jmorris@namei.org>



-- 
James Morris
<jmorris@namei.org>
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
Re: [PATCH] cgroups: implement device whitelist (v4) [message #28421 is a reply to message #28417] Tue, 18 March 2008 05:15 Go to previous messageGo to next message
Li Zefan is currently offline  Li Zefan
Messages: 90
Registered: February 2008
Member
Serge E. Hallyn wrote:
> Implement a cgroup to track and enforce open and mknod restrictions on device
> files.  A device cgroup associates a device access whitelist with each
> cgroup.  A whitelist entry has 4 fields.  'type' is a (all), c (char), or
> b (block).  'all' means it applies to all types and all major and minor
> numbers.  Major and minor are either an integer or * for all.
> Access is a composition of r (read), w (write), and m (mknod).
> 
> The root device cgroup starts with rwm to 'all'.  A child devcg gets
> a copy of the parent.  Admins can then remove devices from the
> whitelist or add new entries.  A child cgroup can never receive a
> device access which is denied its parent.  However when a device
> access is removed from a parent it will not also be removed from the
> child(ren).
> 
> An entry is added using devices.allow, and removed using
> devices.deny.  For instance
> 
> 	echo 'c 1:3 mr' > /cgroups/1/devices.allow
> 
> allows cgroup 1 to read and mknod the device usually known as
> /dev/null.  Doing
> 
> 	echo a > /cgroups/1/devices.deny
> 
> will remove the default 'a *:* mrw' entry.
> 
> CAP_SYS_ADMIN is needed to change permissions or move another task
> to a new cgroup.  A cgroup may not be granted more permissions than
> the cgroup's parent has.  Any task can move itself between cgroups.
> This won't be sufficient, but we can decide the best way to
> adequately restrict movement later.
> 
> The parsing of devices.allow/deny needs to be cleaned up a bit and
> Documented.  I'd like to get an idea whether this approach is otherwise
> acceptable.
> 
> Changelog:
> 	Mar 17 2008: Place specific device cgroup hooks next to
> 		security_inode_{mknod,permission} rather than using
> 		the security hooks.
> 		Also remove most of the controls over tasks moving
> 		between cgroups and playing with the allow and deny
> 		permissions.
> 		Switch to major:minor format.
> 		Rename devcg to 'devices' to conform to cgroup naming.
> 	Mar 13 2008: move the dev_cgroup support into
> 		capability hooks instead of having it
> 		as a separate security module.
> 		Support root_plug with devcg.
> 		Note that due to this change, devcg will
> 		not be enforcing if the dummy module is
> 		loaded, or if selinux is loaded without
> 		capabilities.
> 	Mar 12 2008: allow dev_cgroup lsm to be used when
> 		SECURITY=n, and allow stacking with SELinux
> 		and Smack.  Don't work too hard in Kconfig
> 		to prevent a warning when smack+devcg are
> 		both compiled in, worry about that later.
> 

I would like to give some comments in the code. :)

> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> ---
>  fs/namei.c                    |    9 +
>  include/linux/cgroup_subsys.h |    6 +
>  include/linux/device_cgroup.h |   12 +
>  init/Kconfig                  |    7 +
>  security/Makefile             |    1 +
>  security/device_cgroup.c      |  597 +++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 632 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/device_cgroup.h
>  create mode 100644 security/device_cgroup.c
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index dfb3cb8..6caed32 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -30,6 +30,7 @@
>  #include <linux/capability.h>
>  #include <linux/file.h>
>  #include <linux/fcntl.h>
> +#include <linux/device_cgroup.h>
>  #include <asm/namei.h>
>  #include <asm/uaccess.h>
>  
> @@ -281,6 +282,10 @@ int permission(struct inode *inode, int mask, struct nameidata *nd)
>  	if (retval)
>  		return retval;
>  
> +	retval = devcgroup_inode_permission(inode, mask);
> +	if (retval)
> +		return retval;
> +
>  	return security_inode_permission(inode, mask, nd);
>  }
>  
> @@ -2028,6 +2033,10 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
>  	if (!dir->i_op || !dir->i_op->mknod)
>  		return -EPERM;
>  
> +	error = devcgroup_inode_mknod(mode, dev);
> +	if (error)
> +		return error;
> +
>  	error = security_inode_mknod(dir, dentry, mode, dev);
>  	if (error)
>  		return error;
> diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
> index 1ddebfc..e287745 100644
> --- a/include/linux/cgroup_subsys.h
> +++ b/include/linux/cgroup_subsys.h
> @@ -42,3 +42,9 @@ SUBSYS(mem_cgroup)
>  #endif
>  
>  /* */
> +
> +#ifdef CONFIG_CGROUP_DEVICE
> +SUBSYS(devices)
> +#endif
> +
> +/* */
> diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h
> new file mode 100644
> index 0000000..0b0d9c3
> --- /dev/null
> +++ b/include/linux/device_cgroup.h
> @@ -0,0 +1,12 @@
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +
> +#ifdef CONFIG_CGROUP_DEVICE
> +extern int devcgroup_inode_permission(struct inode *inode, int mask);
> +extern int devcgroup_inode_mknod(int mode, dev_t dev);
> +#else
> +static inline int devcgroup_inode_permission(struct inode *inode, int mask)
> +{ return 0; }
> +static inline int devcgroup_inode_mknod(int mode, dev_t dev)
> +{ return 0; }
> +#endif
> diff --git a/init/Kconfig b/init/Kconfig
> index 009f2d8..30868cd 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -298,6 +298,13 @@ config CGROUP_NS
>            for instance virtual servers and checkpoint/restart
>            jobs.
>  
> +config CGROUP_DEVICE
> +	bool "Device controller for cgroups"
> +	depends on CGROUPS && EXPERIMENTAL
> +	help
> +	  Provides a cgroup implementing whitelists for devices which
> +	  a process in the cgroup can mknod or open.
> +
>  config CPUSETS
>  	bool "Cpuset support"
>  	depends on SMP && CGROUPS
> diff --git a/security/Makefile b/security/Makefile
> index 9e8b025..7ef1107 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
>  obj-$(CONFIG_SECURITY_SMACK)		+= commoncap.o smack/built-in.o
>  obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
>  obj-$(CONFIG_SECURITY_ROOTPLUG)		+= commoncap.o root_plug.o
> +obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> new file mode 100644
> index 0000000..33d8fd8
> --- /dev/null
> +++ b/security/device_cgroup.c
> @@ -0,0 +1,597 @@
> +/*
> + * dev_cgroup.c - device cgroup subsystem
> + *
> + * Copyright 2007 IBM Corp
> + */
> +
> +#include <linux/device_cgroup.h>
> +#include <linux/cgroup.h>
> +#include <linux/ctype.h>
> +#include <linux/list.h>
> +#include <asm/uaccess.h>
> +
> +#define ACC_MKNOD 1
> +#define ACC_READ  2
> +#define ACC_WRITE 4
> +#define ACC_MASK (ACC_MKNOD | ACC_READ | ACC_WRITE)
> +
> +#define DEV_BLOCK 1
> +#define DEV_CHAR  2
> +#define DEV_ALL   4  /* this represents all devices */
> +
> +/*
> + * whitelist locking rules:
> + * cgroup_lock() cannot be taken under cgroup->lock.

When you say cgroup->lock, you mean dev_cgroup->lock, right?
So would it be better to make it clear in the comment?

> + * cgroup->lock can be taken with or without cgroup_lock().
> + *
> + * modifications always require cgroup_lock
> + * modifications to a list which is visible require the
> + *   cgroup->lock *and* cgroup_lock()
> + * walking the list requires cgroup->lock or cgroup_lock().
> + *
> + * reasoning: dev_whitelist_copy() needs to kmalloc, so needs
> + *   a mutex, which the cgroup_lock() is.  Since modifying
> + *   a visible list requires both locks, either lock can be
> + *   taken for walking the list.  Since the wh->spinlock is taken
> + *   for modifying a public-accessible list, the spinlock is
> + *   sufficient for just walking the list.
> + */
> +
> +struct dev_whitelist_item {
> +	u32 major, minor;
> +	short type;
> +	short access;
> +	struct list_head list;
> +};
> +
> +struct dev_cgroup {
> +	struct cgroup_subsys_state css;
> +	struct list_head whitelist;
> +	spinlock_t lock;
> +};
> +
> +static inline struct dev_cgroup *cgroup_to_devcgroup(
> +		struct cgroup *cgroup)
> +{
> +	return container_of(cgroup_subsys_state(cgroup, devices_subsys_id),
> +			    struct dev_cgroup, css);
> +}
> +
> +
> +struct cgroup_subsys devices_subsys;
> +
> +static int devcgroup_can_attach(struct cgroup_subsys *ss,
> +		struct cgroup *new_cgroup, struct task_struct *task)
> +{
> +

redundant empty line

> +	if (current != task && !capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +	return 0;
> +}
> +
> +/*
> + * called under cgroup_lock()
> + */
> +int dev_whitelist_copy(struct list_head *dest, struct list_head *orig)

static int

> +{
> +	struct dev_whitelist_item *wh, *tmp, *new;
> +
> +	list_for_each_entry(wh, orig, list) {
> +		new = kmalloc(sizeof(*wh), GFP_KERNEL);
> +		if (!new)
> +			goto free_and_exit;
> +		new->major = wh->major;
> +		new->minor = wh->minor;
> +		new->type = wh->type;
> +		new->access = wh->access;
> +		list_add_tail(&new->list, dest);
> +	}
> +
> +	return 0;
> +
> +free_and_exit:
> +	list_for_each_entry_safe(wh, tmp, dest, list) {
> +		list_del(&wh->list);
> +		kfree(wh);
> +	}
> +	return -ENOMEM;
> +}
> +
> +/* Stupid prototype - don't bother combining existing entrie
...

Re: [PATCH] cgroups: implement device whitelist (v4) [message #28424 is a reply to message #28417] Tue, 18 March 2008 07:57 Go to previous messageGo to next message
Pavel Emelianov is currently offline  Pavel Emelianov
Messages: 1149
Registered: September 2006
Senior Member
Serge E. Hallyn wrote:
> Implement a cgroup to track and enforce open and mknod restrictions on device
> files.  A device cgroup associates a device access whitelist with each
> cgroup.  A whitelist entry has 4 fields.  'type' is a (all), c (char), or
> b (block).  'all' means it applies to all types and all major and minor
> numbers.  Major and minor are either an integer or * for all.
> Access is a composition of r (read), w (write), and m (mknod).
> 
> The root device cgroup starts with rwm to 'all'.  A child devcg gets
> a copy of the parent.  Admins can then remove devices from the
> whitelist or add new entries.  A child cgroup can never receive a
> device access which is denied its parent.  However when a device
> access is removed from a parent it will not also be removed from the
> child(ren).
> 
> An entry is added using devices.allow, and removed using
> devices.deny.  For instance
> 
> 	echo 'c 1:3 mr' > /cgroups/1/devices.allow
> 
> allows cgroup 1 to read and mknod the device usually known as
> /dev/null.  Doing
> 
> 	echo a > /cgroups/1/devices.deny
> 
> will remove the default 'a *:* mrw' entry.
> 
> CAP_SYS_ADMIN is needed to change permissions or move another task
> to a new cgroup.  A cgroup may not be granted more permissions than
> the cgroup's parent has.  Any task can move itself between cgroups.
> This won't be sufficient, but we can decide the best way to
> adequately restrict movement later.
> 
> The parsing of devices.allow/deny needs to be cleaned up a bit and
> Documented.  I'd like to get an idea whether this approach is otherwise
> acceptable.
> 
> Changelog:
> 	Mar 17 2008: Place specific device cgroup hooks next to
> 		security_inode_{mknod,permission} rather than using
> 		the security hooks.
> 		Also remove most of the controls over tasks moving
> 		between cgroups and playing with the allow and deny
> 		permissions.
> 		Switch to major:minor format.
> 		Rename devcg to 'devices' to conform to cgroup naming.
> 	Mar 13 2008: move the dev_cgroup support into
> 		capability hooks instead of having it
> 		as a separate security module.
> 		Support root_plug with devcg.
> 		Note that due to this change, devcg will
> 		not be enforcing if the dummy module is
> 		loaded, or if selinux is loaded without
> 		capabilities.
> 	Mar 12 2008: allow dev_cgroup lsm to be used when
> 		SECURITY=n, and allow stacking with SELinux
> 		and Smack.  Don't work too hard in Kconfig
> 		to prevent a warning when smack+devcg are
> 		both compiled in, worry about that later.
> 
> Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

Looks-good-to: Pavel Emelyanov <xemul@openvz.org>

> ---
>  fs/namei.c                    |    9 +
>  include/linux/cgroup_subsys.h |    6 +
>  include/linux/device_cgroup.h |   12 +
>  init/Kconfig                  |    7 +
>  security/Makefile             |    1 +
>  security/device_cgroup.c      |  597 +++++++++++++++++++++++++++++++++++++++++
>  6 files changed, 632 insertions(+), 0 deletions(-)
>  create mode 100644 include/linux/device_cgroup.h
>  create mode 100644 security/device_cgroup.c
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index dfb3cb8..6caed32 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -30,6 +30,7 @@
>  #include <linux/capability.h>
>  #include <linux/file.h>
>  #include <linux/fcntl.h>
> +#include <linux/device_cgroup.h>
>  #include <asm/namei.h>
>  #include <asm/uaccess.h>
>  
> @@ -281,6 +282,10 @@ int permission(struct inode *inode, int mask, struct nameidata *nd)
>  	if (retval)
>  		return retval;
>  
> +	retval = devcgroup_inode_permission(inode, mask);
> +	if (retval)
> +		return retval;
> +
>  	return security_inode_permission(inode, mask, nd);
>  }
>  
> @@ -2028,6 +2033,10 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
>  	if (!dir->i_op || !dir->i_op->mknod)
>  		return -EPERM;
>  
> +	error = devcgroup_inode_mknod(mode, dev);
> +	if (error)
> +		return error;
> +
>  	error = security_inode_mknod(dir, dentry, mode, dev);
>  	if (error)
>  		return error;
> diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
> index 1ddebfc..e287745 100644
> --- a/include/linux/cgroup_subsys.h
> +++ b/include/linux/cgroup_subsys.h
> @@ -42,3 +42,9 @@ SUBSYS(mem_cgroup)
>  #endif
>  
>  /* */
> +
> +#ifdef CONFIG_CGROUP_DEVICE
> +SUBSYS(devices)
> +#endif
> +
> +/* */
> diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h
> new file mode 100644
> index 0000000..0b0d9c3
> --- /dev/null
> +++ b/include/linux/device_cgroup.h
> @@ -0,0 +1,12 @@
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +
> +#ifdef CONFIG_CGROUP_DEVICE
> +extern int devcgroup_inode_permission(struct inode *inode, int mask);
> +extern int devcgroup_inode_mknod(int mode, dev_t dev);
> +#else
> +static inline int devcgroup_inode_permission(struct inode *inode, int mask)
> +{ return 0; }
> +static inline int devcgroup_inode_mknod(int mode, dev_t dev)
> +{ return 0; }
> +#endif
> diff --git a/init/Kconfig b/init/Kconfig
> index 009f2d8..30868cd 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -298,6 +298,13 @@ config CGROUP_NS
>            for instance virtual servers and checkpoint/restart
>            jobs.
>  
> +config CGROUP_DEVICE
> +	bool "Device controller for cgroups"
> +	depends on CGROUPS && EXPERIMENTAL
> +	help
> +	  Provides a cgroup implementing whitelists for devices which
> +	  a process in the cgroup can mknod or open.
> +
>  config CPUSETS
>  	bool "Cpuset support"
>  	depends on SMP && CGROUPS
> diff --git a/security/Makefile b/security/Makefile
> index 9e8b025..7ef1107 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -18,3 +18,4 @@ obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
>  obj-$(CONFIG_SECURITY_SMACK)		+= commoncap.o smack/built-in.o
>  obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
>  obj-$(CONFIG_SECURITY_ROOTPLUG)		+= commoncap.o root_plug.o
> +obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
> diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> new file mode 100644
> index 0000000..33d8fd8
> --- /dev/null
> +++ b/security/device_cgroup.c
> @@ -0,0 +1,597 @@
> +/*
> + * dev_cgroup.c - device cgroup subsystem
> + *
> + * Copyright 2007 IBM Corp
> + */
> +
> +#include <linux/device_cgroup.h>
> +#include <linux/cgroup.h>
> +#include <linux/ctype.h>
> +#include <linux/list.h>
> +#include <asm/uaccess.h>
> +
> +#define ACC_MKNOD 1
> +#define ACC_READ  2
> +#define ACC_WRITE 4
> +#define ACC_MASK (ACC_MKNOD | ACC_READ | ACC_WRITE)
> +
> +#define DEV_BLOCK 1
> +#define DEV_CHAR  2
> +#define DEV_ALL   4  /* this represents all devices */
> +
> +/*
> + * whitelist locking rules:
> + * cgroup_lock() cannot be taken under cgroup->lock.
> + * cgroup->lock can be taken with or without cgroup_lock().
> + *
> + * modifications always require cgroup_lock
> + * modifications to a list which is visible require the
> + *   cgroup->lock *and* cgroup_lock()
> + * walking the list requires cgroup->lock or cgroup_lock().
> + *
> + * reasoning: dev_whitelist_copy() needs to kmalloc, so needs
> + *   a mutex, which the cgroup_lock() is.  Since modifying
> + *   a visible list requires both locks, either lock can be
> + *   taken for walking the list.  Since the wh->spinlock is taken
> + *   for modifying a public-accessible list, the spinlock is
> + *   sufficient for just walking the list.
> + */
> +
> +struct dev_whitelist_item {
> +	u32 major, minor;
> +	short type;
> +	short access;
> +	struct list_head list;
> +};
> +
> +struct dev_cgroup {
> +	struct cgroup_subsys_state css;
> +	struct list_head whitelist;
> +	spinlock_t lock;
> +};
> +
> +static inline struct dev_cgroup *cgroup_to_devcgroup(
> +		struct cgroup *cgroup)
> +{
> +	return container_of(cgroup_subsys_state(cgroup, devices_subsys_id),
> +			    struct dev_cgroup, css);
> +}
> +
> +
> +struct cgroup_subsys devices_subsys;
> +
> +static int devcgroup_can_attach(struct cgroup_subsys *ss,
> +		struct cgroup *new_cgroup, struct task_struct *task)
> +{
> +
> +	if (current != task && !capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +	return 0;
> +}
> +
> +/*
> + * called under cgroup_lock()
> + */
> +int dev_whitelist_copy(struct list_head *dest, struct list_head *orig)
> +{
> +	struct dev_whitelist_item *wh, *tmp, *new;
> +
> +	list_for_each_entry(wh, orig, list) {
> +		new = kmalloc(sizeof(*wh), GFP_KERNEL);
> +		if (!new)
> +			goto free_and_exit;
> +		new->major = wh->major;
> +		new->minor = wh->minor;
> +		new->type = wh->type;
> +		new->access = wh->access;
> +		list_add_tail(&new->list, dest);
> +	}
> +
> +	return 0;
> +
> +free_and_exit:
> +	list_for_each_entry_safe(wh, tmp, dest, list) {
> +		list_del(&wh->list);
> +		kfree(wh);
> +	}
> +	return -ENOMEM;
> +}
> +
> +/* Stupid prototype - don't bother combining existing entries */
> +/*
> + * called under cgroup_lock()
> + * since the list is visible to other tasks, we need the spinlock also
> + */
> +int dev_whit
...

Re: [PATCH] cgroups: implement device whitelist (v4) [message #28444 is a reply to message #28421] Tue, 18 March 2008 14:10 Go to previous message
serue is currently offline  serue
Messages: 750
Registered: February 2006
Senior Member
Quoting Li Zefan (lizf@cn.fujitsu.com):
> Serge E. Hallyn wrote:
> > Implement a cgroup to track and enforce open and mknod restrictions on device
> > files.  A device cgroup associates a device access whitelist with each
> > cgroup.  A whitelist entry has 4 fields.  'type' is a (all), c (char), or
> > b (block).  'all' means it applies to all types and all major and minor
> > numbers.  Major and minor are either an integer or * for all.
> > Access is a composition of r (read), w (write), and m (mknod).
> > 
> > The root device cgroup starts with rwm to 'all'.  A child devcg gets
> > a copy of the parent.  Admins can then remove devices from the
> > whitelist or add new entries.  A child cgroup can never receive a
> > device access which is denied its parent.  However when a device
> > access is removed from a parent it will not also be removed from the
> > child(ren).
> > 
> > An entry is added using devices.allow, and removed using
> > devices.deny.  For instance
> > 
> > 	echo 'c 1:3 mr' > /cgroups/1/devices.allow
> > 
> > allows cgroup 1 to read and mknod the device usually known as
> > /dev/null.  Doing
> > 
> > 	echo a > /cgroups/1/devices.deny
> > 
> > will remove the default 'a *:* mrw' entry.
> > 
> > CAP_SYS_ADMIN is needed to change permissions or move another task
> > to a new cgroup.  A cgroup may not be granted more permissions than
> > the cgroup's parent has.  Any task can move itself between cgroups.
> > This won't be sufficient, but we can decide the best way to
> > adequately restrict movement later.
> > 
> > The parsing of devices.allow/deny needs to be cleaned up a bit and
> > Documented.  I'd like to get an idea whether this approach is otherwise
> > acceptable.
> > 
> > Changelog:
> > 	Mar 17 2008: Place specific device cgroup hooks next to
> > 		security_inode_{mknod,permission} rather than using
> > 		the security hooks.
> > 		Also remove most of the controls over tasks moving
> > 		between cgroups and playing with the allow and deny
> > 		permissions.
> > 		Switch to major:minor format.
> > 		Rename devcg to 'devices' to conform to cgroup naming.
> > 	Mar 13 2008: move the dev_cgroup support into
> > 		capability hooks instead of having it
> > 		as a separate security module.
> > 		Support root_plug with devcg.
> > 		Note that due to this change, devcg will
> > 		not be enforcing if the dummy module is
> > 		loaded, or if selinux is loaded without
> > 		capabilities.
> > 	Mar 12 2008: allow dev_cgroup lsm to be used when
> > 		SECURITY=n, and allow stacking with SELinux
> > 		and Smack.  Don't work too hard in Kconfig
> > 		to prevent a warning when smack+devcg are
> > 		both compiled in, worry about that later.
> > 
> 
> I would like to give some comments in the code. :)

Thanks for taking a look.

> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> >  fs/namei.c                    |    9 +
> >  include/linux/cgroup_subsys.h |    6 +
> >  include/linux/device_cgroup.h |   12 +
> >  init/Kconfig                  |    7 +
> >  security/Makefile             |    1 +
> >  security/device_cgroup.c      |  597 +++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 632 insertions(+), 0 deletions(-)
> >  create mode 100644 include/linux/device_cgroup.h
> >  create mode 100644 security/device_cgroup.c
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index dfb3cb8..6caed32 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -30,6 +30,7 @@
> >  #include <linux/capability.h>
> >  #include <linux/file.h>
> >  #include <linux/fcntl.h>
> > +#include <linux/device_cgroup.h>
> >  #include <asm/namei.h>
> >  #include <asm/uaccess.h>
> >  
> > @@ -281,6 +282,10 @@ int permission(struct inode *inode, int mask, struct nameidata *nd)
> >  	if (retval)
> >  		return retval;
> >  
> > +	retval = devcgroup_inode_permission(inode, mask);
> > +	if (retval)
> > +		return retval;
> > +
> >  	return security_inode_permission(inode, mask, nd);
> >  }
> >  
> > @@ -2028,6 +2033,10 @@ int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
> >  	if (!dir->i_op || !dir->i_op->mknod)
> >  		return -EPERM;
> >  
> > +	error = devcgroup_inode_mknod(mode, dev);
> > +	if (error)
> > +		return error;
> > +
> >  	error = security_inode_mknod(dir, dentry, mode, dev);
> >  	if (error)
> >  		return error;
> > diff --git a/include/linux/cgroup_subsys.h b/include/linux/cgroup_subsys.h
> > index 1ddebfc..e287745 100644
> > --- a/include/linux/cgroup_subsys.h
> > +++ b/include/linux/cgroup_subsys.h
> > @@ -42,3 +42,9 @@ SUBSYS(mem_cgroup)
> >  #endif
> >  
> >  /* */
> > +
> > +#ifdef CONFIG_CGROUP_DEVICE
> > +SUBSYS(devices)
> > +#endif
> > +
> > +/* */
> > diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h
> > new file mode 100644
> > index 0000000..0b0d9c3
> > --- /dev/null
> > +++ b/include/linux/device_cgroup.h
> > @@ -0,0 +1,12 @@
> > +#include <linux/module.h>
> > +#include <linux/fs.h>
> > +
> > +#ifdef CONFIG_CGROUP_DEVICE
> > +extern int devcgroup_inode_permission(struct inode *inode, int mask);
> > +extern int devcgroup_inode_mknod(int mode, dev_t dev);
> > +#else
> > +static inline int devcgroup_inode_permission(struct inode *inode, int mask)
> > +{ return 0; }
> > +static inline int devcgroup_inode_mknod(int mode, dev_t dev)
> > +{ return 0; }
> > +#endif
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 009f2d8..30868cd 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -298,6 +298,13 @@ config CGROUP_NS
> >            for instance virtual servers and checkpoint/restart
> >            jobs.
> >  
> > +config CGROUP_DEVICE
> > +	bool "Device controller for cgroups"
> > +	depends on CGROUPS && EXPERIMENTAL
> > +	help
> > +	  Provides a cgroup implementing whitelists for devices which
> > +	  a process in the cgroup can mknod or open.
> > +
> >  config CPUSETS
> >  	bool "Cpuset support"
> >  	depends on SMP && CGROUPS
> > diff --git a/security/Makefile b/security/Makefile
> > index 9e8b025..7ef1107 100644
> > --- a/security/Makefile
> > +++ b/security/Makefile
> > @@ -18,3 +18,4 @@ obj-$(CONFIG_SECURITY_SELINUX)		+= selinux/built-in.o
> >  obj-$(CONFIG_SECURITY_SMACK)		+= commoncap.o smack/built-in.o
> >  obj-$(CONFIG_SECURITY_CAPABILITIES)	+= commoncap.o capability.o
> >  obj-$(CONFIG_SECURITY_ROOTPLUG)		+= commoncap.o root_plug.o
> > +obj-$(CONFIG_CGROUP_DEVICE)		+= device_cgroup.o
> > diff --git a/security/device_cgroup.c b/security/device_cgroup.c
> > new file mode 100644
> > index 0000000..33d8fd8
> > --- /dev/null
> > +++ b/security/device_cgroup.c
> > @@ -0,0 +1,597 @@
> > +/*
> > + * dev_cgroup.c - device cgroup subsystem
> > + *
> > + * Copyright 2007 IBM Corp
> > + */
> > +
> > +#include <linux/device_cgroup.h>
> > +#include <linux/cgroup.h>
> > +#include <linux/ctype.h>
> > +#include <linux/list.h>
> > +#include <asm/uaccess.h>
> > +
> > +#define ACC_MKNOD 1
> > +#define ACC_READ  2
> > +#define ACC_WRITE 4
> > +#define ACC_MASK (ACC_MKNOD | ACC_READ | ACC_WRITE)
> > +
> > +#define DEV_BLOCK 1
> > +#define DEV_CHAR  2
> > +#define DEV_ALL   4  /* this represents all devices */
> > +
> > +/*
> > + * whitelist locking rules:
> > + * cgroup_lock() cannot be taken under cgroup->lock.
> 
> When you say cgroup->lock, you mean dev_cgroup->lock, right?
> So would it be better to make it clear in the comment?

Yes.

> > + * cgroup->lock can be taken with or without cgroup_lock().
> > + *
> > + * modifications always require cgroup_lock
> > + * modifications to a list which is visible require the
> > + *   cgroup->lock *and* cgroup_lock()
> > + * walking the list requires cgroup->lock or cgroup_lock().
> > + *
> > + * reasoning: dev_whitelist_copy() needs to kmalloc, so needs
> > + *   a mutex, which the cgroup_lock() is.  Since modifying
> > + *   a visible list requires both locks, either lock can be
> > + *   taken for walking the list.  Since the wh->spinlock is taken
> > + *   for modifying a public-accessible list, the spinlock is
> > + *   sufficient for just walking the list.
> > + */
> > +
> > +struct dev_whitelist_item {
> > +	u32 major, minor;
> > +	short type;
> > +	short access;
> > +	struct list_head list;
> > +};
> > +
> > +struct dev_cgroup {
> > +	struct cgroup_subsys_state css;
> > +	struct list_head whitelist;
> > +	spinlock_t lock;
> > +};
> > +
> > +static inline struct dev_cgroup *cgroup_to_devcgroup(
> > +		struct cgroup *cgroup)
> > +{
> > +	return container_of(cgroup_subsys_state(cgroup, devices_subsys_id),
...

Previous Topic: [PATCH] cgroups: implement device whitelist lsm (v3)
Next Topic: [PATCH O/4] Block I/O tracking
Goto Forum:
  


Current Time: Tue Jul 16 19:54:04 GMT 2024

Total time taken to generate the page: 0.02983 seconds