[lxc-devel] [cgmanager/master] Fixes and improvements

maciejsszmigiero on Github lxc-bot at linuxcontainers.org
Thu Nov 16 00:41:14 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1446 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171116/fbb4b327/attachment.bin>
-------------- next part --------------
From e417490debb99f4b177a94bb6f15fe30a76c22d8 Mon Sep 17 00:00:00 2001
From: "Maciej S. Szmigiero" <mail at maciej.szmigiero.name>
Date: Thu, 9 Nov 2017 13:50:25 +0100
Subject: [PATCH 1/3] Fix various operations on controllers assigned to the
 unified hierarchy

Many operations supported by cgmanager had issues or didn't work at all on
controllers that were assigned to the unified (v2) hierarchy.

Most of the required fixes were simple, but "remove on empty" operation
needed a completely different implementation for cgroups v2 since they do
not support a child cgroup release notification agent, we need to watch for
modification of "cgroup.events" file in the particular cgroup via inotify
instead.
Unfortunately, we can't use libnih built-in inotify watcher for this since
it does not support watching for an IN_MODIFY event.

Signed-off-by: Maciej S. Szmigiero <mail at maciej.szmigiero.name>
---
 cgmanager.c | 294 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 frontend.h  |   1 +
 fs.c        |  84 +++++++++++------
 fs.h        |   3 +-
 4 files changed, 345 insertions(+), 37 deletions(-)

diff --git a/cgmanager.c b/cgmanager.c
index 293f261..681c239 100644
--- a/cgmanager.c
+++ b/cgmanager.c
@@ -22,6 +22,14 @@
 #include <sys/vfs.h>
 #include <linux/fs.h>
 
+struct autoremove_entry {
+	NihList entry;
+
+	char *gpath, *evpath, *dirname;
+	int wd_cg, wd_events;
+	NihIo *io;
+};
+
 /*
  * Maximum depth of directories we allow in Create
  * Default is 16.  Figure 4 directories per level of container
@@ -30,6 +38,8 @@
  */
 static int maxdepth = 16;
 
+static NihList autoremove_entries;
+
 /* GetPidCgroup */
 int get_pid_cgroup_main(void *parent, char *controller, struct ucred p,
 			 struct ucred r, struct ucred v, char **output)
@@ -349,7 +359,8 @@ int do_create_main(const char *controller, const char *cgroup, struct ucred p,
 			rmdir(path);
 			return -1;
 		}
-		if (!chown_cgroup_path(path, r.uid, r.gid, true)) {
+		if (!chown_cgroup_path(path, r.uid, r.gid, true,
+				       is_unified_controller(controller))) {
 			nih_error("%s: Failed to change ownership on %s to %u:%u", __func__,
 				path, r.uid, r.gid);
 			rmdir(path);
@@ -450,14 +461,15 @@ int do_chown_main(const char *controller, const char *cgroup, struct ucred p,
 	}
 
 	// go ahead and chown it.
-	if (!chown_cgroup_path(path, v.uid, v.gid, false)) {
+	if (!chown_cgroup_path(path, v.uid, v.gid, false,
+			       is_unified_controller(controller))) {
 		nih_error("%s: Failed to change ownership on %s to %u:%u", __func__,
 			path, v.uid, v.gid);
 		return -2;
 	}
 	if (is_unified_controller(controller)) {
 		NIH_MUST( nih_strcat(&path, NULL, U_LEAF) );
-		if (dir_exists(path) && !chown_cgroup_path(path, v.uid, v.gid, false)) {
+		if (dir_exists(path) && !chown_cgroup_path(path, v.uid, v.gid, false, true)) {
 			nih_warn("%s: Failed to chown leaf directory for %s to %u:%u",
 				__func__, path, v.uid, v.gid);
 			return -2;
@@ -900,7 +912,8 @@ int get_tasks_main(void *parent, char *controller, const char *cgroup,
 			struct ucred p, struct ucred r, int32_t **pids)
 {
 	char path[MAXPATHLEN];
-	const char *key = "tasks";
+	const char *key = is_unified_controller(controller) ?
+			  U_LEAF_NAME "/cgroup.procs" : "tasks";
 	int alloced_pids = 0, nrpids = 0;
 
 	if (!sane_cgroup(cgroup)) {
@@ -950,16 +963,17 @@ int get_tasks_main(void *parent, char *controller, const char *cgroup,
 	return nrpids;
 }
 
-int do_collect_tasks(void *parent, char **path, int32_t **pids,
-			int *alloced_pids, int *nrpids)
+static int do_collect_tasks(void *parent, char **path, int32_t **pids,
+			    int *alloced_pids, int *nrpids, bool is_unified)
 {
 	struct dirent dirent, *direntp;
 	DIR *dir;
-	const char *key = "tasks";
+	const char *key = is_unified ? U_LEAF_NAME "/cgroup.procs" : "tasks";
 
 	dir = opendir(*path);
 	if (!dir) {
-		nih_warn("%s: Failed to open dir %s for recursive deletion", __func__, *path);
+		nih_warn("%s: Failed to open dir %s for recursive collection",
+			 __func__, *path);
 		return -2;
 	}
 
@@ -971,7 +985,8 @@ int do_collect_tasks(void *parent, char **path, int32_t **pids,
 		if (!direntp)
 			break;
 		if (!strcmp(direntp->d_name, ".") ||
-		    !strcmp(direntp->d_name, ".."))
+		    !strcmp(direntp->d_name, "..") ||
+		    !strcmp(direntp->d_name, U_LEAF_NAME))
 			continue;
 		childname = NIH_MUST( nih_sprintf(NULL, "%s/%s", *path, direntp->d_name) );
 		rc = lstat(childname, &mystat);
@@ -979,7 +994,8 @@ int do_collect_tasks(void *parent, char **path, int32_t **pids,
 			continue;
 		if (S_ISDIR(mystat.st_mode))
 			if (do_collect_tasks(parent, &childname, pids,
-						alloced_pids, nrpids) == -1)
+					     alloced_pids, nrpids,
+					     is_unified) == -1)
 				nih_info("%s: error descending subdirs", __func__);
 	}
 
@@ -1022,7 +1038,8 @@ int collect_tasks(void *parent, const char *controller, const char *cgroup,
 	}
 
 	rpath = NIH_MUST( nih_strdup(NULL, path) );
-	return do_collect_tasks(parent, &rpath, pids, alloced_pids, nrpids);
+	return do_collect_tasks(parent, &rpath, pids, alloced_pids, nrpids,
+				is_unified_controller(controller));
 }
 
 int get_tasks_recursive_main(void *parent, const char *controller,
@@ -1114,6 +1131,253 @@ int list_children_main(void *parent, char *controller, const char *cgroup,
 	return get_directory_children(parent, path, output);
 }
 
+static int autoremove_entry_destroy(struct autoremove_entry *entry)
+{
+	nih_assert(entry != NULL);
+
+	nih_assert(entry->gpath != NULL);
+	nih_discard(entry->gpath);
+
+	nih_assert(entry->evpath != NULL);
+	nih_discard(entry->evpath);
+
+	nih_assert(entry->dirname != NULL);
+	nih_discard(entry->dirname);
+
+	if (entry->io != NULL)
+		nih_discard(entry->io);
+
+	nih_list_destroy(&entry->entry);
+
+	return 0;
+}
+
+/* if this function returns true then discontinue watching the events file */
+static bool autoremove_events_modified(struct autoremove_entry *entry)
+{
+	FILE *f;
+	char line[1024];
+	int pop_val = -1;
+	nih_local char *leafpath = NULL;
+
+	nih_assert(entry != NULL);
+	nih_assert(entry->evpath != NULL);
+
+	f = fopen(entry->evpath, "r");
+	if (!f) {
+		nih_error("%s: Cannot open %s in watcher: %s",
+			  __func__, entry->evpath, strerror(errno));
+		return false;
+	}
+
+	while (fgets(line, 1024, f) != NULL) {
+		int val;
+
+		if (sscanf(line, " populated %d", &val) != 1)
+			continue;
+
+		pop_val = !!val;
+		break;
+	}
+
+	fclose(f);
+
+	if (pop_val == -1) {
+		nih_error("%s: Cannot find or parse populated value in %s",
+			  __func__, entry->evpath);
+		return false;
+	} else if (pop_val == 1)
+		return false;
+
+	nih_assert(entry->gpath != NULL);
+	leafpath = NIH_MUST( nih_sprintf(NULL, "%s%s", entry->gpath, U_LEAF) );
+	if (rmdir(leafpath) < 0) {
+		if (errno == EBUSY)
+			return false;
+
+		nih_error("%s: Failed to remove %s: %s", __func__, leafpath,
+			  strerror(errno));
+	}
+
+	if (rmdir(entry->gpath) < 0) {
+		if (errno == EBUSY)
+			return false;
+
+		nih_error("%s: Failed to remove %s: %s", __func__, entry->gpath,
+			  strerror(errno));
+		return false;
+	}
+
+	nih_info(_("Removed %s as it was empty"), entry->gpath);
+	return true;
+}
+
+static void autoremove_inotify_read(void *data, NihIo *io, const char *buf,
+				    size_t len)
+{
+	struct autoremove_entry *entry = data;
+	struct inotify_event *event;
+
+	nih_assert(entry != NULL);
+	nih_assert(io != NULL);
+	nih_assert(buf != NULL);
+
+	bool should_exit = false;
+	while (len >= sizeof(*event)) {
+		size_t esize;
+
+		event = (struct inotify_event *)buf;
+		esize = sizeof(*event) + event->len;
+		if (len < esize)
+			break;
+
+		if (event->wd != entry->wd_cg &&
+		    event->wd != entry->wd_events) {
+			nih_error("%s: Got unknown watch descriptor %d (expected %d or %d) while watching %s",
+				  __func__, event->wd, entry->wd_cg,
+				  entry->wd_events, entry->gpath);
+			goto next;
+		}
+
+		if (event->mask & IN_IGNORED) {
+			nih_info(_("%s watch was removed"),
+				 entry->gpath);
+			should_exit = true;
+			goto next;
+		}
+
+		if (event->mask & IN_DELETE &&
+		    event->wd == entry->wd_cg)
+			do {
+				if (event->len < 1) {
+					nih_warn("got DELETE inotify event without object name");
+					break;
+				}
+
+				if (strcmp(event->name, entry->dirname) != 0)
+					break;
+
+				nih_info(_("%s was removed by somebody else"),
+					 entry->gpath);
+				should_exit = true;
+				goto next;
+			} while (0);
+
+		if (event->mask & IN_MODIFY &&
+		    event->wd == entry->wd_events)
+			if (autoremove_events_modified(entry)) {
+				should_exit = true;
+				goto next;
+			}
+
+	next:
+		nih_io_buffer_shrink(io->recv_buf, esize);
+		len -= esize;
+
+		if (should_exit) {
+			nih_discard(entry);
+			return;
+		}
+	}
+}
+
+static int do_remove_on_empty_unified(const char *path)
+{
+	nih_local char *wpath = NIH_MUST( nih_alloc(NULL, PATH_MAX) );
+	nih_local char *evpath = NULL;
+	nih_local char *parentpath = NULL;
+	char *lastpart;
+	struct autoremove_entry *entry;
+	int ifd, wd_cg, wd_events;
+
+	if (realpath(path, wpath) == NULL || strlen(wpath) < 1) {
+		nih_error("%s: Failed to expand path %s: %s", __func__, path,
+			  strerror(errno));
+		return -1;
+	}
+
+	if (wpath[strlen(wpath) - 1] == '/')
+		wpath[strlen(wpath) - 1] = '\0';
+
+	NIH_LIST_FOREACH(&autoremove_entries, lentry) {
+		entry = (struct autoremove_entry *)lentry;
+
+		nih_assert(entry != NULL);
+		nih_assert(entry->gpath != NULL);
+
+		if (strcmp(entry->gpath, wpath) == 0)
+			return 0;
+	}
+
+	evpath = NIH_MUST( nih_sprintf(NULL, "%s/cgroup.events", wpath) );
+
+	parentpath = NIH_MUST( nih_strdup(NULL, wpath) );
+	lastpart = strrchr(parentpath, '/');
+	if (lastpart == NULL) {
+		nih_error("%s: Failed to get last directory in path %s (%s)",
+			  __func__, parentpath, path);
+		return -1;
+	}
+	*lastpart = '\0';
+	lastpart++;
+
+	ifd = inotify_init1(IN_CLOEXEC);
+	if (ifd < 0) {
+		nih_error("%s: Failed to init inotify for %s: %s", __func__,
+			  path, strerror(errno));
+		return -1;
+	}
+
+	/*
+	 * IN_DELETE_SELF or IN_IGNORED events aren't generated for a cgroup
+	 * (or its files) that is being removed, we have to monitor parent cgroup
+	 * directory for IN_DELETE events instead
+	 */
+	wd_cg = inotify_add_watch(ifd, parentpath, IN_DELETE);
+	if (wd_cg < 0) {
+		nih_error("%s: Failed to add watch for %s: %s", __func__, parentpath,
+			  strerror(errno));
+		close(ifd);
+		return -1;
+	}
+
+	wd_events = inotify_add_watch(ifd, evpath, IN_MODIFY);
+	if (wd_events < 0) {
+		nih_error("%s: Failed to add watch for %s: %s", __func__, evpath,
+			  strerror(errno));
+		close(ifd);
+		return -1;
+	}
+
+	entry = NIH_MUST( nih_alloc(NULL, sizeof(*entry)) );
+	nih_list_init(&entry->entry);
+	entry->gpath = NIH_MUST( nih_strdup(NULL, wpath) );
+	entry->evpath = NIH_MUST( nih_strdup(NULL, evpath) );
+	entry->dirname = NIH_MUST( nih_strdup(NULL, lastpart) );
+	entry->wd_cg = wd_cg;
+	entry->wd_events = wd_events;
+	entry->io = NULL;
+
+	nih_list_add(&autoremove_entries, &entry->entry);
+	nih_alloc_set_destructor(entry, autoremove_entry_destroy);
+
+	entry->io = nih_io_reopen(NULL, ifd, NIH_IO_STREAM,
+				  autoremove_inotify_read,
+				  NULL, NULL, entry);
+	if (entry->io == NULL) {
+		nih_error("%s: Failed to add IO for watch for %s",
+			  __func__, evpath);
+		nih_discard(entry);
+		close(ifd);
+		return -1;
+	}
+
+	if (autoremove_events_modified(entry))
+		nih_discard(entry);
+
+	return 0;
+}
+
 int do_remove_on_empty_main(const char *controller, const char *cgroup,
 		struct ucred p, struct ucred r)
 {
@@ -1158,6 +1422,9 @@ int do_remove_on_empty_main(const char *controller, const char *cgroup,
 		return -1;
 	}
 
+	if (is_unified_controller(controller))
+		return do_remove_on_empty_unified(working);
+
 	NIH_MUST( nih_strcat(&working, NULL, "/notify_on_release") );
 
 	if (!set_value_trusted(working, "1\n")) {
@@ -1502,6 +1769,8 @@ main (int argc, char *argv[])
 	struct stat sb;
 	struct rlimit newrlimit;
 
+	nih_list_init(&autoremove_entries);
+
 	nih_main_init (argv[0]);
 
 	nih_option_set_synopsis (_("Control group manager"));
@@ -1584,5 +1853,8 @@ main (int argc, char *argv[])
 
 	ret = nih_main_loop ();
 
+	while (!NIH_LIST_EMPTY(&autoremove_entries))
+		nih_free(autoremove_entries.next);
+
 	return ret;
 }
diff --git a/frontend.h b/frontend.h
index a03e557..4756a2c 100644
--- a/frontend.h
+++ b/frontend.h
@@ -35,6 +35,7 @@
 #include <stdbool.h>
 #include <libgen.h>
 #include <unistd.h>
+#include <sys/inotify.h>
 #include <sys/mount.h>
 #include <dirent.h>
 
diff --git a/fs.c b/fs.c
index 4a106e0..dfcbf44 100644
--- a/fs.c
+++ b/fs.c
@@ -582,6 +582,7 @@ static bool collect_kernel_subsystems(void)
 	FILE *cgf;
 	char line[400];
 	bool bret = false;
+	bool is_io_unified = is_unified_controller("io");
 
 	if ((cgf = fopen("/proc/cgroups", "r")) == NULL) {
 		nih_fatal ("Error opening /proc/cgroups: %s", strerror(errno));
@@ -606,6 +607,13 @@ static bool collect_kernel_subsystems(void)
 		if (*(p+1) != '1')
 			continue;
 
+		/*
+		 * if unified "io" controller is enabled then "blkio"
+		 * v1 controller is not available for use
+		 */
+		if (strcmp(line, "blkio") == 0 && is_io_unified)
+			continue;
+
 		if (!save_mount_subsys(line)) {
 			nih_fatal("Error storing subsystem %s", line);
 			goto out;
@@ -1386,6 +1394,7 @@ static inline char *pid_cgroup(pid_t pid, const char *controller, char *retv)
 	char path[100];
 	char *line = NULL, *cgroup = NULL;
 	size_t len = 0;
+	bool is_unified = is_unified_controller(controller);
 
 	sprintf(path, "/proc/%d/cgroup", pid);
 	if ((f = fopen(path, "r")) == NULL) {
@@ -1394,26 +1403,52 @@ static inline char *pid_cgroup(pid_t pid, const char *controller, char *retv)
 	}
 	while (getline(&line, &len, f) != -1) {
 		char *c1, *c2;
-		char *token, *saveptr = NULL;
+		char *endptr;
+		long cnr;
+
 		if ((c1 = strchr(line, ':')) == NULL)
 			continue;
+		if (c1 == line)
+			continue;
+		*c1 = '\0';
+
+		cnr = strtol(line, &endptr, 10);
+		if (*endptr != '\0')
+			continue;
+
 		if ((c2 = strchr(++c1, ':')) == NULL)
 			continue;
 		*c2 = '\0';
-		for (; (token = strtok_r(c1, ",", &saveptr)); c1 = NULL) {
-			if (!is_same_controller(token, controller))
+
+		if (is_unified) {
+			if (cnr != 0)
 				continue;
-			if (strlen(c2+1) + 1 > MAXPATHLEN) {
-				nih_error("cgroup name too long");
-				goto found;
-			}
-			strncpy(retv, c2+1, strlen(c2+1)+1);
-			drop_newlines(retv);
-			cgroup = retv;
-			goto found;
+		} else {
+			char *token, *saveptr = NULL;
+
+			if (cnr == 0)
+				continue;
+
+			for (; (token = strtok_r(c1, ",", &saveptr));
+			     c1 = NULL)
+				if (is_same_controller(token, controller))
+					break;
+
+			if (token == NULL)
+				continue;
+		}
+
+		if (strlen(c2 + 1) + 1 > MAXPATHLEN) {
+			nih_error("cgroup name too long");
+			break;
 		}
+
+		strncpy(retv, c2 + 1, strlen(c2 + 1) + 1);
+		drop_newlines(retv);
+		cgroup = retv;
+		break;
 	}
-found:
+
 	fclose(f);
 	free(line);
 	if (is_unified_controller(controller))
@@ -1938,12 +1973,10 @@ void get_pid_creds(pid_t pid, uid_t *uid, gid_t *gid)
  *
  * Return true so long as we could chown the directory itself.
  */
-bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid, bool all_children)
+bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid,
+		       bool all_children, bool is_unified)
 {
-	int len;
-
 	nih_assert (path);
-	len = strlen(path);
 	if (chown(path, uid, gid) < 0)
 		return false;
 
@@ -1952,7 +1985,7 @@ bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid, bool all_children
 		struct dirent dirent, *direntp;
 		DIR *d;
 
-		if (len >= MAXPATHLEN)
+		if (strlen(path) >= MAXPATHLEN)
 			return true;
 
 		d = opendir(path);
@@ -1970,16 +2003,17 @@ bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid, bool all_children
 		}
 		closedir(d);
 	} else {
-		// chown only the tasks and procs files
+		// chown only the tasks or procs file
 		nih_local char *fpath = NULL;
-		fpath = NIH_MUST( nih_sprintf(NULL, "%s/cgroup.procs", path) );
-		if (chown(fpath, uid, gid) < 0)
-			nih_error("%s: Failed to chown procs file %s: %s", __func__,
-					fpath, strerror(errno));
-		sprintf(fpath+len, "/tasks");
+
+		fpath = NIH_MUST( nih_sprintf(NULL, "%s/%s", path,
+					      is_unified ?
+					      "cgroup.procs" :
+					      "tasks") );
 		if (chown(fpath, uid, gid) < 0)
-			nih_warn("%s: Failed to chown the tasks file %s: %s\n",
-					__func__, fpath, strerror(errno));
+			nih_error("%s: Failed to chown %s file %s: %s",
+				  __func__, is_unified ? "procs" : "tasks",
+				  fpath, strerror(errno));
 	}
 
 out:
diff --git a/fs.h b/fs.h
index 3a56aa4..e166e8a 100644
--- a/fs.h
+++ b/fs.h
@@ -46,7 +46,8 @@ int file_read_pids(void *parent, const char *path, int32_t **pids,
 void get_pid_creds(pid_t pid, uid_t *uid, gid_t *gid);
 const char *get_controller_path(const char *controller);
 bool hostuid_to_ns(uid_t uid, pid_t pid, uid_t *answer);
-bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid, bool all_children);
+bool chown_cgroup_path(const char *path, uid_t uid, gid_t gid,
+		       bool all_children, bool is_unified);
 bool chmod_cgroup_path(const char *path, int mode);
 bool set_value(const char *controller, const char *path, const char *value);
 bool set_value_trusted(const char *path, const char *value);

From 2cadee8e48687b70c03dc5eaa5725ab665533fd9 Mon Sep 17 00:00:00 2001
From: "Maciej S. Szmigiero" <mail at maciej.szmigiero.name>
Date: Wed, 15 Nov 2017 22:27:57 +0100
Subject: [PATCH 2/3] Add an ability to allow autoremove on pre-mounted v1
 controllers

Some init systems (for example OpenRC) mount v1 controllers on boot to be
able to put their services under them but do not set any release agent on
these mounts.
If such controller is then used with cgmanager the autoremove (or
remove-on-empty) functionality will not work for this controller.

This means that when, for example, per-user-session cgroups are created
under such controller, too, empty cgroups from past sessions will litter
the system.

It is safe to allow autoremove and use our release agent for such
pre-mounted controllers, as long as we enable it (via "notify_on_release")
only on particular (sub-)cgroups that we had received an autoremove request
for.

Add two new options to cgmanager to possibly allow just that (they default
to "disabled" setting for backwards compatibility).

Signed-off-by: Maciej S. Szmigiero <mail at maciej.szmigiero.name>
---
 cgmanager.c | 26 ++++++++++++++++++++++----
 fs.c        | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 fs.h        |  3 +++
 3 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/cgmanager.c b/cgmanager.c
index 681c239..50cc1df 100644
--- a/cgmanager.c
+++ b/cgmanager.c
@@ -1385,8 +1385,9 @@ int do_remove_on_empty_main(const char *controller, const char *cgroup,
 	size_t cgroup_len;
 	nih_local char *working = NULL, *wcgroup = NULL;
 
-	if (was_premounted(controller)) {
-		nih_error("remove-on-empty request for pre-mounted controller");
+	if (was_premounted(controller) &&
+	    !premounted_should_allow_autoremove(controller)) {
+		nih_warn("remove-on-empty request for pre-mounted controller");
 		return -2;
 	}
 
@@ -1461,7 +1462,7 @@ int remove_on_empty_main(const char *controller, const char *cgroup,
 	tok = strtok(c, ",");
 	while (tok) {
 		ret = do_remove_on_empty_main(tok, cgroup, p, r);
-		if (ret == -2)  // pre-mounted, autoremove not an option, ignore
+		if (ret == -2)  // autoremove not supported, ignore
 			goto next;
 		if (ret != 0)
 			return -1;
@@ -1555,7 +1556,9 @@ int do_prune_main(const char *controller, const char *cgroup,
 		return -1;
 	}
 
-	do_recursive_prune(working, !was_premounted(controller));
+	do_recursive_prune(working,
+			   !was_premounted(controller) ||
+			   premounted_should_allow_autoremove(controller));
 
 	return 0;
 }
@@ -1664,6 +1667,14 @@ extra_mounts_set (NihOption *option, const char *arg)
 	return 0;
 }
 
+static int
+allow_autoremove_premounted_set (NihOption *option, const char *arg)
+{
+	allow_autoremove_premounted = NIH_MUST( strdup(arg) );
+
+	return 0;
+}
+
 /**
  * options:
  *
@@ -1676,6 +1687,13 @@ static NihOption options[] = {
 		NULL, "subsystems to mount", NULL, skip_mounts_set },
 	{ 'm', "mount", N_("Extra subsystems to mount"),
 		NULL, "subsystems to mount", NULL, extra_mounts_set },
+	{ 0, "allow-autoremove-premounted",
+	  N_("v1 controllers (comma separated) for which we allow autoremove even though they were premounted or \"all\" to allow it on all such controllers"),
+	  NULL, "premounted_controllers", NULL,
+	  allow_autoremove_premounted_set },
+	{ 0, "autoremove-premounted-set-release-agent",
+	  N_("Set our release agent for premounted v1 controllers with autoremove enabled that do not have an agent already set"),
+	  NULL, NULL, &autoremove_premounted_set_release_agent, NULL },
 	{ 0, "daemon", N_("Detach and run in the background"),
 		NULL, NULL, &daemonise, NULL },
 	{ 0, "sigstop", N_("Raise SIGSTOP when ready"),
diff --git a/fs.c b/fs.c
index dfcbf44..8000165 100644
--- a/fs.c
+++ b/fs.c
@@ -117,6 +117,31 @@ bool dir_exists(const char *path)
 	return true;
 }
 
+char *allow_autoremove_premounted;
+int autoremove_premounted_set_release_agent = FALSE;
+
+bool premounted_should_allow_autoremove(const char *controller)
+{
+	nih_local char *allowed = NULL;
+	char *ctrl;
+
+	if (allow_autoremove_premounted == NULL)
+		return false;
+
+	if (strcmp(allow_autoremove_premounted, "all") == 0)
+		return true;
+
+	allowed = NIH_MUST( nih_strdup(NULL, allow_autoremove_premounted) );
+
+	nih_assert(controller != NULL);
+	for (ctrl = strtok(allowed, ","); ctrl != NULL;
+	     ctrl = strtok(NULL, ","))
+		if (strcmp(controller, ctrl) == 0)
+			return true;
+
+	return false;
+}
+
 /*
  * Where do we want to mount the controllers?  We used to mount
  * them under a tmpfs under /sys/fs/cgroup, for all to share.  Now
@@ -312,12 +337,43 @@ static bool set_release_agent(struct controller_mounts *m)
 	FILE *f;
 	nih_local char *path = NULL;
 
-	if (m->premounted) {
+	if (m->premounted &&
+	    !(premounted_should_allow_autoremove(m->controller)
+	      && autoremove_premounted_set_release_agent)) {
 		nih_info("%s was pre-mounted, not setting a release agent",
 			m->controller);
 		return true;
 	}
 	path = NIH_MUST( nih_sprintf(NULL, "%s/release_agent", m->path) );
+
+	if (m->premounted) {
+		char buf[128];
+		size_t read;
+		bool was_set = false;
+
+		f = fopen(path, "r");
+		if (f == NULL) {
+			nih_error("failed to open %s for reading", path);
+			return false;
+		}
+
+		while (!was_set && (read = fread(buf, 1, sizeof(buf), f)) > 0) {
+			size_t ctr;
+
+			for (ctr = 0; !was_set && ctr < read; ctr++)
+				if (!isspace(buf[ctr]))
+					was_set = true;
+		}
+
+		fclose(f);
+
+		if (was_set) {
+			nih_info("%s was pre-mounted and already has a release agent, not setting our one",
+				 m->controller);
+			return true;
+		}
+	}
+
 	if ((f = fopen(path, "w")) == NULL) {
 		nih_error("failed to open %s for writing", path);
 		return false;
@@ -1321,7 +1377,9 @@ bool create_agent_symlinks(void)
 	}
 
 	for (i=0; i<num_controllers; i++) {
-		if (all_mounts[i].premounted)
+		if (all_mounts[i].premounted &&
+		    !(premounted_should_allow_autoremove(all_mounts[i].controller) &&
+		      autoremove_premounted_set_release_agent))
 			continue;
 
 		ret = snprintf(buf+plen, MAXPATHLEN-plen, "cgm-release-agent.%s",
diff --git a/fs.h b/fs.h
index e166e8a..0ccb8f1 100644
--- a/fs.h
+++ b/fs.h
@@ -30,8 +30,11 @@
 #define U_LEAF "/" U_LEAF_NAME
 
 extern char *all_controllers;
+extern char *allow_autoremove_premounted;
+extern int autoremove_premounted_set_release_agent;
 struct keys_return_type;
 
+bool premounted_should_allow_autoremove(const char *controller);
 int collect_subsystems(char *extra_mounts, char *skip_mounts);
 int setup_cgroup_mounts(void);
 bool compute_pid_cgroup(pid_t pid, const char *controller, const char *cgroup,

From 87b45fdb034a170719e0949094736048bed8a32b Mon Sep 17 00:00:00 2001
From: "Maciej S. Szmigiero" <mail at maciej.szmigiero.name>
Date: Thu, 9 Nov 2017 13:51:01 +0100
Subject: [PATCH 3/3] Make the PAM_CGM module more configurable

This commit cleans up the PAM_CGM module and makes it much more
configurable.

First, the module is rewritten to keep its data using a PAM module internal
data mechanism instead of using a set of static variables since PAM API
allows doing multiple transactions in parallel, including utilizing the
same module, as long as each one uses a separate transaction handle.

Additionally, a PAM man page for pam_set_data() specifically says that
"[i]n general such files [PAM modules] should not contain static
variables".

Second, the following configuration options are added:
* A pattern setting, which allows naming cgroups created by this module
in a different way from "user/foo/0",

* An ability to disable appending of an index suffix to a pattern-derived
cgroup name,

* A limit setting which allows controlling how large these suffix indices
(if they aren't disabled by the previous option) are allowed to grow,

* An ability to have particular values set in the created final cgroups,

* A setting controlling how deep cgroups prune process goes at the session
close time.

All these options (and their behavior) are described at the top of the
pam_cgm.c file, where there was already a description of the module
operation.

Unfortunately, we cannot use libnih option parsing since it does not work
in a shared library context so in order not to reinvent the wheel the
popular popt library was used instead.

The default values of all these options were set in a way so not to result
in any change of the module behavior, unless specifically configured in a
different way by an user, for compatibility with existing deployments.

Signed-off-by: Maciej S. Szmigiero <mail at maciej.szmigiero.name>
---
 Makefile.am     |   2 +-
 configure.ac    |   3 +
 pam/cgmanager.c | 113 +++++---
 pam/cgmanager.h |  34 ++-
 pam/pam_cgm.c   | 811 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 5 files changed, 814 insertions(+), 149 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index efd97f3..91b8a90 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -140,7 +140,7 @@ if HAVE_PAM
 pam_LTLIBRARIES = pam_cgm.la
 pam_cgm_la_SOURCES = pam/pam_cgm.c pam/cgmanager.c pam/cgmanager.h
 pam_cgm_la_CFLAGS = $(AM_CFLAGS)
-pam_cgm_la_LIBADD = $(AM_LIBS) $(PAM_LIBS) -L$(top_srcdir) -lcgmanager
+pam_cgm_la_LIBADD = $(AM_LIBS) $(PAM_LIBS) -L$(top_srcdir) -lpthread -lpopt -lcgmanager
 pam_cgm_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version -shared
 pam_cgm_la_DEPENDENCIES = libcgmanager.la
 
diff --git a/configure.ac b/configure.ac
index efd873c..8cc10ff 100644
--- a/configure.ac
+++ b/configure.ac
@@ -163,6 +163,9 @@ if test "z$pamdir" != "znone"; then
 		[AC_MSG_ERROR([*** libpam not found.])
 		])
 
+	AC_CHECK_LIB([popt],[poptGetContext],,
+		AC_MSG_ERROR([libpopt required but not found]))
+
 	AC_SUBST(PAM_LIBS)
 	AC_SUBST([pamdir])
 fi
diff --git a/pam/cgmanager.c b/pam/cgmanager.c
index 9f6dd18..982117a 100644
--- a/pam/cgmanager.c
+++ b/pam/cgmanager.c
@@ -42,30 +42,31 @@
 #include "cgmanager-client.h"
 #include <nih/alloc.h>
 #include <nih/error.h>
+#include <nih/logging.h>
 #include <nih/string.h>
 
 #include "cgmanager.h"
 
-static NihDBusProxy *cgroup_manager = NULL;
-static int32_t api_version;
-
-void cgm_dbus_disconnect(void)
+void cgm_dbus_disconnect(NihDBusProxy **cgroup_manager)
 {
-       if (cgroup_manager) {
-	       dbus_connection_flush(cgroup_manager->connection);
-	       dbus_connection_close(cgroup_manager->connection);
-               nih_free(cgroup_manager);
-       }
-       cgroup_manager = NULL;
-}
+	nih_assert(cgroup_manager != NULL);
 
-char *ctrl_list;
+	if (*cgroup_manager) {
+		dbus_connection_flush((*cgroup_manager)->connection);
+		dbus_connection_close((*cgroup_manager)->connection);
+		nih_free(*cgroup_manager);
+		*cgroup_manager = NULL;
+	}
+}
 
 #define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
-bool cgm_dbus_connect(void)
+bool cgm_dbus_connect(NihDBusProxy **cgroup_manager)
 {
 	DBusError dbus_error;
-	static DBusConnection *connection;
+	DBusConnection *connection;
+	int32_t api_version;
+
+	nih_assert(cgroup_manager != NULL);
 
 	dbus_error_init(&dbus_error);
 
@@ -78,33 +79,33 @@ bool cgm_dbus_connect(void)
 	}
 	dbus_connection_set_exit_on_disconnect(connection, FALSE);
 	dbus_error_free(&dbus_error);
-	cgroup_manager = nih_dbus_proxy_new(NULL, connection,
+	*cgroup_manager = nih_dbus_proxy_new(NULL, connection,
 				NULL /* p2p */,
 				"/org/linuxcontainers/cgmanager", NULL, NULL);
 	dbus_connection_unref(connection);
-	if (!cgroup_manager) {
+	if (!*cgroup_manager) {
 		NihError *nerr;
 		nerr = nih_error_get();
 		fprintf(stderr, "Error opening cgmanager proxy: %s\n", nerr->message);
 		nih_free(nerr);
-		cgm_dbus_disconnect();
 		return false;
 	}
 
 	// get the api version
-	if (cgmanager_get_api_version_sync(NULL, cgroup_manager, &api_version) != 0) {
+	if (cgmanager_get_api_version_sync(NULL, *cgroup_manager, &api_version) != 0) {
 		NihError *nerr;
 		nerr = nih_error_get();
 		fprintf(stderr, "Error cgroup manager api version: %s\n", nerr->message);
 		nih_free(nerr);
-		cgm_dbus_disconnect();
+		cgm_dbus_disconnect(cgroup_manager);
 		return false;
 	}
 
 	return true;
 }
 
-bool cgm_create(const char *cg, int32_t *existed)
+bool cgm_create(NihDBusProxy *cgroup_manager, const char *ctrl_list, const char *cg,
+		int32_t *existed)
 {
 	if ( cgmanager_create_sync(NULL, cgroup_manager, ctrl_list, cg, existed) != 0) {
 		NihError *nerr;
@@ -115,7 +116,8 @@ bool cgm_create(const char *cg, int32_t *existed)
 	return true;
 }
 
-bool cgm_autoremove(const char *cg)
+bool cgm_autoremove(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		    const char *cg)
 {
 	if ( cgmanager_remove_on_empty_sync(NULL, cgroup_manager, ctrl_list, cg) != 0) {
 		NihError *nerr;
@@ -126,7 +128,7 @@ bool cgm_autoremove(const char *cg)
 	return true;
 }
 
-bool cgm_enter(const char *cg)
+bool cgm_enter(NihDBusProxy *cgroup_manager, const char *ctrl_list, const char *cg)
 {
 	if ( cgmanager_move_pid_sync(NULL, cgroup_manager, ctrl_list, cg,
 				(int32_t) getpid()) != 0 ) {
@@ -138,7 +140,8 @@ bool cgm_enter(const char *cg)
 	return true;
 }
 
-bool cgm_chown(const char *cg, uid_t uid, gid_t gid)
+bool cgm_chown(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+	       const char *cg, uid_t uid, gid_t gid)
 {
 	if ( cgmanager_chown_sync(NULL, cgroup_manager, ctrl_list, cg, uid, gid) != 0) {
 		NihError *nerr;
@@ -149,7 +152,7 @@ bool cgm_chown(const char *cg, uid_t uid, gid_t gid)
 	return true;
 }
 
-char **cgm_list_controllers(void)
+char **cgm_list_controllers(NihDBusProxy *cgroup_manager)
 {
 	char **controllers;
 	if ( cgmanager_list_controllers_sync(NULL, cgroup_manager, &controllers) != 0 ) {
@@ -165,7 +168,8 @@ char **cgm_list_controllers(void)
  * We can't list_children on >1 (not-comounted) controllers.
  * So choose the first controller and get the children of it
  */
-char **cgm_list_children(const char *cg)
+char **cgm_list_children(NihDBusProxy *cgroup_manager,
+			 const char *ctrl_list, const char *cg)
 {
 	char **children;
 	nih_local char *ctrl = NIH_MUST( nih_strdup(NULL, ctrl_list) );
@@ -181,7 +185,8 @@ char **cgm_list_children(const char *cg)
 	return children;
 }
 
-bool cgm_cg_has_tasks(const char *cg)
+bool cgm_cg_has_tasks(NihDBusProxy *cgroup_manager,
+		      const char *ctrl_list, const char *cg)
 {
 	nih_local int32_t * pids;
 	size_t len;
@@ -195,7 +200,20 @@ bool cgm_cg_has_tasks(const char *cg)
 	return len > 0;
 }
 
-void cgm_clear_cgroup(const char *cg)
+bool cgm_cg_set_value(NihDBusProxy *cgroup_manager, const char *controller,
+		      const char *cg, const char *key, const char *val)
+{
+	if (cgmanager_set_value_sync(NULL, cgroup_manager, controller, cg, key, val) != 0) {
+		NihError *nerr;
+		nerr = nih_error_get();
+		nih_free(nerr);
+		return false;
+	}
+	return true;
+}
+
+void cgm_clear_cgroup(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		      const char *cg)
 {
 	int32_t recursive = 1;
 	int32_t existed;
@@ -207,11 +225,42 @@ void cgm_clear_cgroup(const char *cg)
 	}
 }
 
-void cgm_escape(void)
+bool cgm_escape(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		char **ctrl_list_out, bool *all_ok)
 {
-	if ( cgmanager_move_pid_abs_sync(NULL, cgroup_manager, ctrl_list, "/", (int32_t) getpid()) != 0) {
-		NihError *nerr;
-		nerr = nih_error_get();
-		nih_free(nerr);
+	nih_local char *ctrl_local = NIH_MUST( nih_strdup(NULL, ctrl_list) );
+	char *tok, *savetok;
+	bool ret = false;
+
+	nih_assert(ctrl_list != NULL);
+
+	if (ctrl_list_out != NULL)
+		*ctrl_list_out = NULL;
+
+	if (all_ok != NULL)
+		*all_ok = true;
+
+	for (tok = strtok_r(ctrl_local, ",", &savetok); tok != NULL;
+	     tok = strtok_r(NULL, ",", &savetok)) {
+		if (cgmanager_move_pid_abs_sync(NULL, cgroup_manager, tok, "/",
+						(int32_t)getpid()) != 0) {
+			NihError *nerr;
+			nerr = nih_error_get();
+			nih_free(nerr);
+
+			if (all_ok != NULL)
+				*all_ok = false;
+
+			continue;
+		}
+
+		if (ctrl_list_out != NULL)
+			NIH_MUST( nih_strcat_sprintf(ctrl_list_out, NULL, "%s%s",
+						     *ctrl_list_out ? "," : "",
+						     tok) );
+
+		ret = true;
 	}
+
+	return ret;
 }
diff --git a/pam/cgmanager.h b/pam/cgmanager.h
index cae35e7..974e552 100644
--- a/pam/cgmanager.h
+++ b/pam/cgmanager.h
@@ -1,13 +1,25 @@
 #include <stdbool.h>
 
-bool cgm_dbus_connect(void);
-void cgm_dbus_disconnect(void);
-bool cgm_create(const char *cg, int *existed);
-bool cgm_autoremove(const char *cg);
-bool cgm_enter(const char *cg);
-bool cgm_chown(const char *cg, uid_t uid, gid_t gid);
-char **cgm_list_controllers(void);
-char **cgm_list_children(const char *cg);
-bool cgm_cg_has_tasks(const char *cg);
-void cgm_clear_cgroup(const char *cg);
-void cgm_escape(void);
+#include <nih-dbus/dbus_proxy.h>
+
+bool cgm_dbus_connect(NihDBusProxy **cgroup_manager);
+void cgm_dbus_disconnect(NihDBusProxy **cgroup_manager);
+bool cgm_create(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		const char *cg, int *existed);
+bool cgm_autoremove(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		    const char *cg);
+bool cgm_enter(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+	       const char *cg);
+bool cgm_chown(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+	       const char *cg, uid_t uid, gid_t gid);
+char **cgm_list_controllers(NihDBusProxy *cgroup_manager);
+char **cgm_list_children(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+			 const char *cg);
+bool cgm_cg_has_tasks(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		      const char *cg);
+bool cgm_cg_set_value(NihDBusProxy *cgroup_manager, const char *controller,
+		      const char *cg, const char *key, const char *val);
+void cgm_clear_cgroup(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		      const char *cg);
+bool cgm_escape(NihDBusProxy *cgroup_manager, const char *ctrl_list,
+		char **ctrl_list_out, bool *all_ok);
diff --git a/pam/pam_cgm.c b/pam/pam_cgm.c
index ad5d4fa..0ba4f11 100644
--- a/pam/pam_cgm.c
+++ b/pam/pam_cgm.c
@@ -3,12 +3,52 @@
  * Copyright © 2015 Canonical, Inc
  * Author: Serge Hallyn <serge.hallyn at ubuntu.com>
  *
+ * Pattern, value setting support and general cleanup:
+ * Copyright © 2017 Maciej S. Szmigiero <mail at maciej.szmigiero.name>
+ *
  * When a user logs in, this pam module will create cgroups which
- * the user may administer, for any controllers listed on the command
- * line or, if none are listed, then all available controllers.
+ * the user may administer, for any controllers (comma separated) provided
+ * to the "--controllers" (or "-c") command line option, or, if this
+ * option is missing or set to "all", for all the available controllers.
+ *
+ * Names of the created cgroups are configurable via a "--pattern" (or "-p")
+ * command line option.
+ * This pattern-derived name for these cgroups will then have a suffix
+ * consisting of a sequential integer appended until the final name - one
+ * common for all the controllers - is unique for all of them (this can be
+ * disabled using a "--pattern-no-idx-suffix" option).
+ * A "--max-idx" (or "-m") option sets how large this sequential number is
+ * allowed to grow (numbers are reused once "their" cgroups no longer
+ * exist).
+ *
+ * By default, the created cgroups will be named "user/%u/0" for the first
+ * session (where "%u" will be replaced by the user name), "user/%u/1" for
+ * the second, etc.
+ * All the created cgroups will have "remove on empty" setting enabled for
+ * them.
+ *
+ * The created final cgroups can optionally have values set in them using
+ * one or more "--set-value" (or "-s") options.
+ * For example, adding a "--set-value io,io.weight,600" parameter to the
+ * command line will set in the created final cgroup of the "io" controller
+ * a setting named "io.weight" to a value of "600".
+ *
+ * At the session close (logging out) this module will try to prune the
+ * cgroup hierarchy (remove empty cgroups and their children) starting from
+ * the final cgroup name and then proceeding towards parent cgroups.
+ * A "--prune-depth" command line option sets how deep this prune process goes.
+ * The default value of 2 for a final cgroup name of "user/foo/4" will
+ * remove a cgroup named "user/foo/4" (if it is empty), then will do the same
+ * (check and remove) for a cgroup named "user/foo".
  *
- * The cgroup created will be "user/$user/0" for the first session,
- * "user/$user/1" for the second, etc.
+ * The following special placeholders are recognized in a pattern:
+ * "%u" - user name,
+ * "%U" - user id,
+ * "%g" - user primary group name,
+ * "%G" - user primary group id,
+ * "%p" - current process id (PID),
+ * "%P" - current process parent id (PPID),
+ * "\%" - a literal "%" character.
  *
  * See COPYING file for details.
  */
@@ -21,8 +61,12 @@
 #include <errno.h>
 #include <sys/mount.h>
 #include <sys/types.h>
+#include <sys/param.h>
 #include <sys/stat.h>
+#include <grp.h>
+#include <pthread.h>
 #include <pwd.h>
+#include <limits.h>
 
 #define PAM_SM_SESSION
 #include <security/_pam_macros.h>
@@ -30,39 +74,95 @@
 
 #include <linux/unistd.h>
 
+#include <popt.h>
+
 #include <nih-dbus/dbus_connection.h>
 #include <nih/alloc.h>
 #include <nih/string.h>
 #include <nih/error.h>
+#include <nih/list.h>
+#include <nih/logging.h>
 
 #include "cgmanager.h"
 
+#define MODULE_NAME "PAM-CGM"
+
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+
+#if defined(__GNUC__)
+static void mysyslog(int err, const char *format, ...)
+	__attribute__ ((format (printf, 2, 3)));
+#endif
+
 static void mysyslog(int err, const char *format, ...)
 {
 	va_list args;
 
 	va_start(args, format);
-	openlog("PAM-CGM", LOG_CONS|LOG_PID, LOG_AUTH);
+	openlog(MODULE_NAME, LOG_CONS|LOG_PID, LOG_AUTH);
 	vsyslog(err, format, args);
 	va_end(args);
 	closelog();
 }
 
-extern char *ctrl_list;
+struct value_set {
+	NihList entry;
+
+	char *controller;
+	char *setting, *value;
+};
 
-static void get_active_controllers(void)
+static int value_set_destroy(struct value_set *item)
+{
+	nih_assert(item != NULL);
+
+	if (item->controller != NULL)
+		nih_discard(item->controller);
+
+	if (item->setting != NULL)
+		nih_discard(item->setting);
+
+	if (item->value != NULL)
+		nih_discard(item->value);
+
+	nih_list_destroy(&item->entry);
+
+	return 0;
+};
+
+struct handle_data {
+	bool session_open;
+
+	NihDBusProxy *cgroup_manager;
+	char *ctrl_list;
+
+	char *cpattern;
+	int cpattern_no_idx_suffix;
+	unsigned int cprune_depth;
+	unsigned int cmax_idx;
+
+	NihList values;
+
+	char *cgroup_final_name;
+	bool cgroup_created;
+};
+
+static void get_active_controllers(struct handle_data *hd)
 {
 	int i;
-	nih_local char **list = cgm_list_controllers();
+	nih_local char **list = cgm_list_controllers(hd->cgroup_manager);
+
+	nih_assert(hd->ctrl_list == NULL);
 
 	if (!list) {
 		mysyslog(LOG_NOTICE, "unable to detect controllers");
-		ctrl_list = NIH_MUST( nih_strdup(NULL, "all") );
+		hd->ctrl_list = NIH_MUST( nih_strdup(NULL, "all") );
 		return;
 	}
 	for (i = 0; list[i]; i++) {
-		NIH_MUST( nih_strcat_sprintf(&ctrl_list, NULL, "%s%s",
-			ctrl_list ? "," : "", list[i]) );
+		NIH_MUST( nih_strcat_sprintf(&hd->ctrl_list, NULL, "%s%s",
+					     hd->ctrl_list ? "," : "",
+					     list[i]) );
 	}
 }
 
@@ -84,18 +184,19 @@ static bool is_in_list(char *which, char **list) {
 	return false;
 }
 
-static char *validate_and_dup(const char *arg)
+static char *validate_and_dup(NihDBusProxy *cgroup_manager, const char *arg)
 {
 	nih_local char *d = NIH_MUST( nih_strdup(NULL, arg) );
-	nih_local char **valid_list = cgm_list_controllers();
-	char *tok;
+	nih_local char **valid_list = cgm_list_controllers(cgroup_manager);
+	char *tok, *savetok;
 
 	if (!valid_list) {
 		mysyslog(LOG_ERR, "Failed to get controller list\n");
 		return NULL;
 	}
 
-	for (tok = strtok(d, ","); tok; tok = strtok(NULL, ",")) {
+	for (tok = strtok_r(d, ",", &savetok); tok;
+	     tok = strtok_r(NULL, ",", &savetok)) {
 		if (!is_in_list(tok, valid_list)) {
 			mysyslog(LOG_ERR, "Invalid controller: %s\n", tok);
 			return NULL;
@@ -106,158 +207,658 @@ static char *validate_and_dup(const char *arg)
 
 static bool get_uid_gid(const char *user, uid_t *uid, gid_t *gid)
 {
-	struct passwd *pwent;
+	struct passwd uinfo, *uinfo_out = NULL;
+	nih_local char *ubuf = NIH_MUST( nih_alloc(NULL, 2048) );
+
+	nih_assert(user != NULL);
+	nih_assert(gid != NULL);
+	nih_assert(uid != NULL);
 
-	pwent = getpwnam(user);
-	if (!pwent)
+	getpwnam_r(user, &uinfo, ubuf, 2048, &uinfo_out);
+	if (uinfo_out == NULL)
 		return false;
-	*uid = pwent->pw_uid;
-	*gid = pwent->pw_gid;
+
+	*uid = uinfo.pw_uid;
+	*gid = uinfo.pw_gid;
+
+	return true;
+}
+
+static void prune_cgs(struct handle_data *hd, const char *cpath)
+{
+	nih_local char *cpathw = NIH_MUST( nih_strdup(NULL, cpath) );
+	unsigned int depth;
+
+	for (depth = 0; depth < hd->cprune_depth && cpathw[0] != '\0';
+	     depth++) {
+		char *c;
+		nih_local char **list = cgm_list_children(hd->cgroup_manager,
+							  hd->ctrl_list, cpathw);
+		unsigned int i;
+
+		for (i = 0; list != NULL && list[i]; i++) {
+			nih_local char *cgpath =
+				NIH_MUST( nih_sprintf(NULL, "%s/%s",
+						      cpathw,
+						      list[i]) );
+
+			if (!cgm_cg_has_tasks(hd->cgroup_manager,
+					      hd->ctrl_list, cgpath))
+				cgm_clear_cgroup(hd->cgroup_manager,
+						 hd->ctrl_list, cgpath);
+		}
+
+		if (!cgm_cg_has_tasks(hd->cgroup_manager, hd->ctrl_list,
+				      cpathw))
+			cgm_clear_cgroup(hd->cgroup_manager, hd->ctrl_list,
+					 cpathw);
+
+		c = strrchr(cpathw, '/');
+		if (c == NULL)
+			break;
+
+		while (c >= cpathw && *c == '/') {
+			*c = '\0';
+			c--;
+		}
+	}
+}
+
+/* based on libcgroup src/api.c::cgroup_change_cgroup_flags() */
+static bool get_user_cgroup(const char *pattern, uid_t uid, gid_t gid,
+			    char *output, unsigned int outlen)
+{
+	unsigned int i, j;
+	struct passwd uinfo, *uinfo_out = NULL;
+	nih_local char *ubuf = NULL;
+	struct group grinfo, *grinfo_out = NULL;
+	nih_local char *grbuf = NULL;
+
+	nih_assert(pattern != NULL);
+	nih_assert(output != NULL);
+	nih_assert(outlen >= 2);
+
+	for (j = i = 0;
+	     i < strlen(pattern) &&
+		     (j < outlen - 2);
+	     ++i, ++j) {
+		unsigned int available;
+		int written;
+
+		if (pattern[i] != '%') {
+			if (pattern[i] == '\\')
+				++i;
+
+			output[j] = pattern[i];
+
+			continue;
+		}
+
+		/* How many bytes can we write */
+		available = outlen - j - 2;
+		/* Substitution */
+		switch (pattern[++i]) {
+		case 'U':
+			written = snprintf(output + j,
+					   available,
+					   "%u",
+					   (unsigned int)uid);
+			break;
+		case 'u':
+			if (uinfo_out == NULL) {
+				ubuf = NIH_MUST( nih_alloc(NULL, 2048) );
+				getpwuid_r(uid, &uinfo, ubuf, 2048, &uinfo_out);
+			}
+			if (uinfo_out != NULL)
+				written = snprintf(output + j,
+						   available, "%s",
+						   uinfo.pw_name);
+			else
+				written = snprintf(output + j,
+						   available,
+						   "%u",
+						   (unsigned int)uid);
+			break;
+		case 'G':
+			written = snprintf(output + j,
+					   available, "%u",
+					   (unsigned int)gid);
+			break;
+		case 'g':
+			if (grinfo_out == NULL) {
+				grbuf = NIH_MUST( nih_alloc(NULL, 2048) );
+				getgrgid_r(gid, &grinfo, grbuf, 2048, &grinfo_out);
+			}
+			if (grinfo_out != NULL)
+				written = snprintf(output + j,
+						   available, "%s",
+						   grinfo.gr_name);
+			else
+				written = snprintf(output + j,
+						   available, "%u",
+						   (unsigned int)gid);
+			break;
+		case 'P':
+			written = snprintf(output + j,
+					   available, "%d",
+					   (int)getppid());
+			break;
+		case 'p':
+			written = snprintf(output + j,
+					   available, "%d",
+					   (int)getpid());
+			break;
+		default:
+			written = 0;
+		}
+
+		if (written > 0 && written > available)
+			written = available;
+		/*
+		 * written < 1 only when either error occurred
+		 * during snprintf or if no substitution was
+		 * made at all. In both cases, we want to just
+		 * copy input string.
+		 */
+		if(written < 1) {
+			output[j] = '%';
+			if(available > 1)
+				output[++j] =
+					pattern[i];
+		} else {
+			/*
+			 * In next iteration, we will write
+			 * just after the substitution, but j
+			 * will get incremented in the
+			 * meantime.
+			 */
+			j += written - 1;
+		}
+	}
+
+	output[j] = '\0';
 
 	return true;
 }
 
-#define DIRNAMSZ 200
-static int handle_login(const char *user)
+static void set_values(struct handle_data *hd, const char *cgroup)
+{
+	char *clist[2] = { hd->ctrl_list, NULL };
+
+	NIH_LIST_FOREACH(&hd->values, entry) {
+		struct value_set *value = (struct value_set *)entry;
+
+		nih_assert(value->controller != NULL);
+		if (!is_in_list(value->controller, clist))
+			continue;
+
+		nih_assert(value->setting != NULL);
+		nih_assert(value->value != NULL);
+		if (!cgm_cg_set_value(hd->cgroup_manager, value->controller,
+				      cgroup, value->setting, value->value)) {
+			mysyslog(LOG_WARNING,
+				 "failed to set %s = %s in cgroup %s (ctr %s)\n",
+				 value->setting, value->value, cgroup,
+				 value->controller);
+		}
+	}
+}
+
+static int handle_login(struct handle_data *hd, const char *user)
 {
-	int idx = 0, ret;
-	int existed = 1;
-	size_t ulen = strlen("user/") + strlen(user);
-	size_t len = ulen + 50; // Just make sure there's room for "user/$user or an <integer>"
 	uid_t uid = 0;
 	gid_t gid = 0;
-	nih_local char *cg = NIH_MUST( nih_alloc(NULL, len) );
+	unsigned int idx;
+	nih_local char *cpath = NULL;
+	char *cpath_end, *cpath_last_part;
+	unsigned int cpath_space;
 
 	if (!get_uid_gid(user, &uid, &gid)) {
 		mysyslog(LOG_ERR, "failed to get uid and gid for %s\n", user);
 		return PAM_SESSION_ERR;
 	}
 
-	memset(cg, 0, len);
-	strcpy(cg, user);
-
-	ret = snprintf(cg, len, "user/%s", user);
-	if (ret < 0 || ret >= len)
-		return PAM_SESSION_ERR;
-
-	if (!cgm_create(cg, &existed)) {
-		mysyslog(LOG_ERR, "failed to create cgroup %s\n", cg);
+	cpath = NIH_MUST( nih_alloc(NULL, MAXPATHLEN) );
+	if (!get_user_cgroup(hd->cpattern, uid, gid, cpath, MAXPATHLEN)) {
+		mysyslog(LOG_ERR,
+			 "failed to get cgroup name for %s\n", user);
 		return PAM_SESSION_ERR;
 	}
 
-	if (existed == 0) {
-		if (!cgm_autoremove(cg)) {
-			mysyslog(LOG_ERR, "Warning: failed to set autoremove on %s\n", cg);
+	do {
+		char *end, *begin_cur;
+		nih_local char *cpath_part = NULL;
+		bool first;
+
+		end = strrchr(cpath, '/');
+		/* no '/'s in path - no intermediate cgroups */
+		if (end == NULL) {
+			cpath_last_part = cpath;
+			break;
 		}
-	}
 
-	if (!cgm_enter(cg)) {
-		mysyslog(LOG_ERR, "failed to enter cgroup %s\n", cg);
-		return PAM_SESSION_ERR;
-	}
+		cpath_part = NIH_MUST( nih_alloc(NULL, end - cpath + 1) );
+
+		begin_cur = cpath;
+
+		if (*begin_cur == '/')
+			/* position at the last '/' char of prefix of '/'s */
+			while (begin_cur < end && *(begin_cur + 1) == '/')
+				begin_cur++;
+
+		if (begin_cur >= end) {
+			/* the whole path contains just '/'s */
+			cpath_last_part = cpath;
+			break;
+		} else
+			cpath_last_part = end + 1;
+
+		first = true;
+		while (1) {
+			char *begin_cur_search, *end_cur;
+			int existed;
+
+			if (first) {
+				/* ignore '/' at the very first position in the cgroup path */
+				begin_cur_search = begin_cur + 1;
+				first = false;
+			} else
+				begin_cur_search = begin_cur;
+
+			end_cur = strchr(begin_cur_search, '/');
+			if (end_cur == NULL || end_cur > end)
+				break;
+
+			/* include all the suffix '/'s in the path */
+			while (end_cur < end && *(end_cur + 1) == '/')
+				end_cur++;
+
+			memcpy(cpath_part, begin_cur, end_cur - begin_cur);
+			cpath_part[end_cur - begin_cur] = '\0';
+			if (!cgm_create(hd->cgroup_manager, hd->ctrl_list,
+					cpath_part, &existed)) {
+				mysyslog(LOG_ERR,
+					 "failed to create intermediate user cgroup %s\n",
+					 cpath_part);
+				return PAM_SESSION_ERR;
+			}
+
+			if (!cgm_enter(hd->cgroup_manager, hd->ctrl_list,
+				       cpath_part)) {
+				mysyslog(LOG_ERR, "failed to enter intermediate user cgroup %s\n",
+					 cpath_part);
+
+				if (existed != 1)
+					cgm_autoremove(hd->cgroup_manager,
+						       hd->ctrl_list,
+						       cpath_part);
+
+				return PAM_SESSION_ERR;
+			}
+
+			if (existed != 1 && !cgm_autoremove(hd->cgroup_manager,
+							    hd->ctrl_list,
+							    ""))
+				mysyslog(LOG_ERR,
+					 "Warning: failed to set autoremove on %s\n",
+					 cpath_part);
+
+			begin_cur = end_cur + 1;
+		}
+	} while (0);
 
-	while (idx >= 0) {
-		sprintf(cg, "%d", idx);
-		if (!cgm_create(cg, &existed)) {
-			mysyslog(LOG_ERR, "failed to create a user cgroup\n");
-			return PAM_SESSION_ERR;
+	nih_assert(strlen(cpath) < MAXPATHLEN);
+	cpath_end = cpath + strlen(cpath);
+	cpath_space = MAXPATHLEN - strlen(cpath);
+
+	nih_assert(hd->cmax_idx >= 1);
+	for (idx = 0; idx < hd->cmax_idx; idx++) {
+		int existed;
+
+		if (!hd->cpattern_no_idx_suffix) {
+			snprintf(cpath_end,
+				 cpath_space, "%u", idx);
+			cpath[MAXPATHLEN - 1] = '\0';
 		}
 
-		if (existed == 1) {
-			idx++;
-			continue;
+		if (!cgm_create(hd->cgroup_manager, hd->ctrl_list,
+				cpath_last_part, &existed)) {
+			mysyslog(LOG_ERR,
+				 "failed to create a user cgroup %s\n",
+				 cpath_last_part);
+			goto ret_fail;
 		}
 
-		if (!cgm_chown(cg, uid, gid)) {
-			mysyslog(LOG_ERR, "Warning: failed to chown %s\n", cg);
+		if (!hd->cpattern_no_idx_suffix && existed == 1)
+			continue;
+
+		if (existed != 1) {
+			if (!cgm_chown(hd->cgroup_manager, hd->ctrl_list,
+				       cpath_last_part, uid, gid))
+				mysyslog(LOG_ERR,
+					 "Warning: failed to chown %s\n",
+					 cpath_last_part);
+
+			set_values(hd, cpath_last_part);
 		}
 
-		if (!cgm_autoremove(cg)) {
-			mysyslog(LOG_ERR, "Warning: failed to set autoremove on %s\n", cg);
+		if (!cgm_enter(hd->cgroup_manager, hd->ctrl_list,
+			       cpath_last_part)) {
+			mysyslog(LOG_ERR, "failed to enter user cgroup %s\n",
+				 cpath_last_part);
+
+			if (existed != 1)
+				cgm_autoremove(hd->cgroup_manager,
+					       hd->ctrl_list,
+					       cpath_last_part);
+
+			goto ret_fail;
 		}
 
-		if (!cgm_enter(cg)) {
-			mysyslog(LOG_ERR, "failed to enter user cgroup %s\n", cg);
-			return PAM_SESSION_ERR;
+		if (existed != 1 && !cgm_autoremove(hd->cgroup_manager,
+						    hd->ctrl_list,
+						    ""))
+			mysyslog(LOG_ERR,
+				 "Warning: failed to set autoremove on %s\n",
+				 cpath_last_part);
+
+		nih_assert(hd->cgroup_final_name == NULL);
+		hd->cgroup_final_name = NIH_MUST( nih_strdup(NULL, cpath) );
+		hd->cgroup_created = existed != 1;
+
+		return PAM_SUCCESS;
+	}
+
+	mysyslog(LOG_ERR, "max idx reached, cgroup not created\n");
+
+ret_fail:
+	if (cpath != NULL)
+		prune_cgs(hd, cpath);
+
+	return PAM_SESSION_ERR;
+}
+
+static bool process_options_build_ctrls(struct handle_data *hd, int argc,
+					const char **argv)
+{
+	char *controllers = NULL;
+	char *cpattern = "user/%u/";
+	int prune_depth = 2; /* prune user name and session idx cgroups by default */;
+	int max_idx = 100;
+	poptContext poptCtx;
+	int poptRet;
+	bool ret = false;
+
+	const struct poptOption options[] = {
+		{ "controllers", 'c', POPT_ARG_STRING, &controllers, 0, NULL,
+		  NULL },
+
+		{ "pattern", 'p', POPT_ARG_STRING, &cpattern, 0, NULL, NULL },
+		{ "pattern-no-idx-suffix", '\0', POPT_ARG_NONE,
+		  &hd->cpattern_no_idx_suffix, 0, NULL, NULL },
+
+		{ "prune-depth", '\0', POPT_ARG_INT, &prune_depth, 0, NULL,
+		  NULL },
+		{ "max-idx", 'm', POPT_ARG_INT, &max_idx, 0, NULL, NULL },
+
+		{ "set-value", 's', POPT_ARG_STRING, NULL, 's', NULL, NULL },
+
+		POPT_TABLEEND
+	};
+
+	poptCtx = poptGetContext(MODULE_NAME, argc, argv, options,
+				 POPT_CONTEXT_KEEP_FIRST);
+	while ((poptRet = poptGetNextOpt(poptCtx)) > 0) {
+		if (poptRet == 's') {
+			char *arg = poptGetOptArg(poptCtx);
+			char *setting, *val;
+			struct value_set *entry;
+
+			if (arg == NULL) {
+				mysyslog(LOG_ERR,
+					 "cannot get set-value arg, ignoring\n");
+				continue;
+			}
+
+			setting = strchr(arg, ',');
+			if (setting == NULL) {
+				mysyslog(LOG_ERR,
+					 "cannot get controller from set-value arg\n");
+				free(arg);
+				goto ret_free;
+			}
+			*setting = '\0';
+			setting++;
+
+			val = strchr(setting, ',');
+			if (val == NULL) {
+				mysyslog(LOG_ERR,
+					 "cannot get value from set-value arg\n");
+				free(arg);
+				goto ret_free;
+			}
+			*val = '\0';
+			val++;
+
+			entry = NIH_MUST( nih_alloc(NULL, sizeof(*entry)) );
+			nih_list_init(&entry->entry);
+			entry->controller = NIH_MUST( nih_strdup(NULL, arg) );
+			entry->setting = NIH_MUST( nih_strdup(NULL, setting) );
+			entry->value = NIH_MUST( nih_strdup(NULL, val) );
+
+			nih_list_add(&hd->values, &entry->entry);
+			nih_alloc_set_destructor(entry, value_set_destroy);
+
+			free(arg);
+		} else
+			mysyslog(LOG_WARNING, "unknown popt return value %d\n",
+				 poptRet);
+	}
+	if (poptRet != -1) {
+		mysyslog(LOG_ERR, "%s: %s\n",
+			 poptBadOption(poptCtx, POPT_BADOPTION_NOALIAS),
+			 poptStrerror(poptRet));
+		goto ret_free;
+	}
+
+	if (controllers == NULL || strcmp(controllers, "all") == 0)
+		get_active_controllers(hd);
+	else {
+		hd->ctrl_list = validate_and_dup(hd->cgroup_manager,
+						 controllers);
+		if (hd->ctrl_list == NULL) {
+			mysyslog(LOG_ERR, "bad controller arguments\n");
+			goto ret_free;
 		}
-		break;
 	}
 
-	return PAM_SUCCESS;
+	hd->cpattern = NIH_MUST( nih_strdup(NULL, cpattern) );
+
+	if (prune_depth < 0) {
+		mysyslog(LOG_ERR, "prune depth can't be negative\n");
+		goto ret_free;
+	}
+	hd->cprune_depth = prune_depth;
+
+	if (max_idx < 1) {
+		mysyslog(LOG_ERR, "max idx must be at least 1\n");
+		goto ret_free;
+	}
+	hd->cmax_idx = max_idx;
+
+	ret = true;
+
+ret_free:
+	poptFreeContext(poptCtx);
+	return ret;
+}
+
+static void hd_cleanup(pam_handle_t *pamh, void *data, int error_status)
+{
+	struct handle_data *hd = data;
+
+	if (hd->ctrl_list != NULL)
+		nih_discard(hd->ctrl_list);
+
+	if (hd->cpattern != NULL)
+		nih_discard(hd->cpattern);
+
+	if (hd->cgroup_final_name != NULL)
+		nih_discard(hd->cgroup_final_name);
+
+	if (hd->cgroup_manager != NULL)
+		mysyslog(LOG_ERR,
+			 "cleaning up a handle that is still connected - bad\n");
+
+	while (!NIH_LIST_EMPTY(&hd->values))
+		nih_free(hd->values.next);
+
+	nih_discard(hd);
 }
 
 int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc,
-		const char **argv)
+			const char **argv)
 {
+	const void *hd_ptr;
+	struct handle_data *hd;
 	const char *PAM_user = NULL;
 	int ret;
 
-	if (!cgm_dbus_connect()) {
+	if (pam_get_data(pamh, MODULE_NAME, &hd_ptr) != PAM_SUCCESS) {
+		hd = NIH_MUST( nih_alloc(NULL, sizeof(*hd)) );
+		memset(hd, 0, sizeof(*hd));
+		nih_list_init(&hd->values);
+
+		ret = pam_set_data(pamh, MODULE_NAME, hd, hd_cleanup);
+		if (ret != PAM_SUCCESS) {
+			nih_discard(hd);
+			mysyslog(LOG_ERR, "cannot set handle data (%d)\n", ret);
+			return ret;
+		}
+	} else
+		hd = (struct handle_data *)hd_ptr;
+
+	nih_assert(hd != NULL);
+
+	if (hd->session_open) {
+		mysyslog(LOG_ERR,
+			 "this PAM handle already has an open session\n");
+		return PAM_SYSTEM_ERR;
+	}
+
+	if (!cgm_dbus_connect(&hd->cgroup_manager)) {
 		mysyslog(LOG_ERR, "Failed to connect to cgmanager\n");
-		return PAM_SESSION_ERR;
+		ret = PAM_SESSION_ERR;
+		goto ret_exit;
 	}
-	if (argc > 1 && strcmp(argv[0], "-c") == 0) {
-		ctrl_list = validate_and_dup(argv[1]);
-		if (!ctrl_list) {
-			cgm_dbus_disconnect();
-			mysyslog(LOG_ERR, "PAM-CGM: bad controller arguments\n");
-			return PAM_SESSION_ERR;
-		}
+
+	if (pthread_mutex_lock(&mutex) != 0) {
+		mysyslog(LOG_ERR, "unable to lock mutex\n");
+		ret = PAM_SESSION_ERR;
+		goto ret_disc;
 	}
-	if (!ctrl_list)
-		get_active_controllers();
-	cgm_escape();
+
+	if (!process_options_build_ctrls(hd, argc, argv)) {
+		ret = PAM_SESSION_ERR;
+		goto ret_unlock;
+	}
+
+	do {
+		char *ctrls_new;
+		if (!cgm_escape(hd->cgroup_manager, hd->ctrl_list, &ctrls_new,
+				NULL)) {
+			mysyslog(LOG_ERR, "cannot escape into root cgroups\n");
+			ret = PAM_SESSION_ERR;
+			goto ret_unlock;
+		}
+
+		nih_discard(hd->ctrl_list);
+		hd->ctrl_list = ctrls_new;
+	} while (0);
 
 	ret = pam_get_user(pamh, &PAM_user, NULL);
 	if (ret != PAM_SUCCESS) {
-		cgm_dbus_disconnect();
-		mysyslog(LOG_ERR, "PAM-CGM: couldn't get user\n");
-		return PAM_SESSION_ERR;
+		mysyslog(LOG_ERR, "couldn't get user\n");
+		goto ret_unlock;
 	}
 
-	ret = handle_login(PAM_user);
-	cgm_dbus_disconnect();
+	ret = handle_login(hd, PAM_user);
+
+ret_unlock:
+	pthread_mutex_unlock(&mutex);
+
+ret_disc:
+	cgm_dbus_disconnect(&hd->cgroup_manager);
+
+ret_exit:
+	if (ret == PAM_SUCCESS)
+		hd->session_open = true;
+	else
+		pam_set_data(pamh, MODULE_NAME, NULL, NULL);
+
 	return ret;
 }
 
-static void prune_user_cgs(const char *user)
+void do_close_session(struct handle_data *hd)
 {
-	nih_local char **list = NULL;
-	nih_local char *path = NULL;
-	int i;
+	char *ctrls_new;
 
-	path = NIH_MUST( nih_sprintf(NULL, "user/%s", user) );
-	list = cgm_list_children(path);
-	if (!list)
+	if (pthread_mutex_lock(&mutex) != 0) {
+		mysyslog(LOG_ERR, "unable to lock mutex\n");
 		return;
-	for (i = 0; list[i]; i++) {
-		nih_local char *cgpath = NIH_MUST( nih_sprintf(NULL, "%s/%s", path, list[i]) );
-		if (!cgm_cg_has_tasks(cgpath))
-			cgm_clear_cgroup(cgpath);
 	}
-	if (!cgm_cg_has_tasks(path))
-		cgm_clear_cgroup(path);
+
+	if (!cgm_escape(hd->cgroup_manager, hd->ctrl_list, &ctrls_new,
+			NULL)) {
+		mysyslog(LOG_ERR, "cannot escape into root cgroups on session close\n");
+		goto ret_unlock;
+	}
+
+	nih_discard(hd->ctrl_list);
+	hd->ctrl_list = ctrls_new;
+
+	if (hd->cgroup_created) {
+		nih_assert(hd->cgroup_final_name != NULL);
+		if (!cgm_cg_has_tasks(hd->cgroup_manager, hd->ctrl_list,
+				      hd->cgroup_final_name))
+			cgm_clear_cgroup(hd->cgroup_manager,
+					 hd->ctrl_list, hd->cgroup_final_name);
+	}
+
+	if (hd->cgroup_final_name != NULL)
+		prune_cgs(hd, hd->cgroup_final_name);
+
+ret_unlock:
+	pthread_mutex_unlock(&mutex);
 }
 
 int pam_sm_close_session(pam_handle_t *pamh, int flags, int argc,
-		const char **argv)
+			 const char **argv)
 {
-	const char *PAM_user = NULL;
-	int ret = pam_get_user(pamh, &PAM_user, NULL);
+	const void *hd_ptr;
+	struct handle_data *hd;
+	int ret;
 
+	ret = pam_get_data(pamh, MODULE_NAME, &hd_ptr);
 	if (ret != PAM_SUCCESS) {
-		mysyslog(LOG_ERR, "PAM-CGM: couldn't get user\n");
-		return PAM_SESSION_ERR;
+		mysyslog(LOG_ERR, "cannot get handle data (%d)\n", ret);
+		return ret;
+	} else
+		hd = (struct handle_data *)hd_ptr;
+
+	if (!hd->session_open) {
+		mysyslog(LOG_ERR,
+			 "this PAM handle session isn't open (concurrency problem?)\n");
+		return PAM_SYSTEM_ERR;
 	}
 
-	if (cgm_dbus_connect()) {
-		if (argc > 1 && strcmp(argv[0], "-c") == 0)
-			ctrl_list = validate_and_dup(argv[1]);
-		if (!ctrl_list)
-			get_active_controllers();
-		cgm_escape();
-		prune_user_cgs(PAM_user);
-		cgm_dbus_disconnect();
+	if (cgm_dbus_connect(&hd->cgroup_manager)) {
+		do_close_session(hd);
+		cgm_dbus_disconnect(&hd->cgroup_manager);
 	}
+
+	hd->session_open = false;
+	pam_set_data(pamh, MODULE_NAME, NULL, NULL);
+
 	return PAM_SUCCESS;
 }


More information about the lxc-devel mailing list