[lxc-devel] [lxc/master] fix cgfs failure for unpriv users

hallyn on Github lxc-bot at linuxcontainers.org
Fri Feb 26 20:11:49 UTC 2016


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 955 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20160226/23d27434/attachment.bin>
-------------- next part --------------
From 191b86778d1277228eaeab861e50c1ee8f601b42 Mon Sep 17 00:00:00 2001
From: Serge Hallyn <serge.hallyn at ubuntu.com>
Date: Fri, 26 Feb 2016 20:03:09 +0000
Subject: [PATCH] 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 at ubuntu.com>
---
 src/lxc/cgfs.c      | 25 ++++++++++++++++++++-----
 src/lxc/cgmanager.c | 15 ---------------
 src/lxc/cgroup.c    | 15 +++++++++++++++
 src/lxc/cgroup.h    |  1 +
 4 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index e2d8934..8c1cb64 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 9f83a6e..8206a51 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;
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 8954733..8ee0629 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 e3d3ce4..63dcc92 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


More information about the lxc-devel mailing list