make heavier use of process_lock (v2)

pthread_mutex_lock() will only return an error if it was set to
PTHREAD_MUTEX_ERRORCHECK and we are recursively calling it (and
would otherwise have deadlocked).  If that's the case then log a
message for future debugging and exit.  Trying to "recover" is
nonsense at that point.

process_lock() was held over too long a time in lxcapi_start()
in the daemonize case.  (note the non-daemonized case still needs a
check to enforce that it must NOT be called while threaded).  Add
process_lock() at least across all open/close/socket() calls.

Anything done after a fork() doesn't need the locks as it is no
longer threaded - so some open/close/dups()s are not locked for
that reason.  However, some common functions are called from both
threaded and non-threaded contexts.  So after doing a fork(), do
a possibly-extraneous process_unlock() to make sure that, if we
were forked while pthread mutex was held, we aren't deadlocked by
nobody.

Tested that lp:~serge-hallyn/+junk/lxc-test still works with this
patch.

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>
Tested-by: S.Çağlar Onur <caglar@10ur.org>
Acked-by: Stéphane Graber <stgraber@ubuntu.com>

src/lxc/af_unix.c

@@ -30,6 +30,7 @@
 #include <sys/un.h>
 
 #include "log.h"
+#include "lxclock.h"
 
 lxc_log_define(lxc_af_unix, lxc);
 
@@ -42,7 +43,9 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	if (flags & O_TRUNC)
 		unlink(path);
 
+	process_lock();
 	fd = socket(PF_UNIX, type, 0);
+	process_unlock();
 	if (fd < 0)
 		return -1;
 
@@ -57,7 +60,9 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 	if (path[0]) {
 		len = strlen(path);
 		if (len >= sizeof(addr.sun_path)) {
+			process_lock();
 			close(fd);
+			process_unlock();
 			errno = ENAMETOOLONG;
 			return -1;
 		}
@@ -66,14 +71,18 @@ int lxc_af_unix_open(const char *path, int type, int flags)
 
 	if (bind(fd, (struct sockaddr *)&addr, sizeof(addr))) {
 		int tmp = errno;
+		process_lock();
 		close(fd);
+		process_unlock();
 		errno = tmp;
 		return -1;
 	}
 
 	if (type == SOCK_STREAM && listen(fd, 100)) {
 		int tmp = errno;
+		process_lock();
 		close(fd);
+		process_unlock();
 		errno = tmp;
 		return -1;
 	}
@@ -90,7 +99,9 @@ int lxc_af_unix_close(int fd)
 	    addr.sun_path[0])
 		unlink(addr.sun_path);
 
+	process_lock();
 	close(fd);
+	process_unlock();
 
 	return 0;
 }
@@ -100,7 +111,9 @@ int lxc_af_unix_connect(const char *path)
 	int fd;
 	struct sockaddr_un addr;
 
+	process_lock();
 	fd = socket(PF_UNIX, SOCK_STREAM, 0);
+	process_unlock();
 	if (fd < 0)
 		return -1;
 
@@ -113,7 +126,9 @@ int lxc_af_unix_connect(const char *path)
 
 	if (connect(fd, (struct sockaddr *)&addr, sizeof(addr))) {
 		int tmp = errno;
+		process_lock();
 		close(fd);
+		process_unlock();
 		errno = tmp;
 		return -1;
 	}

src/lxc/apparmor.c

@@ -28,6 +28,7 @@
 
 #include "log.h"
 #include "apparmor.h"
+#include "lxclock.h"
 
 lxc_log_define(lxc_apparmor, lxc);
 
@@ -53,7 +54,9 @@ extern char *aa_get_profile(pid_t pid)
 		return NULL;
 	}
 again:
+	process_lock();
 	f = fopen(path, "r");
+	process_unlock();
 	if (!f) {
 		SYSERROR("opening %s\n", path);
 		if (buf)
@@ -65,11 +68,15 @@ extern char *aa_get_profile(pid_t pid)
 	memset(buf, 0, sz);
 	if (!buf) {
 		ERROR("out of memory");
+		process_lock();
 		fclose(f);
+		process_unlock();
 		return NULL;
 	}
 	ret = fread(buf, 1, sz - 1, f);
+	process_lock();
 	fclose(f);
+	process_unlock();
 	if (ret >= sz)
 		goto again;
 	if (ret < 0) {
@@ -108,11 +115,15 @@ static int check_apparmor_enabled(void)
 	ret = stat(AA_MOUNT_RESTR, &statbuf);
 	if (ret != 0)
 		return 0;
+	process_lock();
 	fin = fopen(AA_ENABLED_FILE, "r");
+	process_unlock();
 	if (!fin)
 		return 0;
 	ret = fscanf(fin, "%c", &e);
+	process_lock();
 	fclose(fin);
+	process_unlock();
 	if (ret == 1 && e == 'Y')
 		return 1;
 	return 0;

src/lxc/attach.c

@@ -50,6 +50,7 @@
 #include "utils.h"
 #include "commands.h"
 #include "cgroup.h"
+#include "lxclock.h"
 
 #if HAVE_SYS_PERSONALITY_H
 #include <sys/personality.h>
@@ -78,7 +79,9 @@ struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 	/* read capabilities */
 	snprintf(proc_fn, MAXPATHLEN, "/proc/%d/status", pid);
 
+	process_lock();
 	proc_file = fopen(proc_fn, "r");
+	process_unlock();
 	if (!proc_file) {
 		SYSERROR("Could not open %s", proc_fn);
 		goto out_error;
@@ -95,7 +98,9 @@ struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 
 	if (line)
 		free(line);
+	process_lock();
 	fclose(proc_file);
+	process_unlock();
 
 	if (!found) {
 		SYSERROR("Could not read capability bounding set from %s", proc_fn);
@@ -106,14 +111,18 @@ struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid)
 	/* read personality */
 	snprintf(proc_fn, MAXPATHLEN, "/proc/%d/personality", pid);
 
+	process_lock();
 	proc_file = fopen(proc_fn, "r");
+	process_unlock();
 	if (!proc_file) {
 		SYSERROR("Could not open %s", proc_fn);
 		goto out_error;
 	}
 
 	ret = fscanf(proc_file, "%lx", &info->personality);
+	process_lock();
 	fclose(proc_file);
+	process_unlock();
 
 	if (ret == EOF || ret == 0) {
 		SYSERROR("Could not read personality from %s", proc_fn);
@@ -162,15 +171,19 @@ int lxc_attach_to_ns(pid_t pid, int which)
 		}
 
 		snprintf(path, MAXPATHLEN, "/proc/%d/ns/%s", pid, ns[i]);
+		process_lock();
 		fd[i] = open(path, O_RDONLY | O_CLOEXEC);
+		process_unlock();
 		if (fd[i] < 0) {
 			saved_errno = errno;
 
 			/* close all already opened file descriptors before
 			 * we return an error, so we don't leak them
 			 */
+			process_lock();
 			for (j = 0; j < i; j++)
 				close(fd[j]);
+			process_unlock();
 
 			errno = saved_errno;
 			SYSERROR("failed to open '%s'", path);
@@ -190,7 +203,9 @@ int lxc_attach_to_ns(pid_t pid, int which)
 			return -1;
 		}
 
+		process_lock();
 		close(fd[i]);
+		process_unlock();
 	}
 
 	return 0;
@@ -378,14 +393,18 @@ char *lxc_attach_getpwshell(uid_t uid)
 	 * getent program, and we need to capture its
 	 * output, so we use a pipe for that purpose
 	 */
+	process_lock();
 	ret = pipe(pipes);
+	process_unlock();
 	if (ret < 0)
 		return NULL;
 
 	pid = fork();
 	if (pid < 0) {
+		process_lock();
 		close(pipes[0]);
 		close(pipes[1]);
+		process_unlock();
 		return NULL;
 	}
 
@@ -397,9 +416,13 @@ char *lxc_attach_getpwshell(uid_t uid)
 		int found = 0;
 		int status;
 
+		process_lock();
 		close(pipes[1]);
+		process_unlock();
 
+		process_lock();
 		pipe_f = fdopen(pipes[0], "r");
+		process_unlock();
 		while (getline(&line, &line_bufsz, pipe_f) != -1) {
 			char *token;
 			char *saveptr = NULL;
@@ -456,7 +479,9 @@ char *lxc_attach_getpwshell(uid_t uid)
 		}
 
 		free(line);
+		process_lock();
 		fclose(pipe_f);
+		process_unlock();
 	again:
 		if (waitpid(pid, &status, 0) < 0) {
 			if (errno == EINTR)
@@ -489,6 +514,7 @@ char *lxc_attach_getpwshell(uid_t uid)
 			NULL
 		};
 
+		process_unlock(); // we're no longer sharing
 		close(pipes[0]);
 
 		/* we want to capture stdout */
@@ -651,7 +677,9 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 	 *   close socket                                 close socket
 	 *                                                run program
 	 */
+	process_lock();
 	ret = socketpair(PF_LOCAL, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets);
+	process_unlock();
 	if (ret < 0) {
 		SYSERROR("could not set up required IPC mechanism for attaching");
 		free(cwd);
@@ -689,7 +717,9 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		/* inital thread, we close the socket that is for the
 		 * subprocesses
 		 */
+		process_lock();
 		close(ipc_sockets[1]);
+		process_unlock();
 		free(cwd);
 
 		/* get pid from intermediate process */
@@ -761,7 +791,9 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 
 		/* now shut down communication with child, we're done */
 		shutdown(ipc_sockets[0], SHUT_RDWR);
+		process_lock();
 		close(ipc_sockets[0]);
+		process_unlock();
 		free(init_ctx->aa_profile);
 		free(init_ctx);
 
@@ -778,14 +810,17 @@ int lxc_attach(const char* name, const char* lxcpath, lxc_attach_exec_t exec_fun
 		 * otherwise the pid we're waiting for may never exit
 		 */
 		shutdown(ipc_sockets[0], SHUT_RDWR);
+		process_lock();
 		close(ipc_sockets[0]);
+		process_unlock();
 		if (to_cleanup_pid)
 			(void) wait_for_pid(to_cleanup_pid);
 		free(init_ctx->aa_profile);
 		free(init_ctx);
 		return -1;
 	}
 
+	process_unlock(); // we're no longer sharing
 	/* first subprocess begins here, we close the socket that is for the
 	 * initial thread
 	 */