* [PATCH] fuzz: Expect the cmdline in a freeable GString
@ 2020-07-14 17:46 Alexander Bulekov
2020-07-14 17:58 ` Darren Kenny
2020-07-15 13:58 ` Thomas Huth
0 siblings, 2 replies; 3+ messages in thread
From: Alexander Bulekov @ 2020-07-14 17:46 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Alexander Bulekov, f4bug,
darren.kenny, Bandan Das, Stefan Hajnoczi, Paolo Bonzini
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 <alxndr@bu.edu>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fuzz: Expect the cmdline in a freeable GString
2020-07-14 17:46 [PATCH] fuzz: Expect the cmdline in a freeable GString Alexander Bulekov
@ 2020-07-14 17:58 ` Darren Kenny
2020-07-15 13:58 ` Thomas Huth
1 sibling, 0 replies; 3+ messages in thread
From: Darren Kenny @ 2020-07-14 17:58 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: Laurent Vivier, Thomas Huth, f4bug, Alexander Bulekov,
Bandan Das, Stefan Hajnoczi, Paolo Bonzini
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 <alxndr@bu.edu>
Reviewed-by: Darren Kenny <darren.kenny@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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] fuzz: Expect the cmdline in a freeable GString
2020-07-14 17:46 [PATCH] fuzz: Expect the cmdline in a freeable GString Alexander Bulekov
2020-07-14 17:58 ` Darren Kenny
@ 2020-07-15 13:58 ` Thomas Huth
1 sibling, 0 replies; 3+ messages in thread
From: Thomas Huth @ 2020-07-15 13:58 UTC (permalink / raw)
To: Alexander Bulekov, qemu-devel
Cc: Laurent Vivier, f4bug, darren.kenny, Bandan Das, Stefan Hajnoczi,
Paolo Bonzini
On 14/07/2020 19.46, 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 <alxndr@bu.edu>
> ---
> 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(-)
/* 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
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
... and queued to my "qtest-next" branch.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-15 13:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 17:46 [PATCH] fuzz: Expect the cmdline in a freeable GString Alexander Bulekov
2020-07-14 17:58 ` Darren Kenny
2020-07-15 13:58 ` Thomas Huth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).