run_buffer(): unblock all signals for spawned scripts.

Currently, all scripts, specified as "lxc.network.script.up", inherit
lxc-execute's signal mask.
This, for example, includes blocked SIGALRM signal which, in turn, makes
alarm(2), sleep(3) and setitimer(2) functions silently unusable in all programs,
invoked in turn by the "lxc.network.script.up".
To fix this, run_buffer() should restore default signal mask prior to
executing "lxc.network.script.up".

A naive implementation would temprorary unblock all signals just before
calling popen() and block them back immediately after it.
But that would result in an immediate delivery of all pending signals just
after their unblocking.
Thus, we should restore default signal mask exactly in child (after fork())
just before calling exec().
To achieve this, a home-brewed popen() alternative is needed.
The added lxc_popen() and lxc_pclose() are mostly taken from glibc with
several simplifications (as we currently need only "re" mode).
The implementation uses Linux-specific pipe2() system-call,
which is only available since Linux 2.6.27 and supported by glibc since
version 2.9 (according to pipe(2) man-page), but this shouldn't be a
problem as lxc requires a fairly recent kernel too.

lxc_popen()/lxc_pclose() are meant to be direct replacements for their
stdio counterparts, so they perform no process_lock() locking
themselves. (as fopen_cloexec() does)
All existing users of popen()/pclose() are converted to the new
lxc_popen()/lxc_pclose().

(mazo: don't clear close-on-exec flag for parent's end;
place the new functions in utils.c;
convert bdev.c to use the new functions;
coding style fixes;
comments fixes;
commit message tweaks)

Signed-off-by: Ivan Bolsunov <bolsunov@telum.ru>
Signed-off-by: Andrey Mazo <mazo@telum.ru>

src/lxc/bdev.c

@@ -501,24 +501,24 @@ struct bdev_ops dir_ops = {
 
 static int zfs_list_entry(const char *path, char *output, size_t inlen)
 {
-	FILE *f;
+	struct lxc_popen_FILE *f;
 	int found=0;
 
 	process_lock();
-	f = popen("zfs list 2> /dev/null", "r");
+	f = lxc_popen("zfs list 2> /dev/null");
 	process_unlock();
 	if (f == NULL) {
 		SYSERROR("popen failed");
 		return 0;
 	}
-	while (fgets(output, inlen, f)) {
+	while (fgets(output, inlen, f->f)) {
 		if (strstr(output, path)) {
 			found = 1;
 			break;
 		}
 	}
 	process_lock();
-	(void) pclose(f);
+	(void) lxc_pclose(f);
 	process_unlock();
 
 	return found;
@@ -813,7 +813,7 @@ static int lvm_umount(struct bdev *bdev)
 }
 
 static int lvm_compare_lv_attr(const char *path, int pos, const char expected) {
-	FILE *f;
+	struct lxc_popen_FILE *f;
 	int ret, len, status, start=0;
 	char *cmd, output[12];
 	const char *lvscmd = "lvs --unbuffered --noheadings -o lv_attr %s 2>/dev/null";
@@ -826,18 +826,18 @@ static int lvm_compare_lv_attr(const char *path, int pos, const char expected) {
 		return -1;
 
 	process_lock();
-	f = popen(cmd, "r");
+	f = lxc_popen(cmd);
 	process_unlock();
 
 	if (f == NULL) {
 		SYSERROR("popen failed");
 		return -1;
 	}
 
-	ret = fgets(output, 12, f) == NULL;
+	ret = fgets(output, 12, f->f) == NULL;
 
 	process_lock();
-	status = pclose(f);
+	status = lxc_pclose(f);
 	process_unlock();
 
 	if (ret || WEXITSTATUS(status))

src/lxc/conf.c

@@ -350,12 +350,12 @@ static char *mkifname(char *template)
 
 static int run_buffer(char *buffer)
 {
-	FILE *f;
+	struct lxc_popen_FILE *f;
 	char *output;
 	int ret;
 
 	process_lock();
-	f = popen(buffer, "r");
+	f = lxc_popen(buffer);
 	process_unlock();
 	if (!f) {
 		SYSERROR("popen failed");
@@ -366,18 +366,18 @@ static int run_buffer(char *buffer)
 	if (!output) {
 		ERROR("failed to allocate memory for script output");
 		process_lock();
-		pclose(f);
+		lxc_pclose(f);
 		process_unlock();
 		return -1;
 	}
 
-	while(fgets(output, LXC_LOG_BUFFER_SIZE, f))
+	while(fgets(output, LXC_LOG_BUFFER_SIZE, f->f))
 		DEBUG("script output: %s", output);
 
 	free(output);
 
 	process_lock();
-	ret = pclose(f);
+	ret = lxc_pclose(f);
 	process_unlock();
 	if (ret == -1) {
 		SYSERROR("Script exited on error");

src/lxc/utils.c

@@ -616,6 +616,139 @@ FILE *fopen_cloexec(const char *path, const char *mode)
 	return ret;
 }
 
+/* must be called with process_lock() held */
+extern struct lxc_popen_FILE *lxc_popen(const char *command)
+{
+	struct lxc_popen_FILE *fp = NULL;
+	int parent_end = -1, child_end = -1;
+	int pipe_fds[2];
+	pid_t child_pid;
+
+	int r = pipe2(pipe_fds, O_CLOEXEC);
+
+	if (r < 0) {
+		ERROR("pipe2 failure");
+		return NULL;
+	}
+
+	parent_end = pipe_fds[0];
+	child_end = pipe_fds[1];
+
+	child_pid = fork();
+
+	if (child_pid == 0) {
+		/* child */
+		int child_std_end = STDOUT_FILENO;
+
+		if (child_end != child_std_end) {
+			/* dup2() doesn't dup close-on-exec flag */
+			dup2(child_end, child_std_end);
+
+			/* it's safe not to close child_end here
+			 * as it's marked close-on-exec anyway
+			 */
+		} else {
+			/*
+			 * The descriptor is already the one we will use.
+			 * But it must not be marked close-on-exec.
+			 * Undo the effects.
+			 */
+			fcntl(child_end, F_SETFD, 0);
+		}
+
+		/*
+		 * Unblock signals.
+		 * This is the main/only reason
+		 * why we do our lousy popen() emulation.
+		 */
+		{
+			sigset_t mask;
+			sigfillset(&mask);
+			sigprocmask(SIG_UNBLOCK, &mask, NULL);
+		}
+
+		execl("/bin/sh", "sh", "-c", command, (char *) NULL);
+		exit(127);
+	}
+
+	/* parent */
+
+	close(child_end);
+	child_end = -1;
+
+	if (child_pid < 0) {
+		ERROR("fork failure");
+		goto error;
+	}
+
+	fp = calloc(1, sizeof(*fp));
+	if (!fp) {
+		ERROR("failed to allocate memory");
+		goto error;
+	}
+
+	fp->f = fdopen(parent_end, "r");
+	if (!fp->f) {
+		ERROR("fdopen failure");
+		goto error;
+	}
+
+	fp->child_pid = child_pid;
+
+	return fp;
+
+error:
+
+	if (fp) {
+		if (fp->f) {
+			fclose(fp->f);
+			parent_end = -1; /* so we do not close it second time */
+		}
+
+		free(fp);
+	}
+
+	if (child_end != -1)
+		close(child_end);
+	if (parent_end != -1)
+		close(parent_end);
+
+	return NULL;
+}
+
+/* must be called with process_lock() held */
+extern int lxc_pclose(struct lxc_popen_FILE *fp)
+{
+	FILE *f = NULL;
+	pid_t child_pid = 0;
+	int wstatus = 0;
+	pid_t wait_pid;
+
+	if (fp) {
+		f = fp->f;
+		child_pid = fp->child_pid;
+		/* free memory (we still need to close file stream) */
+		free(fp);
+		fp = NULL;
+	}
+
+	if (!f || fclose(f)) {
+		ERROR("fclose failure");
+		return -1;
+	}
+
+	do {
+		wait_pid = waitpid(child_pid, &wstatus, 0);
+	} while (wait_pid == -1 && errno == EINTR);
+
+	if (wait_pid == -1) {
+		ERROR("waitpid failure");
+		return -1;
+	}
+
+	return wstatus;
+}
+
 char *lxc_string_replace(const char *needle, const char *replacement, const char *haystack)
 {
 	ssize_t len = -1, saved_len = -1;

src/lxc/utils.h

@@ -158,6 +158,34 @@ static inline int signalfd(int fd, const sigset_t *mask, int flags)
 FILE *fopen_cloexec(const char *path, const char *mode);
 
 
+/* Struct to carry child pid from lxc_popen() to lxc_pclose().
+ * Not an opaque struct to allow direct access to the underlying FILE *
+ * (i.e., struct lxc_popen_FILE *file; fgets(buf, sizeof(buf), file->f))
+ * without additional wrappers.
+ */
+struct lxc_popen_FILE {
+	FILE *f;
+	pid_t child_pid;
+};
+
+/* popen(command, "re") replacement that restores default signal mask
+ * via sigprocmask(2) (unblocks all signals) after fork(2) but prior to calling exec(3).
+ * In short, popen(command, "re") does pipe() + fork()                 + exec()
+ * while lxc_popen(command)       does pipe() + fork() + sigprocmask() + exec().
+ * Must be called with process_lock() held.
+ * Returns pointer to struct lxc_popen_FILE, that should be freed with lxc_pclose().
+ * On error returns NULL.
+ */
+extern struct lxc_popen_FILE *lxc_popen(const char *command);
+
+/* pclose() replacement to be used on struct lxc_popen_FILE *,
+ * returned by lxc_popen().
+ * Waits for associated process to terminate, returns its exit status and
+ * frees resources, pointed to by struct lxc_popen_FILE *.
+ * Must be called with process_lock() held.
+ */
+extern int lxc_pclose(struct lxc_popen_FILE *fp);
+
 /**
  * BUILD_BUG_ON - break compile if a condition is true.
  * @condition: the condition which the compiler should know is false.