qemu-devel
[Top][All Lists]
Advanced

[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>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]