>From 0949c47c03456dae91c15740731b1350296b0497 Mon Sep 17 00:00:00 2001 From: Bruno Haible Date: Wed, 2 Dec 2020 17:52:00 +0100 Subject: [PATCH 2/2] spawn-pipe: Allow caller to specify directory for the subprocess. * lib/spawn-pipe.h (create_pipe_out, create_pipe_in, create_pipe_bidi): Add directory argument. * lib/spawn-pipe.c: Include canonicalize.h, filename.h, findprog.h. (create_pipe): Add directory argument. If specified, resolve the program file name and make it absolute, first. Pass the directory to spawnpvech and posix_spawn_file_actions_addchdir. (create_pipe_bidi, create_pipe_in, create_pipe_out): Add directory argument. * modules/spawn-pipe (Depends-on): Add canonicalize, filename, findprog-in, posix_spawn, posix_spawn_file_actions_addchdir. * tests/test-spawn-pipe-main.c (test_pipe): Update. * NEWS: Mention the change. * lib/csharpcomp.c (compile_csharp_using_mono, compile_csharp_using_sscli): Update. * lib/javacomp.c (is_envjavac_gcj, is_envjavac_gcj43, is_gcj_present, is_gcj_43): Update. * lib/javaversion.c (execute_and_read_line): Update. * lib/pipe-filter-gi.c (pipe_filter_gi_create): Update. * lib/pipe-filter-ii.c (pipe_filter_ii_execute): Update. --- ChangeLog | 23 +++++++++ NEWS | 5 ++ lib/csharpcomp.c | 11 +++-- lib/javacomp.c | 16 +++--- lib/javaversion.c | 4 +- lib/pipe-filter-gi.c | 2 +- lib/pipe-filter-ii.c | 2 +- lib/spawn-pipe.c | 114 +++++++++++++++++++++++++++++++++++-------- lib/spawn-pipe.h | 7 +++ modules/spawn-pipe | 5 ++ tests/test-spawn-pipe-main.c | 2 +- 11 files changed, 154 insertions(+), 37 deletions(-) diff --git a/ChangeLog b/ChangeLog index b5ea083..b32f190 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,28 @@ 2020-12-02 Bruno Haible + spawn-pipe: Allow caller to specify directory for the subprocess. + * lib/spawn-pipe.h (create_pipe_out, create_pipe_in, create_pipe_bidi): + Add directory argument. + * lib/spawn-pipe.c: Include canonicalize.h, filename.h, findprog.h. + (create_pipe): Add directory argument. If specified, resolve the program + file name and make it absolute, first. Pass the directory to spawnpvech + and posix_spawn_file_actions_addchdir. + (create_pipe_bidi, create_pipe_in, create_pipe_out): Add directory + argument. + * modules/spawn-pipe (Depends-on): Add canonicalize, filename, + findprog-in, posix_spawn, posix_spawn_file_actions_addchdir. + * tests/test-spawn-pipe-main.c (test_pipe): Update. + * NEWS: Mention the change. + * lib/csharpcomp.c (compile_csharp_using_mono, + compile_csharp_using_sscli): Update. + * lib/javacomp.c (is_envjavac_gcj, is_envjavac_gcj43, is_gcj_present, + is_gcj_43): Update. + * lib/javaversion.c (execute_and_read_line): Update. + * lib/pipe-filter-gi.c (pipe_filter_gi_create): Update. + * lib/pipe-filter-ii.c (pipe_filter_ii_execute): Update. + +2020-12-02 Bruno Haible + execute: Allow caller to specify directory for the subprocess. * lib/execute.h (execute): Add directory argument. * lib/execute.c: Include canonicalize.h, filename.h, findprog.h. diff --git a/NEWS b/NEWS index 445266d..747165a 100644 --- a/NEWS +++ b/NEWS @@ -60,6 +60,11 @@ User visible incompatible changes Date Modules Changes +2020-12-02 spawn-pipe The functions 'create_pipe_out', 'create_pipe_in', + 'create_pipe_bidi' now take a 4th argument + 'const char *directory'. To maintain the previous + behaviour, insert NULL as additional 4th argument. + 2020-12-02 execute The function 'execute' now takes a 4th argument 'const char *directory'. To maintain the previous behaviour, insert NULL as additional 4th argument. diff --git a/lib/csharpcomp.c b/lib/csharpcomp.c index cf68c5a..57d2084 100644 --- a/lib/csharpcomp.c +++ b/lib/csharpcomp.c @@ -84,8 +84,8 @@ compile_csharp_using_mono (const char * const *sources, argv[0] = "mcs"; argv[1] = "--version"; argv[2] = NULL; - child = create_pipe_in ("mcs", "mcs", argv, DEV_NULL, true, true, false, - fd); + child = create_pipe_in ("mcs", "mcs", argv, NULL, + DEV_NULL, true, true, false, fd); mcs_present = false; if (child != -1) { @@ -193,7 +193,8 @@ compile_csharp_using_mono (const char * const *sources, free (command); } - child = create_pipe_in ("mcs", "mcs", argv, NULL, false, true, true, fd); + child = create_pipe_in ("mcs", "mcs", argv, NULL, + NULL, false, true, true, fd); /* Read the subprocess output, copying it to stderr. Drop the last line if it starts with "Compilation succeeded". */ @@ -270,8 +271,8 @@ compile_csharp_using_sscli (const char * const *sources, argv[0] = "csc"; argv[1] = "-help"; argv[2] = NULL; - child = create_pipe_in ("csc", "csc", argv, DEV_NULL, true, true, false, - fd); + child = create_pipe_in ("csc", "csc", argv, NULL, + DEV_NULL, true, true, false, fd); csc_present = false; if (child != -1) { diff --git a/lib/javacomp.c b/lib/javacomp.c index fec5180..c24bb69 100644 --- a/lib/javacomp.c +++ b/lib/javacomp.c @@ -664,8 +664,8 @@ is_envjavac_gcj (const char *javac) argv[1] = "-c"; argv[2] = command; argv[3] = NULL; - child = create_pipe_in (javac, BOURNE_SHELL, argv, DEV_NULL, true, true, - false, fd); + child = create_pipe_in (javac, BOURNE_SHELL, argv, NULL, + DEV_NULL, true, true, false, fd); if (child == -1) goto failed; @@ -746,8 +746,8 @@ is_envjavac_gcj43 (const char *javac) argv[1] = "-c"; argv[2] = command; argv[3] = NULL; - child = create_pipe_in (javac, BOURNE_SHELL, argv, DEV_NULL, true, true, - false, fd); + child = create_pipe_in (javac, BOURNE_SHELL, argv, NULL, + DEV_NULL, true, true, false, fd); if (child == -1) goto failed; @@ -1414,8 +1414,8 @@ is_gcj_present (void) argv[0] = "gcj"; argv[1] = "--version"; argv[2] = NULL; - child = create_pipe_in ("gcj", "gcj", argv, DEV_NULL, true, true, - false, fd); + child = create_pipe_in ("gcj", "gcj", argv, NULL, + DEV_NULL, true, true, false, fd); gcj_present = false; if (child != -1) { @@ -1530,8 +1530,8 @@ is_gcj_43 (void) argv[0] = "gcj"; argv[1] = "--version"; argv[2] = NULL; - child = create_pipe_in ("gcj", "gcj", argv, DEV_NULL, true, true, - false, fd); + child = create_pipe_in ("gcj", "gcj", argv, NULL, + DEV_NULL, true, true, false, fd); gcj_43 = false; if (child != -1) { diff --git a/lib/javaversion.c b/lib/javaversion.c index fd99291..e50c499 100644 --- a/lib/javaversion.c +++ b/lib/javaversion.c @@ -65,8 +65,8 @@ execute_and_read_line (const char *progname, int exitstatus; /* Open a pipe to the JVM. */ - child = create_pipe_in (progname, prog_path, prog_argv, DEV_NULL, false, - true, false, fd); + child = create_pipe_in (progname, prog_path, prog_argv, NULL, + DEV_NULL, false, true, false, fd); if (child == -1) return false; diff --git a/lib/pipe-filter-gi.c b/lib/pipe-filter-gi.c index f7f8f6e..7528053 100644 --- a/lib/pipe-filter-gi.c +++ b/lib/pipe-filter-gi.c @@ -498,7 +498,7 @@ pipe_filter_gi_create (const char *progname, /* Open a bidirectional pipe to a subprocess. */ filter->child = create_pipe_bidi (progname, prog_path, (char **) prog_argv, - null_stderr, true, exit_on_error, + NULL, null_stderr, true, exit_on_error, filter->fd); filter->progname = progname; filter->null_stderr = null_stderr; diff --git a/lib/pipe-filter-ii.c b/lib/pipe-filter-ii.c index b29f802..384a59b 100644 --- a/lib/pipe-filter-ii.c +++ b/lib/pipe-filter-ii.c @@ -271,7 +271,7 @@ pipe_filter_ii_execute (const char *progname, /* Open a bidirectional pipe to a subprocess. */ child = create_pipe_bidi (progname, prog_path, (char **) prog_argv, - null_stderr, true, exit_on_error, + NULL, null_stderr, true, exit_on_error, fd); if (child == -1) return -1; diff --git a/lib/spawn-pipe.c b/lib/spawn-pipe.c index a4c4d39..209bbf5 100644 --- a/lib/spawn-pipe.c +++ b/lib/spawn-pipe.c @@ -32,8 +32,11 @@ #include #include +#include "canonicalize.h" #include "error.h" #include "fatal-signal.h" +#include "filename.h" +#include "findprog.h" #include "unistd-safer.h" #include "wait-process.h" #include "gettext.h" @@ -120,12 +123,62 @@ nonintr_open (const char *pathname, int oflag, mode_t mode) static pid_t create_pipe (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, bool pipe_stdin, bool pipe_stdout, const char *prog_stdin, const char *prog_stdout, bool null_stderr, bool slave_process, bool exit_on_error, int fd[2]) { + int saved_errno; + char *prog_path_to_free = NULL; + + if (directory != NULL) + { + /* If a change of directory is requested, make sure PROG_PATH is absolute + before we do so. This is needed because + - posix_spawn and posix_spawnp are required to resolve a relative + PROG_PATH *after* changing the directory. See + : + "if this pathname does not start with a it shall be + interpreted relative to the working directory of the child + process _after_ all file_actions have been performed." + But this would be a surprising application behaviour, possibly + even security relevant. + - For the Windows CreateProcess() function, it is unspecified whether + a relative file name is interpreted to the parent's current + directory or to the specified directory. See + */ + if (! IS_ABSOLUTE_FILE_NAME (prog_path)) + { + const char *resolved_prog = + find_in_given_path (prog_path, getenv ("PATH"), false); + if (resolved_prog == NULL) + goto fail_with_errno; + if (resolved_prog != prog_path) + prog_path_to_free = (char *) resolved_prog; + prog_path = resolved_prog; + + if (! IS_ABSOLUTE_FILE_NAME (prog_path)) + { + char *absolute_prog = + canonicalize_filename_mode (prog_path, CAN_MISSING | CAN_NOLINKS); + if (absolute_prog == NULL) + { + saved_errno = errno; + free (prog_path_to_free); + goto fail_with_saved_errno; + } + free (prog_path_to_free); + prog_path_to_free = absolute_prog; + prog_path = absolute_prog; + + if (! IS_ABSOLUTE_FILE_NAME (prog_path)) + abort (); + } + } + } + #if (defined _WIN32 && ! defined __CYGWIN__) || defined __KLIBC__ /* Native Windows API. @@ -139,7 +192,6 @@ create_pipe (const char *progname, int nulloutfd; int stdinfd; int stdoutfd; - int saved_errno; /* FIXME: Need to free memory allocated by prepare_spawn. */ prog_argv = prepare_spawn (prog_argv); @@ -227,16 +279,17 @@ create_pipe (const char *progname, (HANDLE) _get_osfhandle (null_stderr ? nulloutfd : STDERR_FILENO); child = spawnpvech (P_NOWAIT, prog_path, (const char **) prog_argv, - (const char **) environ, NULL, + (const char **) environ, directory, stdin_handle, stdout_handle, stderr_handle); if (child == -1 && errno == ENOEXEC) { /* prog is not a native executable. Try to execute it as a shell script. Note that prepare_spawn() has already prepended a hidden element "sh.exe" to prog_argv. */ + prog_argv[0] = prog_path; --prog_argv; child = spawnpvech (P_NOWAIT, prog_argv[0], (const char **) prog_argv, - (const char **) environ, NULL, + (const char **) environ, directory, stdin_handle, stdout_handle, stderr_handle); } } @@ -256,6 +309,13 @@ create_pipe (const char *progname, close (ifd[1]); # else /* __KLIBC__ */ + if (!(directory == NULL && strcmp (directory, ".") == 0)) + { + /* A directory argument is not supported in this implementation. */ + saved_errno = EINVAL; + goto fail_with_saved_errno; + } + int orig_stdin; int orig_stdout; int orig_stderr; @@ -330,17 +390,15 @@ create_pipe (const char *progname, close (ifd[1]); # endif + free (prog_path_to_free); + if (child == -1) { - if (exit_on_error || !null_stderr) - error (exit_on_error ? EXIT_FAILURE : 0, saved_errno, - _("%s subprocess failed"), progname); if (pipe_stdout) close (ifd[0]); if (pipe_stdin) close (ofd[1]); - errno = saved_errno; - return -1; + goto fail_with_saved_errno; } if (pipe_stdout) @@ -426,6 +484,9 @@ create_pipe (const char *progname, prog_stdout, O_WRONLY, 0)) != 0) + || (directory != NULL + && (err = posix_spawn_file_actions_addchdir (&actions, + directory))) || (slave_process && ((err = posix_spawnattr_init (&attrs)) != 0 || (attrs_allocated = true, @@ -435,9 +496,13 @@ create_pipe (const char *progname, || (err = posix_spawnattr_setflags (&attrs, POSIX_SPAWN_SETSIGMASK)) != 0))) - || (err = posix_spawnp (&child, prog_path, &actions, - attrs_allocated ? &attrs : NULL, prog_argv, - environ)) + || (err = (directory != NULL + ? posix_spawn (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, + environ) + : posix_spawnp (&child, prog_path, &actions, + attrs_allocated ? &attrs : NULL, prog_argv, + environ))) != 0)) { if (actions_allocated) @@ -446,9 +511,6 @@ create_pipe (const char *progname, posix_spawnattr_destroy (&attrs); if (slave_process) unblock_fatal_signals (); - if (exit_on_error || !null_stderr) - error (exit_on_error ? EXIT_FAILURE : 0, err, - _("%s subprocess failed"), progname); if (pipe_stdout) { close (ifd[0]); @@ -459,8 +521,9 @@ create_pipe (const char *progname, close (ofd[0]); close (ofd[1]); } - errno = err; - return -1; + free (prog_path_to_free); + saved_errno = err; + goto fail_with_saved_errno; } posix_spawn_file_actions_destroy (&actions); if (attrs_allocated) @@ -474,6 +537,7 @@ create_pipe (const char *progname, close (ofd[0]); if (pipe_stdout) close (ifd[1]); + free (prog_path_to_free); if (pipe_stdout) fd[0] = ifd[0]; @@ -482,6 +546,15 @@ create_pipe (const char *progname, return child; #endif + + fail_with_errno: + saved_errno = errno; + fail_with_saved_errno: + if (exit_on_error || !null_stderr) + error (exit_on_error ? EXIT_FAILURE : 0, saved_errno, + _("%s subprocess failed"), progname); + errno = saved_errno; + return -1; } /* Open a bidirectional pipe. @@ -495,11 +568,12 @@ create_pipe (const char *progname, pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, bool null_stderr, bool slave_process, bool exit_on_error, int fd[2]) { - pid_t result = create_pipe (progname, prog_path, prog_argv, + pid_t result = create_pipe (progname, prog_path, prog_argv, directory, true, true, NULL, NULL, null_stderr, slave_process, exit_on_error, fd); @@ -516,12 +590,13 @@ create_pipe_bidi (const char *progname, pid_t create_pipe_in (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, const char *prog_stdin, bool null_stderr, bool slave_process, bool exit_on_error, int fd[1]) { int iofd[2]; - pid_t result = create_pipe (progname, prog_path, prog_argv, + pid_t result = create_pipe (progname, prog_path, prog_argv, directory, false, true, prog_stdin, NULL, null_stderr, slave_process, exit_on_error, iofd); @@ -540,12 +615,13 @@ create_pipe_in (const char *progname, pid_t create_pipe_out (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, const char *prog_stdout, bool null_stderr, bool slave_process, bool exit_on_error, int fd[1]) { int iofd[2]; - pid_t result = create_pipe (progname, prog_path, prog_argv, + pid_t result = create_pipe (progname, prog_path, prog_argv, directory, true, false, NULL, prog_stdout, null_stderr, slave_process, exit_on_error, iofd); diff --git a/lib/spawn-pipe.h b/lib/spawn-pipe.h index be0f1c8..02856a8 100644 --- a/lib/spawn-pipe.h +++ b/lib/spawn-pipe.h @@ -51,6 +51,10 @@ extern "C" { argv[]. It is a NULL-terminated array. prog_argv[0] should normally be identical to prog_path. + If directory is not NULL, the subprocess is started in that directory. If + prog_path is a relative file name, it resolved before changing to that + directory. The current directory of the current process remains unchanged. + If slave_process is true, the child process will be terminated when its creator receives a catchable fatal signal or exits normally. If slave_process is false, the child process will continue running in this @@ -93,6 +97,7 @@ extern "C" { */ extern pid_t create_pipe_out (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, const char *prog_stdout, bool null_stderr, bool slave_process, bool exit_on_error, int fd[1]); @@ -106,6 +111,7 @@ extern pid_t create_pipe_out (const char *progname, */ extern pid_t create_pipe_in (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, const char *prog_stdin, bool null_stderr, bool slave_process, bool exit_on_error, int fd[1]); @@ -134,6 +140,7 @@ extern pid_t create_pipe_in (const char *progname, */ extern pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv, + const char *directory, bool null_stderr, bool slave_process, bool exit_on_error, int fd[2]); diff --git a/modules/spawn-pipe b/modules/spawn-pipe index 2e78170..9d80193 100644 --- a/modules/spawn-pipe +++ b/modules/spawn-pipe @@ -10,20 +10,25 @@ m4/spawn-pipe.m4 Depends-on: dup2 +canonicalize environ error fatal-signal +filename +findprog-in gettext-h msvc-nothrow open pipe2 pipe2-safer spawn +posix_spawn posix_spawnp posix_spawn_file_actions_init posix_spawn_file_actions_addclose posix_spawn_file_actions_adddup2 posix_spawn_file_actions_addopen +posix_spawn_file_actions_addchdir posix_spawn_file_actions_destroy posix_spawnattr_init posix_spawnattr_setsigmask diff --git a/tests/test-spawn-pipe-main.c b/tests/test-spawn-pipe-main.c index 946871b..fe4f94e 100644 --- a/tests/test-spawn-pipe-main.c +++ b/tests/test-spawn-pipe-main.c @@ -52,7 +52,7 @@ test_pipe (const char *prog, bool stderr_closed) argv[0] = (char *) prog; argv[1] = (char *) (stderr_closed ? "1" : "0"); argv[2] = NULL; - pid = create_pipe_bidi (prog, prog, argv, false, true, true, fd); + pid = create_pipe_bidi (prog, prog, argv, NULL, false, true, true, fd); ASSERT (0 <= pid); ASSERT (STDERR_FILENO < fd[0]); ASSERT (STDERR_FILENO < fd[1]); -- 2.7.4