[lxc-devel] [PATCH] cgmanager: several fixes

Serge Hallyn serge.hallyn at ubuntu.com
Wed Oct 8 05:28:42 UTC 2014


These all fix various ways that cgroup actions could fail if an
unprivileged user's cgroup paths were not all the same for all
controllers.

1. in cgm_{g,s}et, use the right controller, not the first in the list,
   to get the cgroup path.

2. when we pass 'all' to cgmanager for a ${METHOD}_abs, make sure that all
   cgroup paths are the same.  That isn't necessary for methods not
   taking an absolute path, so split up the former
   cgm_supports_multiple_controllers() function into two booleans, one
   telling whether cgm supports it, and another telling us whether
   cgm supports it AND all controller cgroup paths are the same.

3. separately, do_cgm_enter with abs=true couldn't work if all
   cgroup paths were not the same.  So just ditch that helper and
   call lxc_cgmanager_enter() where needed, because the special
   cases would be more complicated.

This patches fixes at least lxc-attach and lxc-cgroup in cases where
the unpriv user's cgroups paths are not all the same.

Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
---
 src/lxc/cgmanager.c | 141 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 100 insertions(+), 41 deletions(-)

diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index 4038c41..0aad9e5 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -182,9 +182,60 @@ static bool cgm_dbus_connect(void)
 	return true;
 }
 
-static inline bool cgm_supports_multiple_controllers(void)
+static bool cgm_supports_multiple_controllers;
+/*
+ * if cgm_all_controllers_same is true, then cgm_supports_multiple_controllers
+ * is true
+ */
+static bool cgm_all_controllers_same;
+
+static void check_supports_multiple_controllers(void)
 {
-	return api_version >= CGM_SUPPORTS_MULT_CONTROLLERS;
+	FILE *f;
+	char *line = NULL, *prevpath = NULL;
+	size_t sz = 0;
+
+	cgm_supports_multiple_controllers = false;
+	cgm_all_controllers_same = false;
+
+	if (api_version < CGM_SUPPORTS_MULT_CONTROLLERS) {
+		cgm_supports_multiple_controllers = false;
+		return;
+	}
+
+	cgm_supports_multiple_controllers = true;
+
+	f = fopen("/proc/self/cgroup", "r");
+	if (!f)
+		return;
+
+	cgm_all_controllers_same = true;
+
+	while (getline(&line, &sz, f) != -1) {
+		/* file format: hierarchy:subsystems:group */
+		char *colon;
+		if (!line[0])
+			continue;
+
+		colon = strchr(line, ':');
+		if (!colon)
+			continue;
+		colon = strchr(colon+1, ':');
+		if (!colon)
+			continue;
+		colon++;
+		if (!prevpath) {
+			prevpath = alloca(strlen(colon)+1);
+			strcpy(prevpath, colon);
+			continue;
+		}
+		if (strcmp(prevpath, colon) != 0) {
+			cgm_all_controllers_same = false;
+			fclose(f);
+			return;
+		}
+	}
+	fclose(f);
 }
 
 static int send_creds(int sock, int rpid, int ruid, int rgid)
@@ -251,7 +302,7 @@ static bool lxc_cgmanager_escape(void)
 	char **slist = subsystems;
 	int i;
 
-	if (cgm_supports_multiple_controllers())
+	if (cgm_all_controllers_same)
 		slist = subsystems_inone;
 
 	for (i = 0; slist[i]; i++) {
@@ -367,7 +418,7 @@ static int chown_cgroup_wrapper(void *data)
 	}
 	destuid = get_ns_uid(arg->origuid);
 
-	if (cgm_supports_multiple_controllers())
+	if (cgm_supports_multiple_controllers)
 		slist = subsystems_inone;
 
 	for (i = 0; slist[i]; i++) {
@@ -425,7 +476,7 @@ static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
 	 * This can't be done in the child namespace because it only group-owns
 	 * the cgroup
 	 */
-	if (cgm_supports_multiple_controllers())
+	if (cgm_supports_multiple_controllers)
 		slist = subsystems_inone;
 
 	for (i = 0; slist[i]; i++) {
@@ -465,6 +516,9 @@ static void *cgm_init(const char *name)
 		ERROR("Error connecting to cgroup manager");
 		return NULL;
 	}
+
+	check_supports_multiple_controllers();
+
 	d = malloc(sizeof(*d));
 	if (!d) {
 		cgm_dbus_disconnect();
@@ -509,7 +563,7 @@ static void cgm_destroy(void *hdata)
 		return;
 	}
 
-	if (cgm_supports_multiple_controllers())
+	if (cgm_supports_multiple_controllers)
 		slist = subsystems_inone;
 	for (i = 0; slist[i]; i++)
 		cgm_remove_cgroup(slist[i], d->cgroup_path);
@@ -530,7 +584,7 @@ static inline void cleanup_cgroups(char *path)
 	int i;
 	char **slist = subsystems;
 
-	if (cgm_supports_multiple_controllers())
+	if (cgm_supports_multiple_controllers)
 		slist = subsystems_inone;
 	for (i = 0; slist[i]; i++)
 		cgm_remove_cgroup(slist[i], path);
@@ -576,7 +630,7 @@ again:
 	}
 	existed = 0;
 
-	if (cgm_supports_multiple_controllers())
+	if (cgm_supports_multiple_controllers)
 		slist = subsystems_inone;
 
 	for (i = 0; slist[i]; i++) {
@@ -636,35 +690,29 @@ static bool lxc_cgmanager_enter(pid_t pid, const char *controller,
 	return true;
 }
 
-/* Internal helper, must be called with cgmanager dbus socket open */
-static bool do_cgm_enter(pid_t pid, const char *cgroup_path, bool abs)
-{
-	char **slist = subsystems;
-	int i;
-
-	if (cgm_supports_multiple_controllers())
-		slist = subsystems_inone;
-
-	for (i = 0; slist[i]; i++) {
-		if (!lxc_cgmanager_enter(pid, slist[i], cgroup_path, abs))
-			return false;
-	}
-	return true;
-}
-
 static inline bool cgm_enter(void *hdata, pid_t pid)
 {
 	struct cgm_data *d = hdata;
+	char **slist = subsystems;
 	bool ret = false;
+	int i;
+
+	if (!d || !d->cgroup_path)
+		return false;
 
 	if (!cgm_dbus_connect()) {
 		ERROR("Error connecting to cgroup manager");
 		return false;
 	}
-	if (!d || !d->cgroup_path)
-		goto out;
-	if (do_cgm_enter(pid, d->cgroup_path, false))
-		ret = true;
+
+	if (cgm_all_controllers_same)
+		slist = subsystems_inone;
+
+	for (i = 0; slist[i]; i++) {
+		if (!lxc_cgmanager_enter(pid, slist[i], d->cgroup_path, false))
+			goto out;
+	}
+	ret = true;
 out:
 	cgm_dbus_disconnect();
 	return ret;
@@ -784,7 +832,7 @@ static void do_cgm_get(const char *name, const char *lxcpath, const char *filena
 			WARN("Failed to warn cgm_get of error; parent may hang");
 		exit(1);
 	}
-	cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
+	cgroup = try_get_abs_cgroup(name, lxcpath, controller);
 	if (!cgroup) {
 		cgm_dbus_disconnect();
 		ret = write(outp, &len, sizeof(len));
@@ -924,7 +972,7 @@ static void do_cgm_set(const char *name, const char *lxcpath, const char *filena
 			WARN("Failed to warn cgm_set of error; parent may hang");
 		exit(1);
 	}
-	cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
+	cgroup = try_get_abs_cgroup(name, lxcpath, controller);
 	if (!cgroup) {
 		cgm_dbus_disconnect();
 		ret = write(outp, &retval, sizeof(retval));
@@ -1222,24 +1270,35 @@ static bool cgm_chown(void *hdata, struct lxc_conf *conf)
  */
 static bool cgm_attach(const char *name, const char *lxcpath, pid_t pid)
 {
-	bool pass;
+	bool pass = true;
 	char *cgroup = NULL;
+	char **slist = subsystems;
+	int i;
 
 	if (!cgm_dbus_connect()) {
 		ERROR("Error connecting to cgroup manager");
 		return false;
 	}
-	// cgm_create makes sure that we have the same cgroup name for all
-	// subsystems, so since this is a slow command over the cmd socket,
-	// just get the cgroup name for the first one.
-	cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
-	if (!cgroup) {
-		ERROR("Failed to get cgroup for controller %s", subsystems[0]);
-		cgm_dbus_disconnect();
-		return false;
-	}
 
-	pass = do_cgm_enter(pid, cgroup, abs_cgroup_supported());
+	check_supports_multiple_controllers();
+
+	if (cgm_all_controllers_same)
+		slist = subsystems_inone;
+
+	for (i = 0; slist[i]; i++) {
+		cgroup = try_get_abs_cgroup(name, lxcpath, slist[i]);
+		if (!cgroup) {
+			ERROR("Failed to get cgroup for controller %s", slist[i]);
+			cgm_dbus_disconnect();
+			return false;
+		}
+
+		if (!lxc_cgmanager_enter(pid, slist[i], cgroup, abs_cgroup_supported())) {
+			pass = false;
+			break;
+		}
+
+	}
 	cgm_dbus_disconnect();
 	if (!pass)
 		ERROR("Failed to enter group %s", cgroup);
-- 
2.1.0



More information about the lxc-devel mailing list