Merge pull request #101 from ahippo/master

run_buffer(): unblock all signals for spawned scripts.

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,7 +826,7 @@ 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) {
@@ -834,10 +834,10 @@ static int lvm_compare_lv_attr(const char *path, int pos, const char expected) {
 		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

@@ -280,12 +280,12 @@ static struct caps_opt caps_opt[] = {};
 
 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");
@@ -296,18 +296,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.