[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] fuzz: Fix leak when assembling datadir path string
From: |
Thomas Huth |
Subject: |
Re: [PATCH] fuzz: Fix leak when assembling datadir path string |
Date: |
Fri, 17 Jul 2020 18:47:26 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 |
On 17/07/2020 18.35, Alexander Bulekov wrote:
> We freed the string containing the final datadir path, but did not free
> the path to the executable's directory that we get from
> g_path_get_dirname(). Fix that.
>
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>
> I ran it with Thomas' fixed build-oss-fuzz job:
> https://gitlab.com/a1xndr/qemu/-/jobs/644463736
Looks like the fuzzer triggered a crash there, see line 5850 ...
shouldn't the job fail in that case? ... i.e. is the fuzzer still
exiting with return code 0?
> diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
> index 6bc17ef313..031594a686 100644
> --- a/tests/qtest/fuzz/fuzz.c
> +++ b/tests/qtest/fuzz/fuzz.c
> @@ -143,7 +143,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char
> ***envp)
> {
>
> char *target_name;
> - char *dir;
> + char *bindir, *datadir;
> bool serialize = false;
>
> /* Initialize qgraph and modules */
> @@ -164,11 +164,13 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char
> ***envp)
> * location of the executable. Using this we add exec_dir/pc-bios to
> * the datadirs.
> */
> - dir = g_build_filename(g_path_get_dirname(**argv), "pc-bios", NULL);
> - if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> - qemu_add_data_dir(dir);
> + bindir = g_path_get_dirname(**argv);
> + datadir = g_build_filename(bindir, "pc-bios", NULL);
> + g_free(bindir);
> + if (g_file_test(datadir, G_FILE_TEST_IS_DIR)) {
> + qemu_add_data_dir(datadir);
> }
> - g_free(dir);
> + g_free(datadir);
> } else if (*argc > 1) { /* The target is specified as an argument */
> target_name = (*argv)[1];
> if (!strstr(target_name, "--fuzz-target=")) {
>
Patch looks fine, thanks!
Reviewed-by: Thomas Huth <thuth@redhat.com>