From e6964486d387d618463c67be0180f2b5250c682c Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 5 Jul 2022 11:06:31 +0200 Subject: [PATCH] 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 | 7 +++++ src/dfuzzer.c | 48 +++++++++++++++++++--------------- src/fuzz.c | 68 ++++++------------------------------------------ src/fuzz.h | 1 + src/util.c | 56 +++++++++++++++++++++++++++++++++++++++ src/util.h | 2 ++ test/meson.build | 1 + test/test-util.c | 30 +++++++++++++++++++++ 8 files changed, 132 insertions(+), 81 deletions(-) create mode 100644 test/test-util.c diff --git a/man/dfuzzer.xml b/man/dfuzzer.xml index 3e667df..e9c3e67 100644 --- a/man/dfuzzer.xml +++ b/man/dfuzzer.xml @@ -117,6 +117,13 @@ unsuccessfully, fail message is printed with its return value. + + + + Don't suppress stdout/stderr of a COMMAND + specified via + + diff --git a/src/dfuzzer.c b/src/dfuzzer.c index b0d26bf..953f4a1 100644 --- a/src/dfuzzer.c +++ b/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; diff --git a/src/fuzz.c b/src/fuzz.c index 3ddbbc2..29188dd 100644 --- a/src/fuzz.c +++ b/src/fuzz.c @@ -20,14 +20,10 @@ */ #include #include -#include #include #include #include #include -#include -#include -#include #include #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, diff --git a/src/fuzz.h b/src/fuzz.h index df2f2ee..20a6608 100644 --- a/src/fuzz.h +++ b/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); /** diff --git a/src/util.c b/src/util.c index 682f102..7027895 100644 --- a/src/util.c +++ b/src/util.c @@ -1,11 +1,18 @@ /** @file util.c */ #include +#include #include #include #include +#include #include #include +#include +#include + +#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; +} diff --git a/src/util.h b/src/util.h index 8d19c46..36c5915 100644 --- a/src/util.h +++ b/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); diff --git a/test/meson.build b/test/meson.build index 9a3b05e..19a6528 100644 --- a/test/meson.build +++ b/test/meson.build @@ -1,5 +1,6 @@ tests += [ [files('test-rand.c')], + [files('test-util.c')], ] # vi: sw=8 ts=8 et: diff --git a/test/test-util.c b/test/test-util.c new file mode 100644 index 0000000..fbc895f --- /dev/null +++ b/test/test-util.c @@ -0,0 +1,30 @@ +#include +#include +#include + +#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(); +}