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é <berra...@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 :|
