Re: [PATCH v2] linux-user: fix resource leaks in gen-vdso

["Daniel P. Berrangé" <berrange@redhat.com>]

Ping for review/acks

On Tue, May 13, 2025 at 04:03:46PM +0100, Daniel P. Berrangé wrote:
> There are a number of resource leaks in gen-vdso. In theory they are
> harmless because this is a short lived process, but when building QEMU
> with --extra-cflags="-fsanitize=address" problems ensure. The gen-vdso
> program is run as part of the build, and that aborts due to the
> sanitizer identifying memory leaks, leaving QEMU unbuildable.
> 
> FAILED: libqemu-x86_64-linux-user.a.p/vdso.c.inc
> /var/home/berrange/src/virt/qemu/build/linux-user/gen-vdso -o libqemu-x86_64-linux-user.a.p/vdso.c.inc ../linux-user/x86_64/vdso.so
> 
> =================================================================
> ==1696332==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 2968 byte(s) in 1 object(s) allocated from:
>     #0 0x56495873f1f3  (/var/home/berrange/src/virt/qemu/build/linux-user/gen-vdso+0xa11f3) (BuildId: b69e241ad44719b6f3934f3c71dfc6727e8bdb12)
>     #1 0x564958780b90  (/var/home/berrange/src/virt/qemu/build/linux-user/gen-vdso+0xe2b90) (BuildId: b69e241ad44719b6f3934f3c71dfc6727e8bdb12)
> 
> This complaint is about the 'buf' variable, however, the FILE objects
> are also leaked in some error scenarios, so this fix refactors the
> cleanup paths to fix all leaks. For completeness it also reports an
> error if fclose() fails on 'inf'.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> Changed in v2:
> 
>  - Add missing braces
> 
>  linux-user/gen-vdso.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/linux-user/gen-vdso.c b/linux-user/gen-vdso.c
> index 721f38d5a3..fce9d5cbc3 100644
> --- a/linux-user/gen-vdso.c
> +++ b/linux-user/gen-vdso.c
> @@ -56,13 +56,14 @@ static unsigned rt_sigreturn_addr;
>  
>  int main(int argc, char **argv)
>  {
> -    FILE *inf, *outf;
> +    FILE *inf = NULL, *outf = NULL;
>      long total_len;
>      const char *prefix = "vdso";
>      const char *inf_name;
>      const char *outf_name = NULL;
> -    unsigned char *buf;
> +    unsigned char *buf = NULL;
>      bool need_bswap;
> +    int ret = EXIT_FAILURE;
>  
>      while (1) {
>          int opt = getopt(argc, argv, "o:p:r:s:");
> @@ -129,7 +130,6 @@ int main(int argc, char **argv)
>          fprintf(stderr, "%s: incomplete read\n", inf_name);
>          return EXIT_FAILURE;
>      }
> -    fclose(inf);
>  
>      /*
>       * Identify which elf flavor we're processing.
> @@ -205,19 +205,24 @@ int main(int argc, char **argv)
>      fprintf(outf, "    .rt_sigreturn_ofs = 0x%x,\n", rt_sigreturn_addr);
>      fprintf(outf, "};\n");
>  
> -    /*
> -     * Everything should have gone well.
> -     */
> -    if (fclose(outf)) {
> -        goto perror_outf;
> +    ret = EXIT_SUCCESS;
> +
> + cleanup:
> +    free(buf);
> +
> +    if (outf && fclose(outf) != 0) {
> +        ret = EXIT_FAILURE;
> +    }
> +    if (inf && fclose(inf) != 0) {
> +        ret = EXIT_FAILURE;
>      }
> -    return EXIT_SUCCESS;
> +    return ret;
>  
>   perror_inf:
>      perror(inf_name);
> -    return EXIT_FAILURE;
> +    goto cleanup;
>  
>   perror_outf:
>      perror(outf_name);
> -    return EXIT_FAILURE;
> +    goto cleanup;
>  }
> -- 
> 2.49.0
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|