On Tuesday, 2020-07-14 at 13:46:16 -04, Alexander Bulekov wrote: > In the initial FuzzTarget, get_init_cmdline returned a char *. With this > API, we had no guarantee about where the string came from. For example, > i440fx-qtest-reboot-fuzz simply returned a pointer to a string literal, > while the QOS-based targets build the arguments out in a GString an > return the gchar *str pointer. Since we did not try to free the cmdline, > we have a leak for any targets that do not simply return string > literals. Clean up this mess by forcing fuzz-targets to return > a GString, that we can free. > > Signed-off-by: Alexander Bulekov <alx...@bu.edu>
Reviewed-by: Darren Kenny <darren.ke...@oracle.com> > --- > tests/qtest/fuzz/fuzz.c | 13 ++++++------- > tests/qtest/fuzz/fuzz.h | 6 +++--- > tests/qtest/fuzz/i440fx_fuzz.c | 4 ++-- > tests/qtest/fuzz/qos_fuzz.c | 6 +++--- > 4 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c > index 0b66e43409..6bc17ef313 100644 > --- a/tests/qtest/fuzz/fuzz.c > +++ b/tests/qtest/fuzz/fuzz.c > @@ -199,16 +199,15 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char > ***envp) > } > > /* Run QEMU's softmmu main with the fuzz-target dependent arguments */ > - const char *init_cmdline = fuzz_target->get_init_cmdline(fuzz_target); > - init_cmdline = g_strdup_printf("%s -qtest /dev/null -qtest-log %s", > - init_cmdline, > - getenv("QTEST_LOG") ? "/dev/fd/2" > - : "/dev/null"); > - > + GString *cmd_line = fuzz_target->get_init_cmdline(fuzz_target); > + g_string_append_printf(cmd_line, > + " -qtest /dev/null -qtest-log %s", > + getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null"); > > /* Split the runcmd into an argv and argc */ > wordexp_t result; > - wordexp(init_cmdline, &result, 0); > + wordexp(cmd_line->str, &result, 0); > + g_string_free(cmd_line, true); > > qemu_init(result.we_wordc, result.we_wordv, NULL); > > diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h > index 72d5710f6c..9ca3d107c5 100644 > --- a/tests/qtest/fuzz/fuzz.h > +++ b/tests/qtest/fuzz/fuzz.h > @@ -50,10 +50,10 @@ typedef struct FuzzTarget { > > > /* > - * returns the arg-list that is passed to qemu/softmmu init() > - * Cannot be NULL > + * Returns the arguments that are passed to qemu/softmmu init(). Freed by > + * the caller. > */ > - const char* (*get_init_cmdline)(struct FuzzTarget *); > + GString *(*get_init_cmdline)(struct FuzzTarget *); > > /* > * will run once, prior to running qemu/softmmu init. > diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c > index e2f31e56f9..bf966d478b 100644 > --- a/tests/qtest/fuzz/i440fx_fuzz.c > +++ b/tests/qtest/fuzz/i440fx_fuzz.c > @@ -158,9 +158,9 @@ static void i440fx_fuzz_qos_fork(QTestState *s, > > static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest" > " -m 0 -display none"; > -static const char *i440fx_argv(FuzzTarget *t) > +static GString *i440fx_argv(FuzzTarget *t) > { > - return i440fx_qtest_argv; > + return g_string_new(i440fx_qtest_argv); > } > > static void fork_init(void) > diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c > index 0c68f5361f..d52f3ebd83 100644 > --- a/tests/qtest/fuzz/qos_fuzz.c > +++ b/tests/qtest/fuzz/qos_fuzz.c > @@ -66,7 +66,7 @@ void *qos_allocate_objects(QTestState *qts, QGuestAllocator > **p_alloc) > return allocate_objects(qts, current_path + 1, p_alloc); > } > > -static const char *qos_build_main_args(void) > +static GString *qos_build_main_args(void) > { > char **path = fuzz_path_vec; > QOSGraphNode *test_node; > @@ -88,7 +88,7 @@ static const char *qos_build_main_args(void) > /* Prepend the arguments that we need */ > g_string_prepend(cmd_line, > TARGET_NAME " -display none -machine accel=qtest -m 64 "); > - return cmd_line->str; > + return cmd_line; > } > > /* > @@ -189,7 +189,7 @@ static void walk_path(QOSGraphNode *orig_path, int len) > g_free(path_str); > } > > -static const char *qos_get_cmdline(FuzzTarget *t) > +static GString *qos_get_cmdline(FuzzTarget *t) > { > /* > * Set a global variable that we use to identify the qos_path for our > -- > 2.26.2