Allow reading the jailed process' stdout and stderr.

Also fix some nits while in there.

BUG=None
TEST=libminijail_unittest on alex and lumpy.

Change-Id: I1bd227f196618d275da6e5da4ce91e90a370baa2
Reviewed-on: https://gerrit.chromium.org/gerrit/43460
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Jorge Lucangeli Obes <jorgelo@chromium.org>
diff --git a/libminijail.c b/libminijail.c
index bc65829..9b8d465 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -862,32 +862,64 @@
 	return 0;
 }
 
+int setup_pipe_end(int fds[2], size_t index)
+{
+	if (index > 1)
+		return -1;
+
+	close(fds[1 - index]);
+	return fds[index];
+}
+
+int setup_and_dupe_pipe_end(int fds[2], size_t index, int fd)
+{
+	if (index > 1)
+		return -1;
+
+	close(fds[1 - index]);
+	/* dup2(2) the corresponding end of the pipe into |fd|. */
+	return dup2(fds[index], fd);
+}
+
 int API minijail_run(struct minijail *j, const char *filename,
 		     char *const argv[])
 {
-	return minijail_run_pid_pipe(j, filename, argv, NULL, NULL);
+	return minijail_run_pid_pipes(j, filename, argv,
+				      NULL, NULL, NULL, NULL);
 }
 
 int API minijail_run_pid(struct minijail *j, const char *filename,
 			 char *const argv[], pid_t *pchild_pid)
 {
-	return minijail_run_pid_pipe(j, filename, argv, pchild_pid, NULL);
+	return minijail_run_pid_pipes(j, filename, argv, pchild_pid,
+				      NULL, NULL, NULL);
 }
 
 int API minijail_run_pipe(struct minijail *j, const char *filename,
 			  char *const argv[], int *pstdin_fd)
 {
-	return minijail_run_pid_pipe(j, filename, argv, NULL, pstdin_fd);
+	return minijail_run_pid_pipes(j, filename, argv, NULL, pstdin_fd,
+				      NULL, NULL);
 }
 
 int API minijail_run_pid_pipe(struct minijail *j, const char *filename,
 			      char *const argv[], pid_t *pchild_pid,
 			      int *pstdin_fd)
 {
+	return minijail_run_pid_pipes(j, filename, argv, pchild_pid, pstdin_fd,
+				      NULL, NULL);
+}
+
+int API minijail_run_pid_pipes(struct minijail *j, const char *filename,
+			      char *const argv[], pid_t *pchild_pid,
+			      int *pstdin_fd, int *pstdout_fd, int *pstderr_fd)
+{
 	char *oldenv, *oldenv_copy = NULL;
 	pid_t child_pid;
 	int pipe_fds[2];
 	int stdin_fds[2];
+	int stdout_fds[2];
+	int stderr_fds[2];
 	int ret;
 	/* We need to remember this across the minijail_preexec() call. */
 	int pid_namespace = j->flags.pids;
@@ -918,6 +950,24 @@
 			return -EFAULT;
 	}
 
+	/*
+	 * If we want to read from the child process' standard output,
+	 * create the pipe(2) now.
+	 */
+	if (pstdout_fd) {
+		if (pipe(stdout_fds))
+			return -EFAULT;
+	}
+
+	/*
+	 * If we want to read from the child process' standard error,
+	 * create the pipe(2) now.
+	 */
+	if (pstderr_fd) {
+		if (pipe(stderr_fds))
+			return -EFAULT;
+	}
+
 	/* Use sys_clone() if and only if we're creating a pid namespace.
 	 *
 	 * tl;dr: WARNING: do not mix pid namespaces and multithreading.
@@ -996,10 +1046,25 @@
 		 * If we want to write to the child process' standard input,
 		 * set up the write end of the pipe.
 		 */
-		if (pstdin_fd) {
-			close(stdin_fds[0]);	/* read endpoint */
-			*pstdin_fd = stdin_fds[1];
-		}
+		if (pstdin_fd)
+			*pstdin_fd = setup_pipe_end(stdin_fds,
+						    1	/* write end */);
+
+		/*
+		 * If we want to read from the child process' standard output,
+		 * set up the read end of the pipe.
+		 */
+		if (pstdout_fd)
+			*pstdout_fd = setup_pipe_end(stdout_fds,
+						     0	/* read end */);
+
+		/*
+		 * If we want to read from the child process' standard error,
+		 * set up the read end of the pipe.
+		 */
+		if (pstderr_fd)
+			*pstderr_fd = setup_pipe_end(stderr_fds,
+						     0	/* read end */);
 
 		return 0;
 	}
@@ -1010,12 +1075,31 @@
 	 * set up the read end of the pipe.
 	 */
 	if (pstdin_fd) {
-		close(stdin_fds[1]);	/* write endpoint */
-		/* dup2(2) the read end of the pipe into stdin. */
-		if (dup2(stdin_fds[0], 0))
+		if (setup_and_dupe_pipe_end(stdin_fds, 0 /* read end */,
+					    STDIN_FILENO) < 0)
 			die("failed to set up stdin pipe");
 	}
 
+	/*
+	 * If we want to read from the jailed process' standard output,
+	 * set up the write end of the pipe.
+	 */
+	if (pstdout_fd) {
+		if (setup_and_dupe_pipe_end(stdout_fds, 1 /* write end */,
+					    STDOUT_FILENO) < 0)
+			die("failed to set up stdout pipe");
+	}
+
+	/*
+	 * If we want to read from the jailed process' standard error,
+	 * set up the write end of the pipe.
+	 */
+	if (pstderr_fd) {
+		if (setup_and_dupe_pipe_end(stderr_fds, 1 /* write end */,
+					    STDERR_FILENO) < 0)
+			die("failed to set up stderr pipe");
+	}
+
 	/* Drop everything that cannot be inherited across execve. */
 	minijail_preexec(j);
 	/* Jail this process and its descendants... */
diff --git a/libminijail.h b/libminijail.h
index c231572..05d2c15 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -118,6 +118,19 @@
 			  char *const argv[], pid_t *pchild_pid,
 			  int *pstdin_fd);
 
+/* Run the specified command in the given minijail, execve(3)-style.
+ * Update |*pchild_pid| with the pid of the child.
+ * Update |*pstdin_fd| with a fd that allows writing to the child's
+ * standard input.
+ * Update |*pstdout_fd| with a fd that allows reading from the child's
+ * standard output.
+ * Update |*pstderr_fd| with a fd that allows reading from the child's
+ * standard error.
+ */
+int minijail_run_pid_pipes(struct minijail *j, const char *filename,
+			   char *const argv[], pid_t *pchild_pid,
+			   int *pstdin_fd, int *pstdout_fd, int *pstderr_fd);
+
 /* Kill the specified minijail. The minijail must have been created with pid
  * namespacing; if it was, all processes inside it are atomically killed.
  */
diff --git a/libminijail_unittest.c b/libminijail_unittest.c
index a2304a2..193c1dc 100644
--- a/libminijail_unittest.c
+++ b/libminijail_unittest.c
@@ -147,18 +147,72 @@
   pid_t pid;
   int child_stdin;
   int mj_run_ret;
-  int write_ret;
+  ssize_t write_ret;
   int status;
+  char filename[] = "test/read_stdin";
+  char *argv[2];
+  argv[0] = filename;
+  argv[1] = NULL;
 
   struct minijail *j = minijail_new();
-  mj_run_ret = minijail_run_pid_pipe(j, "test/read_stdin",
-                                     NULL, &pid, &child_stdin);
+  mj_run_ret = minijail_run_pid_pipe(j, argv[0], argv, &pid, &child_stdin);
   EXPECT_EQ(mj_run_ret, 0);
   write_ret = write(child_stdin, "test\n", strlen("test\n"));
   EXPECT_GT(write_ret, -1);
 
   waitpid(pid, &status, 0);
+  ASSERT_TRUE(WIFEXITED(status));
+  EXPECT_EQ(WEXITSTATUS(status), 0);
 
+  minijail_destroy(j);
+}
+
+TEST(test_minijail_run_pid_pipes) {
+  pid_t pid;
+  int child_stdin, child_stdout, child_stderr;
+  int mj_run_ret;
+  ssize_t write_ret, read_ret;
+  const size_t buf_len = 128;
+  char buf[buf_len];
+  int status;
+  char filename[] = "/bin/cat";
+  char teststr[] = "test\n";
+  size_t teststr_len = strlen(teststr);
+  char *argv[4];
+
+  struct minijail *j = minijail_new();
+
+  argv[0] = filename;
+  argv[1] = NULL;
+  mj_run_ret = minijail_run_pid_pipes(j, argv[0], argv,
+                                      &pid, &child_stdin, &child_stdout, NULL);
+  EXPECT_EQ(mj_run_ret, 0);
+
+  write_ret = write(child_stdin, teststr, teststr_len);
+  EXPECT_EQ(write_ret, (int)teststr_len);
+
+  read_ret = read(child_stdout, buf, 8);
+  EXPECT_EQ(read_ret, (int)teststr_len);
+  buf[teststr_len] = 0;
+  EXPECT_EQ(strcmp(buf, teststr), 0);
+
+  EXPECT_EQ(kill(pid, SIGTERM), 0);
+  waitpid(pid, &status, 0);
+  ASSERT_TRUE(WIFSIGNALED(status));
+  EXPECT_EQ(WTERMSIG(status), SIGTERM);
+
+  argv[0] = "/bin/sh";
+  argv[1] = "-c";
+  argv[2] = "echo test >&2";
+  argv[3] = NULL;
+  mj_run_ret = minijail_run_pid_pipes(j, argv[0], argv, &pid, &child_stdin,
+                                      &child_stdout, &child_stderr);
+  EXPECT_EQ(mj_run_ret, 0);
+
+  read_ret = read(child_stderr, buf, buf_len);
+  EXPECT_GE(read_ret, (int)teststr_len);
+
+  waitpid(pid, &status, 0);
   ASSERT_TRUE(WIFEXITED(status));
   EXPECT_EQ(WEXITSTATUS(status), 0);