[lxc-devel] [PATCH 2/5] cgroup: don't set devices cgroup if not in init_user_ns (v2)

[serge.hallyn at ubuntu.com]

From: Serge Hallyn <serge.hallyn at ubuntu.com>

Changelog: (Jul 22) only do this when actually mapping ids

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/cgroup.c | 11 ++++++++++-
 src/lxc/conf.c   | 16 ++++++++++------
 src/lxc/conf.h   |  6 ++++--
 src/lxc/lxc.h    |  4 +++-
 src/lxc/start.c  |  4 ++--
 5 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index c707519..d75c064 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -286,12 +286,21 @@ static int do_cgroup_set(const char *path, const char *value)
  *
  * Returns 0 on success, < 0 on error.
  */
-int lxc_cgroup_set_bypath(const char *cgrelpath, const char *filename, const char *value)
+int lxc_cgroup_set_bypath(const char *cgrelpath, const char *filename,
+			  const char *value, int in_userns)
 {
 	int ret;
 	char *cgabspath;
 	char path[MAXPATHLEN];
 
+	/*
+	 * exception: if in a user namespace, don't try to set devices cgroup.
+	 * (a) a task in non-init userns can't use most devices anyway, and
+	 * (b) a task in non-init userns cannot write to devices.{allow,deny}
+	 */
+	if (in_userns && strncmp(filename, "devices.", 8) == 0)
+		return 0;
+
 	cgabspath = cgroup_path_get(filename, cgrelpath);
 	if (!cgabspath)
 		return -1;
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index f7ac030..b4cace5 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -1428,21 +1428,25 @@ static int setup_kmsg(const struct lxc_rootfs *rootfs,
 }
 
 static int _setup_cgroup(const char *cgpath, struct lxc_list *cgroups,
-			  int devices)
+			  struct lxc_conf *conf, int devices)
 {
 	struct lxc_list *iterator;
 	struct lxc_cgroup *cg;
 	int ret = -1;
+	int in_userns = 0;
 
 	if (lxc_list_empty(cgroups))
 		return 0;
 
+	if (!lxc_list_empty(&conf->id_map))
+		in_userns = 1;
+
 	lxc_list_for_each(iterator, cgroups) {
 		cg = iterator->elem;
 
 		if (devices == !strncmp("devices", cg->subsystem, 7)) {
 			if (lxc_cgroup_set_bypath(cgpath, cg->subsystem,
-			    cg->value)) {
+			    cg->value, in_userns)) {
 				ERROR("Error setting %s to %s for %s\n",
 				      cg->subsystem, cg->value, cgpath);
 				goto out;
@@ -1458,14 +1462,14 @@ out:
 	return ret;
 }
 
-int setup_cgroup_devices(const char *cgpath, struct lxc_list *cgroups)
+int setup_cgroup_devices(const char *cgpath, struct lxc_list *cgroups, struct lxc_conf *conf)
 {
-	return _setup_cgroup(cgpath, cgroups, 1);
+	return _setup_cgroup(cgpath, cgroups, conf, 1);
 }
 
-int setup_cgroup(const char *cgpath, struct lxc_list *cgroups)
+int setup_cgroup(const char *cgpath, struct lxc_list *cgroups, struct lxc_conf *conf)
 {
-	return _setup_cgroup(cgpath, cgroups, 0);
+	return _setup_cgroup(cgpath, cgroups, conf, 0);
 }
 
 static void parse_mntopt(char *opt, unsigned long *flags, char **data)
diff --git a/src/lxc/conf.h b/src/lxc/conf.h
index 065b1df..90daa46 100644
--- a/src/lxc/conf.h
+++ b/src/lxc/conf.h
@@ -293,8 +293,10 @@ struct lxc_conf {
 int run_lxc_hooks(const char *name, char *hook, struct lxc_conf *conf,
 		  const char *lxcpath, char *argv[]);
 
-extern int setup_cgroup(const char *cgpath, struct lxc_list *cgroups);
-extern int setup_cgroup_devices(const char *cgpath, struct lxc_list *cgroups);
+extern int setup_cgroup(const char *cgpath, struct lxc_list *cgroups,
+			struct lxc_conf *conf);
+extern int setup_cgroup_devices(const char *cgpath, struct lxc_list *cgroups,
+			struct lxc_conf *conf);
 extern int detect_shared_rootfs(void);
 
 /*
diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
index 67e0d9e..8170eef 100644
--- a/src/lxc/lxc.h
+++ b/src/lxc/lxc.h
@@ -142,9 +142,11 @@ extern lxc_state_t lxc_state(const char *name, const char *lxcpath);
  * @cgpath    : the cgroup path of the container
  * @filename  : the cgroup attribute filename
  * @value     : the value to be set
+ * @in_userns : whether container is in a userns
  * Returns 0 on success, < 0 otherwise
  */
-extern int lxc_cgroup_set_bypath(const char *cgpath, const char *filename, const char *value);
+extern int lxc_cgroup_set_bypath(const char *cgpath, const char *filename, const char *value,
+			int in_userns);
 
 /*
  * Set a specified value for a specified subsystem. The specified
diff --git a/src/lxc/start.c b/src/lxc/start.c
index 00020de..00af884 100644
--- a/src/lxc/start.c
+++ b/src/lxc/start.c
@@ -672,7 +672,7 @@ int lxc_spawn(struct lxc_handler *handler)
 	if ((handler->cgroup = lxc_cgroup_path_create(curcgroup, name)) == NULL)
 		goto out_delete_net;
 
-	if (setup_cgroup(handler->cgroup, &handler->conf->cgroup)) {
+	if (setup_cgroup(handler->cgroup, &handler->conf->cgroup, handler->conf)) {
 		ERROR("failed to setup the cgroups for '%s'", name);
 		goto out_delete_net;
 	}
@@ -707,7 +707,7 @@ int lxc_spawn(struct lxc_handler *handler)
 	if (lxc_sync_barrier_child(handler, LXC_SYNC_POST_CONFIGURE))
 		goto out_delete_net;
 
-	if (setup_cgroup_devices(handler->cgroup, &handler->conf->cgroup)) {
+	if (setup_cgroup_devices(handler->cgroup, &handler->conf->cgroup, handler->conf)) {
 		/* an unfortunate special case: startup hooks may have already
 		 * setup the cgroup.  If a setting fails, and this is the devices
 		 * subsystem, *and* we are already in a subset of the cgroup,
-- 
1.8.1.2