9:22 a.m.
Currently the code is doing a redundant fork when passed the
COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN flag. The additional sub-process does a
chroot() which has no effect because all file handles are already open at that
point, then simply copies its input to its output.
This change simply replaces the above with a dup2 of the passed file handle to
STDIN of the command process.
---
daemon/guestfsd.c | 89 +++++--------------------------------------------------
1 file changed, 7 insertions(+), 82 deletions(-)
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 4b3dd5f..7e83a2a 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -789,8 +789,8 @@ commandrvf (char **stdoutput, char **stderror, int flags,
size_t so_size = 0, se_size = 0;
int so_fd[2], se_fd[2];
int flag_copy_stdin = flags & COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN;
- int stdin_fd[2] = { -1, -1 };
- pid_t pid, stdin_pid = -1;
+ int flag_copy_fd = flags & COMMAND_FLAG_FD_MASK;
+ pid_t pid = -1;
int r, quit, i;
fd_set rset, rset2;
char buf[256];
@@ -820,13 +820,6 @@ commandrvf (char **stdoutput, char **stderror, int flags,
abort ();
}
- if (flag_copy_stdin) {
- if (pipe (stdin_fd) == -1) {
- error (0, errno, "pipe");
- abort ();
- }
- }
-
pid = fork ();
if (pid == -1) {
error (0, errno, "fork");
@@ -838,9 +831,7 @@ commandrvf (char **stdoutput, char **stderror, int flags,
signal (SIGPIPE, SIG_DFL);
close (0);
if (flag_copy_stdin) {
- dup2 (stdin_fd[PIPE_READ], STDIN_FILENO);
- close (stdin_fd[PIPE_READ]);
- close (stdin_fd[PIPE_WRITE]);
+ dup2 (flag_copy_fd, STDIN_FILENO);
} else {
/* Set stdin to /dev/null (ignore failure) */
ignore_value (open ("/dev/null", O_RDONLY|O_CLOEXEC));
@@ -860,60 +851,6 @@ commandrvf (char **stdoutput, char **stderror, int flags,
_exit (EXIT_FAILURE);
}
- if (flag_copy_stdin) {
- int fd = flags & COMMAND_FLAG_FD_MASK;
-
- stdin_pid = fork ();
- if (stdin_pid == -1) {
- error (0, errno, "fork");
- abort ();
- }
-
- if (stdin_pid == 0) { /* Child process copying stdin. */
- close (so_fd[PIPE_READ]);
- close (so_fd[PIPE_WRITE]);
- close (se_fd[PIPE_READ]);
- close (se_fd[PIPE_WRITE]);
-
- close (STDOUT_FILENO);
- dup2 (stdin_fd[PIPE_WRITE], STDOUT_FILENO);
- close (stdin_fd[PIPE_READ]);
- close (stdin_fd[PIPE_WRITE]);
-
- if (chroot (sysroot) == -1) {
- perror ("chroot");
- _exit (EXIT_FAILURE);
- }
-
- ssize_t n;
- char buffer[BUFSIZ];
- while ((n = read (fd, buffer, sizeof buffer)) > 0) {
- if (xwrite (STDOUT_FILENO, buffer, n) == -1)
- /* EPIPE error indicates the command process has exited
- * early. If the command process fails that will be caught
- * by the daemon, and if not, then it's not an error.
- */
- _exit (errno == EPIPE ? EXIT_SUCCESS : EXIT_FAILURE);
- }
-
- if (n == -1) {
- perror ("read");
- _exit (EXIT_FAILURE);
- }
-
- if (close (fd) == -1) {
- perror ("close");
- _exit (EXIT_FAILURE);
- }
-
- _exit (EXIT_SUCCESS);
- }
-
- close (fd);
- close (stdin_fd[PIPE_READ]);
- close (stdin_fd[PIPE_WRITE]);
- }
-
/* Parent process. */
close (so_fd[PIPE_WRITE]);
close (se_fd[PIPE_WRITE]);
@@ -950,8 +887,8 @@ commandrvf (char **stdoutput, char **stderror, int flags,
}
close (so_fd[PIPE_READ]);
close (se_fd[PIPE_READ]);
+ if (flag_copy_stdin) close (flag_copy_fd);
waitpid (pid, NULL, 0);
- if (stdin_pid >= 0) waitpid (stdin_pid, NULL, 0);
return -1;
}
@@ -1033,21 +970,9 @@ commandrvf (char **stdoutput, char **stderror, int flags,
}
}
- if (flag_copy_stdin) {
- /* Check copy process didn't fail. */
- if (waitpid (stdin_pid, &r, 0) != stdin_pid) {
- perror ("waitpid");
- kill (pid, 9);
- waitpid (pid, NULL, 0);
- return -1;
- }
-
- if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
- fprintf (stderr, "failed copying from input file, see earlier messages (r =
%d)\n", r);
- kill (pid, 9);
- waitpid (pid, NULL, 0);
- return -1;
- }
+ if (flag_copy_stdin && close (flag_copy_fd) == -1) {
+ perror ("close");
+ return -1;
}
/* Get the exit status of the command. */
--
1.7.11.7