From: Serge Hallyn <serge.hallyn@ubuntu.com>
Date: Fri, 26 Feb 2016 20:03:09 +0000 (+0000)
Subject: fix cgfs failure for unpriv users
X-Git-Tag: lxc-2.0.0.rc4~2^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F855%2Fhead;p=thirdparty%2Flxc.git

fix cgfs failure for unpriv users

Cgmanager was taught awhile ago that only some cgroup controllers are
crucial.  Teach cgfs the same thing.

This patch needs improvement, but will fix failure of lxc without cgmanager
for unprivileged users for now.  In particular, needed improvements include:

1. the check for crucial subsystems needs to include lxc.use
2. we should keep a list of the actually used subsystems so we don't keep
trying to chmod and enter after create has found we couldn't use a particular
subsystem

This fixes unprivileged lxc use.  It does not appear to suffice to fix
nested unprivilegd lxd usage.

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
---

diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index e2d893465..8c1cb64ea 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -807,6 +807,17 @@ static char *cgroup_rename_nsgroup(const char *mountpath, const char *oldname, p
 	return newname;
 }
 
+static bool is_crucial_hierarchy(struct cgroup_hierarchy *h)
+{
+	char **p;
+
+	for (p = h->subsystems; *p; p++) {
+		if (is_crucial_cgroup_subsystem(*p))
+			return true;
+	}
+	return false;
+}
+
 /* create a new cgroup */
 static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const char *path_pattern, struct cgroup_meta_data *meta_data, const char *sub_pattern)
 {
@@ -974,8 +985,11 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
 				current_entire_path = NULL;
 				goto cleanup_name_on_this_level;
 			} else if (r < 0 && errno != EEXIST) {
-				SYSERROR("Could not create cgroup '%s' in '%s'.", current_entire_path, info_ptr->designated_mount_point->mount_point);
-				goto cleanup_from_error;
+				if (is_crucial_hierarchy(info_ptr->hierarchy)) {
+					SYSERROR("Could not create cgroup '%s' in '%s'.", current_entire_path, info_ptr->designated_mount_point->mount_point);
+					goto cleanup_from_error;
+				}
+				goto skip;
 			} else if (r == 0) {
 				/* successfully created */
 				r = lxc_grow_array((void ***)&info_ptr->created_paths, &info_ptr->created_paths_capacity, info_ptr->created_paths_count + 1, 8);
@@ -999,6 +1013,7 @@ static struct cgroup_process_info *lxc_cgroupfs_create(const char *name, const c
 					goto cleanup_from_error;
 				}
 
+skip:
 				/* already existed but path component of pattern didn't contain '%n',
 				 * so this is not an error; but then we don't need current_entire_path
 				 * anymore...
@@ -1180,7 +1195,7 @@ static int lxc_cgroupfs_enter(struct cgroup_process_info *info, pid_t pid, bool
 
 		r = lxc_write_to_file(cgroup_tasks_fn, pid_buf, strlen(pid_buf), false);
 		free(cgroup_tasks_fn);
-		if (r < 0) {
+		if (r < 0 && is_crucial_hierarchy(info_ptr->hierarchy)) {
 			SYSERROR("Could not add pid %lu to cgroup %s: internal error", (unsigned long)pid, cgroup_path);
 			return -1;
 		}
@@ -1509,7 +1524,7 @@ static bool cgroupfs_mount_cgroup(void *hdata, const char *root, int type)
 			if (!abs_path)
 				goto out_error;
 			r = mount(abs_path, abs_path2, "none", MS_BIND, 0);
-			if (r < 0) {
+			if (r < 0 && is_crucial_hierarchy(info->hierarchy)) {
 				SYSERROR("error bind-mounting %s to %s", abs_path, abs_path2);
 				goto out_error;
 			}
@@ -2600,7 +2615,7 @@ static bool cgfs_chown(void *hdata, struct lxc_conf *conf)
 			continue;
 		}
 		r = do_cgfs_chown(cgpath, conf);
-		if (!r) {
+		if (!r && is_crucial_hierarchy(info_ptr->hierarchy)) {
 			ERROR("Failed chowning %s\n", cgpath);
 			free(cgpath);
 			return false;
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 9f83a6e63..937c36ed1 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -1242,21 +1242,6 @@ static bool subsys_is_writeable(const char *controller, const char *probe)
 	return ret;
 }
 
-/*
- * Return true if this is a subsystem which we cannot do
- * without
- */
-static bool is_crucial_subsys(const char *s)
-{
-	if (strcmp(s, "systemd") == 0)
-		return true;
-	if (strcmp(s, "name=systemd") == 0)
-		return true;
-	if (strcmp(s, "freezer") == 0)
-		return true;
-	return false;
-}
-
 static char *get_last_controller_in_list(char *list)
 {
 	char *p;
@@ -1302,7 +1287,7 @@ static bool verify_final_subsystems(const char *cgroup_use)
 		char *p = get_last_controller_in_list(subsystems[i]);
 
 		if (!subsys_is_writeable(p, probe)) {
-			if (is_crucial_subsys(p)) {
+			if (is_crucial_cgroup_subsystem(p)) {
 				ERROR("Cannot write to crucial subsystem %s\n",
 					subsystems[i]);
 				goto out;
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 8954733c1..8ee0629a1 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -220,3 +220,18 @@ void prune_init_scope(char *cg)
 			*point = '\0';
 	}
 }
+
+/*
+ * Return true if this is a subsystem which we cannot do
+ * without
+ */
+bool is_crucial_cgroup_subsystem(const char *s)
+{
+	if (strcmp(s, "systemd") == 0)
+		return true;
+	if (strcmp(s, "name=systemd") == 0)
+		return true;
+	if (strcmp(s, "freezer") == 0)
+		return true;
+	return false;
+}
diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index e3d3ce4cc..63dcc9254 100644
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -83,5 +83,6 @@ extern void cgroup_disconnect(void);
 extern cgroup_driver_t cgroup_driver(void);
 
 extern void prune_init_scope(char *cg);
+extern bool is_crucial_cgroup_subsystem(const char *s);
 
 #endif