RunApp: safer chdir. Overhauled "sparn a child and get its output" in general

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

3 files changed, 33 insertions, 73 deletions

diff --git a/lib/Plugins/CCpp.cpp b/lib/Plugins/CCpp.cpp
index 1a01c01..26562be 100644
--- a/lib/Plugins/CCpp.cpp
+++ b/lib/Plugins/CCpp.cpp
@@ -99,54 +99,28 @@ static string concat_str_vector(char **strings)
 /* Returns status. See `man 2 wait` for status information. */
 static int ExecVP(char **pArgs, uid_t uid, int redirect_stderr, string& pOutput)
 {
-    int pipeout[2];
-    pid_t child;
-
-    xpipe(pipeout);
-    child = fork();
-    if (child == -1)
-    {
-        perror_msg_and_die("fork");
-    }
-    if (child == 0)
-    {
-        VERB1 log("Executing: %s", concat_str_vector(pArgs).c_str());
-        close(pipeout[0]); /* read side of the pipe */
-        xmove_fd(pipeout[1], STDOUT_FILENO);
-        /* Make sure stdin is safely open to nothing */
-        xmove_fd(xopen("/dev/null", O_RDONLY), STDIN_FILENO);
-
-        struct passwd* pw = getpwuid(uid);
-        gid_t gid = pw ? pw->pw_gid : uid;
-        setgroups(1, &gid);
-        xsetregid(gid, gid);
-        xsetreuid(uid, uid);
-        setsid();
-
-        /* Nuke everything which may make setlocale() switch to non-POSIX locale:
-         * we need to avoid having gdb output in some obscure language.
-         */
-        unsetenv("LANG");
-        unsetenv("LC_ALL");
-        unsetenv("LC_COLLATE");
-        unsetenv("LC_CTYPE");
-        unsetenv("LC_MESSAGES");
-        unsetenv("LC_MONETARY");
-        unsetenv("LC_NUMERIC");
-        unsetenv("LC_TIME");
-
-        if (redirect_stderr)
-        {
-            /* We want parent to see errors in the same stream */
-            xdup2(STDOUT_FILENO, STDERR_FILENO);
-        }
-        execvp(pArgs[0], pArgs);
-        /* VERB1 since sometimes we expect errors here */
-        VERB1 perror_msg("Can't execute '%s'", pArgs[0]);
-        exit(1);
-    }
+    /* Nuke everything which may make setlocale() switch to non-POSIX locale:
+     * we need to avoid having gdb output in some obscure language.
+     */
+    static const char *const unsetenv_vec[] = {
+        "LANG",
+        "LC_ALL",
+        "LC_COLLATE",
+        "LC_CTYPE",
+        "LC_MESSAGES",
+        "LC_MONETARY",
+        "LC_NUMERIC",
+        "LC_TIME",
+        NULL
+    };
+
+    int flags = EXECFLG_INPUT_NUL | EXECFLG_OUTPUT | EXECFLG_SETGUID | EXECFLG_SETSID | EXECFLG_QUIET;
+    if (redirect_stderr)
+        flags |= EXECFLG_ERR2OUT;
+    VERB1 flags &= ~EXECFLG_QUIET;
 
-    close(pipeout[1]); /* write side of the pipe */
+    int pipeout[2];
+    pid_t child = fork_execv_on_steroids(flags, pArgs, pipeout, (char**)unsetenv_vec, /*dir:*/ NULL, uid);
 
     int r;
     char buff[1024];
@@ -155,8 +129,8 @@ static int ExecVP(char **pArgs, uid_t uid, int redirect_stderr, string& pOutput)
         buff[r] = '\0';
         pOutput += buff;
     }
-
     close(pipeout[0]);
+
     int status;
     waitpid(child, &status, 0); /* prevent having zombie child process */
 
diff --git a/lib/Plugins/RunApp.cpp b/lib/Plugins/RunApp.cpp
index f7cbc01..c1f725b 100644
--- a/lib/Plugins/RunApp.cpp
+++ b/lib/Plugins/RunApp.cpp
@@ -33,7 +33,8 @@ using namespace std;
 
 void CActionRunApp::Run(const char *pActionDir, const char *pArgs)
 {
-    /* Don't update_client() - actions run at crash time */
+    /* Don't update_client() - actions run at crash time, there is no client
+     * to talk to at that point */
     log("RunApp('%s','%s')", pActionDir, pArgs);
 
     vector_string_t args;
@@ -45,38 +46,21 @@ void CActionRunApp::Run(const char *pActionDir, const char *pArgs)
         return;
     }
 
-//FIXME: need to be able to escape " in .conf
-    /* Chdir to the dump dir. Command can analyze component and such.
+    /* NB: we chdir to the dump dir. Command can analyze component and such.
      * Example:
      * test x"`cat component`" = x"xorg-x11-apps" && cp /var/log/Xorg.0.log .
      */
-//Can do it using chdir() in child if we'd open-code popen
-    string cd_and_cmd = ssprintf("cd '%s'; %s", pActionDir, cmd);
-    VERB1 log("RunApp: executing '%s'", cd_and_cmd.c_str());
-    FILE *fp = popen(cd_and_cmd.c_str(), "r");
-    if (fp == NULL)
-    {
-        /* Happens only on resource starvation (fork fails or out-of-mem) */
-        return;
-    }
-
-//FIXME: RunApp("gzip -9 </var/log/Xorg.0.log", "Xorg.0.log.gz") fails
-//since we mangle NULs.
-    string output;
-    char line[1024];
-    while (fgets(line, 1024, fp) != NULL)
-    {
-        if (args.size() > FILENAME)
-            output += line;
-    }
-    pclose(fp);
+    size_t cmd_out_size;
+    char *cmd_out = run_in_shell_and_save_output(/*flags:*/ 0, cmd, pActionDir, &cmd_out_size);
 
     if (args.size() > FILENAME)
     {
         CDebugDump dd;
         dd.Open(pActionDir);
-        dd.SaveText(args[FILENAME].c_str(), output.c_str());
+        dd.SaveBinary(args[FILENAME].c_str(), cmd_out, cmd_out_size);
     }
+
+    free(cmd_out);
 }
 
 PLUGIN_INFO(ACTION,
diff --git a/lib/Plugins/SOSreport.cpp b/lib/Plugins/SOSreport.cpp
index 6f23165..899b446 100644
--- a/lib/Plugins/SOSreport.cpp
+++ b/lib/Plugins/SOSreport.cpp
@@ -91,7 +91,9 @@ void CActionSOSreport::Run(const char *pActionDir, const char *pArgs)
     update_client(_("running sosreport: %s"), command.c_str());
     std::string output = command;
     output += '\n';
-    output += popen_and_save_output(command.c_str());
+    char *command_out = run_in_shell_and_save_output(/*flags:*/ 0, command.c_str(), /*dir:*/ NULL, /*size_p:*/ NULL);
+    output += command_out;
+    free(command_out);
     update_client(_("done running sosreport"));
 
     std::string sosreport_filename = ParseFilename(output);