fuzz: rework df_exec_cmd_check()

Explicitly use fork() & execl() to avoid the file descriptor
shenanigans, and move & rename the function to util.[ch].

Also, when at it, add a unit test for the function as well.

man/dfuzzer.xml

@@ -117,6 +117,13 @@
                 unsuccessfully, fail message is printed with its return value.</para></listitem>
             </varlistentry>
 
+            <varlistentry>
+                <term><option>--show-command-output</option></term>
+
+                <listitem><para>Don't suppress stdout/stderr of a <replaceable>COMMAND</replaceable>
+                specified via <option>--command=</option></para></listitem>
+            </varlistentry>
+
             <varlistentry>
                 <term><option>-f <replaceable>FILENAME</replaceable></option></term>
                 <term><option>--dictionary=<replaceable>FILENAME</replaceable></option></term>

src/dfuzzer.c

@@ -597,6 +597,7 @@ static void df_print_help(const char *name)
          "  -I --iterations=ITER        Set both the minimum and maximum number of iterations to ITER\n"
          "                              See --max-iterations= and --min-iterations= above\n"
          "  -e --command=COMMAND        Command/script to execute after each method call.\n"
+         "     --show-command-output    Don't suppress stdout/stderr of a COMMAND.\n"
          "  -f --dictionary=FILENAME    Name of a file with custom dictionary which is used as input\n"
          "                              for fuzzed methods before generating random data.\n"
          "\nExamples:\n\n"
@@ -622,30 +623,32 @@ static void df_parse_parameters(int argc, char **argv)
                  * short variant */
                 ARG_SKIP_METHODS = 0x100,
                 ARG_SKIP_PROPERTIES,
+                ARG_SHOW_COMMAND_OUTPUT
         };
 
         static const struct option options[] = {
-                { "buffer-limit",       required_argument,  NULL,   'b'                 },
-                { "debug",              no_argument,        NULL,   'd'                 },
-                { "command",            required_argument,  NULL,   'e'                 },
-                { "string-file",        required_argument,  NULL,   'f'                 },
-                { "help",               no_argument,        NULL,   'h'                 },
-                { "interface",          required_argument,  NULL,   'i'                 },
-                { "list",               no_argument,        NULL,   'l'                 },
-                { "mem-limit",          required_argument,  NULL,   'm'                 },
-                { "bus",                required_argument,  NULL,   'n'                 },
-                { "object",             required_argument,  NULL,   'o'                 },
-                { "property",           required_argument,  NULL,   'p'                 },
-                { "no-suppressions",    no_argument,        NULL,   's'                 },
-                { "method",             required_argument,  NULL,   't'                 },
-                { "verbose",            no_argument,        NULL,   'v'                 },
-                { "log-dir",            required_argument,  NULL,   'L'                 },
-                { "version",            no_argument,        NULL,   'V'                 },
-                { "max-iterations",     required_argument,  NULL,   'x'                 },
-                { "min-iterations",     required_argument,  NULL,   'y'                 },
-                { "iterations",         required_argument,  NULL,   'I'                 },
-                { "skip-methods",       no_argument,        NULL,   ARG_SKIP_METHODS    },
-                { "skip-properties",    no_argument,        NULL,   ARG_SKIP_PROPERTIES },
+                { "buffer-limit",        required_argument,  NULL,   'b'                     },
+                { "debug",               no_argument,        NULL,   'd'                     },
+                { "command",             required_argument,  NULL,   'e'                     },
+                { "string-file",         required_argument,  NULL,   'f'                     },
+                { "help",                no_argument,        NULL,   'h'                     },
+                { "interface",           required_argument,  NULL,   'i'                     },
+                { "list",                no_argument,        NULL,   'l'                     },
+                { "mem-limit",           required_argument,  NULL,   'm'                     },
+                { "bus",                 required_argument,  NULL,   'n'                     },
+                { "object",              required_argument,  NULL,   'o'                     },
+                { "property",            required_argument,  NULL,   'p'                     },
+                { "no-suppressions",     no_argument,        NULL,   's'                     },
+                { "method",              required_argument,  NULL,   't'                     },
+                { "verbose",             no_argument,        NULL,   'v'                     },
+                { "log-dir",             required_argument,  NULL,   'L'                     },
+                { "version",             no_argument,        NULL,   'V'                     },
+                { "max-iterations",      required_argument,  NULL,   'x'                     },
+                { "min-iterations",      required_argument,  NULL,   'y'                     },
+                { "iterations",          required_argument,  NULL,   'I'                     },
+                { "skip-methods",        no_argument,        NULL,   ARG_SKIP_METHODS        },
+                { "skip-properties",     no_argument,        NULL,   ARG_SKIP_PROPERTIES     },
+                { "show-command-output", no_argument,        NULL,   ARG_SHOW_COMMAND_OUTPUT },
                 {}
         };
 
@@ -795,6 +798,9 @@ static void df_parse_parameters(int argc, char **argv)
                         case ARG_SKIP_PROPERTIES:
                                 df_skip_properties = TRUE;
                                 break;
+                        case ARG_SHOW_COMMAND_OUTPUT:
+                                df_fuzz_set_show_command_output(TRUE);
+                                break;
                         default:    // '?'
                                 exit(1);
                                 break;

src/fuzz.c

@@ -20,14 +20,10 @@
  */
 #include <ctype.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <gio/gio.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <sys/wait.h>
 #include <unistd.h>
 
 #include "fuzz.h"
@@ -37,13 +33,13 @@
 #include "util.h"
 
 static guint64 fuzz_buffer_length = MAX_BUFFER_LENGTH;
+static gboolean show_command_output = FALSE;
 /** Pointer on D-Bus interface proxy for calling methods. */
 static GDBusProxy *df_dproxy;
 /** Exceptions counter; if MAX_EXCEPTIONS is reached testing continues
   * with a next method */
 static char df_except_counter = 0;
 
-
 void df_fuzz_set_buffer_length(const guint64 length)
 {
         g_assert(length <= MAX_BUFFER_LENGTH);
@@ -56,6 +52,11 @@ guint64 df_fuzz_get_buffer_length(void)
         return fuzz_buffer_length;
 }
 
+void df_fuzz_set_show_command_output(gboolean value)
+{
+        show_command_output = value;
+}
+
 guint64 df_get_number_of_iterations(const char *signature)
 {
         guint64 iterations = 0;
@@ -169,59 +170,6 @@ static void df_fuzz_write_log(const struct df_dbus_method *method, GVariant *val
         }
 }
 
-/**
- * @function Executes command/script cmd.
- * @param cmd Command/Script to execute
- * @return 0 on successful completition of cmd or when cmd is NULL, value
- * higher than 0 on unsuccessful completition of cmd or -1 on error
- */
-static int df_exec_cmd_check(const char *cmd)
-{
-        if (cmd == NULL)
-                return 0;
-
-        const char *fn = "/dev/null";
-        g_auto(fd_t) stdoutcpy = -1, stderrcpy = -1, fd = -1;
-        int status = 0;
-
-        fd = open(fn, O_RDWR, S_IRUSR | S_IWUSR);
-        if (fd == -1) {
-                perror("open");
-                return -1;
-        }
-
-        // backup std descriptors
-        stdoutcpy = dup(1);
-        if (stdoutcpy < 0)
-                return -1;
-        stderrcpy = dup(2);
-        if (stderrcpy < 0)
-                return -1;
-
-        // make stdout and stderr go to fd
-        if (dup2(fd, 1) < 0)
-                return -1;
-        if (dup2(fd, 2) < 0)
-                return -1;
-        fd = safe_close(fd);      // fd no longer needed
-
-        // execute cmd
-        status = system(cmd);
-
-        // restore std descriptors
-        if (dup2(stdoutcpy, 1) < 0)
-                return -1;
-        stdoutcpy = safe_close(stdoutcpy);
-        if (dup2(stderrcpy, 2) < 0)
-                return -1;
-        stderrcpy = safe_close(stderrcpy);
-
-
-        if (status == -1)
-                return status;
-        return WEXITSTATUS(status);
-}
-
 static int df_check_if_exited(const int pid) {
         g_autoptr(FILE) f = NULL;
         g_autoptr(char) line = NULL;
@@ -373,7 +321,7 @@ int df_fuzz_test_method(
                 /* Convert the floating variant reference into a full one */
                 value = g_variant_ref_sink(value);
                 ret = df_fuzz_call_method(method, value);
-                execr = df_exec_cmd_check(execute_cmd);
+                execr = execute_cmd ? df_execute_external_command(execute_cmd, show_command_output) : 0;
 
                 if (ret < 0) {
                         df_fail("%s  %sFAIL%s [M] %s - unexpected response\n",
@@ -382,7 +330,7 @@ int df_fuzz_test_method(
                 }
 
                 if (execr < 0)
-                        return df_fail_ret(-1, "df_exec_cmd_check() failed: %m");
+                        return df_fail_ret(-1, "df_execute_external_command() failed: %m");
                 else if (execr > 0) {
                         df_fail("%s  %sFAIL%s [M] %s - '%s' returned %s%d%s\n",
                                 ansi_cr(), ansi_red(), ansi_normal(), method->name,

src/fuzz.h

@@ -75,6 +75,7 @@ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(df_dbus_property_t, df_dbus_property_clear)
 
 void df_fuzz_set_buffer_length(const guint64 length);
 guint64 df_fuzz_get_buffer_length(void);
+void df_fuzz_set_show_command_output(gboolean value);
 
 guint64 df_get_number_of_iterations(const char *signature);
 /**

src/util.c

@@ -1,11 +1,18 @@
 /** @file util.c */
 
 #include <errno.h>
+#include <fcntl.h>
 #include <gio/gio.h>
 #include <stdarg.h>
 #include <stdint.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <sys/wait.h>
+#include <sys/stat.h>
+
+#include "util.h"
+#include "log.h"
 
 char *strjoin_real(const char *x, ...) {
         va_list ap;
@@ -59,3 +66,52 @@ int safe_strtoull(const gchar *p, guint64 *ret)
 
         return 0;
 }
+
+int df_execute_external_command(const char *command, gboolean show_output)
+{
+        pid_t pid;
+
+        g_assert(command);
+
+        pid = fork();
+
+        if (pid < 0)
+                return df_fail_ret(-1, "Failed to fork: %m\n");
+        if (pid > 0) {
+                /* Parent process */
+                siginfo_t status;
+
+                for (;;) {
+                        if (waitid(P_PID, pid, &status, WEXITED) < 0) {
+                                if (errno == EINTR)
+                                        continue;
+
+                                return df_fail_ret(-1, "Error when waiting for a child: %m\n");
+                        }
+
+                        break;
+                }
+
+                return status.si_status;
+        }
+
+        /* Child process */
+        g_auto(fd_t) null_fd = -1;
+
+        /* Redirect stdin/stdout/stderr to /dev/null */
+        null_fd = open("/dev/null", O_RDWR);
+        if (null_fd < 0)
+                return df_fail_ret(-1, "Failed to open /dev/null: %m\n");
+
+        for (guint8 i = 0; i < 3; i++) {
+                if (i > 0 && show_output)
+                        break;
+                if (dup2(null_fd, i) < 0)
+                        return df_fail_ret(-1, "Failed to replace fd %d with /dev/null: %m\n", i);
+        }
+
+        if (execl("/bin/sh", "sh", "-c", command, (char*) NULL) < 0)
+                return df_fail_ret(-1, "Failed to execl(): %m\n");
+
+        return 0;
+}

src/util.h

@@ -128,3 +128,5 @@ DEFINE_ANSI_FUNC(cyan,       CYAN);
 DEFINE_ANSI_FUNC(normal,     NORMAL);
 DEFINE_ANSI_FUNC(bold,       BOLD);
 DEFINE_ANSI_FUNC(cr,         CR);
+
+int df_execute_external_command(const char *command, gboolean show_output);

test/meson.build

@@ -1,5 +1,6 @@
 tests += [
         [files('test-rand.c')],
+        [files('test-util.c')],
 ]
 
 # vi: sw=8 ts=8 et:

test/test-util.c

@@ -0,0 +1,30 @@
+#include <gio/gio.h>
+#include <glib.h>
+#include <stdio.h>
+
+#include "util.h"
+#include "log.h"
+
+static void test_df_execute_external_command(void)
+{
+        for (guint8 i = 0; i < 2; i++) {
+                gboolean show_output = i == 0 ? FALSE : TRUE;
+                g_test_message("show_output: %s", show_output ? "TRUE" : "FALSE");
+
+                g_assert_true(df_execute_external_command("true", show_output) == 0);
+                g_assert_true(df_execute_external_command("true; echo hello world; cat /proc/$$/stat", show_output) == 0);
+                g_assert_true(df_execute_external_command("true; echo hello world; false /proc/$$/stat", show_output) > 0);
+                g_assert_true(df_execute_external_command("exit 66", show_output) == 66);
+                g_assert_true(df_execute_external_command("this-should-not-exist", show_output) > 0);
+                g_assert_true(df_execute_external_command("kill -SEGV $$", show_output) > 0);
+        }
+}
+
+int main(int argc, char *argv[])
+{
+        g_test_init(&argc, &argv, NULL);
+
+        g_test_add_func("/df_util/df_execute_external_command", test_df_execute_external_command);
+
+        return g_test_run();
+}