* [PATCH] linux-user: fix resource leaks in gen-vdso
@ 2025-02-19 10:03 Daniel P. Berrangé
2025-03-20 17:38 ` Daniel P. Berrangé
2025-03-20 23:16 ` Richard Henderson
0 siblings, 2 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2025-02-19 10:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Laurent Vivier, Daniel P. Berrangé
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>
---
linux-user/gen-vdso.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
diff --git a/linux-user/gen-vdso.c b/linux-user/gen-vdso.c
index 721f38d5a3..88d94b19eb 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;
- }
- return EXIT_SUCCESS;
+ ret = EXIT_SUCCESS;
+
+ cleanup:
+ free(buf);
+
+ if (outf &&
+ fclose(outf) != 0)
+ ret = EXIT_FAILURE;
+ if (inf &&
+ fclose(inf) != 0)
+ ret = EXIT_FAILURE;
+ return ret;
perror_inf:
perror(inf_name);
- return EXIT_FAILURE;
+ goto cleanup;
perror_outf:
perror(outf_name);
- return EXIT_FAILURE;
+ goto cleanup;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] linux-user: fix resource leaks in gen-vdso
2025-02-19 10:03 [PATCH] linux-user: fix resource leaks in gen-vdso Daniel P. Berrangé
@ 2025-03-20 17:38 ` Daniel P. Berrangé
2025-03-20 23:16 ` Richard Henderson
1 sibling, 0 replies; 3+ messages in thread
From: Daniel P. Berrangé @ 2025-03-20 17:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Laurent Vivier
ping, any thoughts on this patch
On Wed, Feb 19, 2025 at 10:03:36AM +0000, 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>
> ---
> linux-user/gen-vdso.c | 29 +++++++++++++++++------------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/linux-user/gen-vdso.c b/linux-user/gen-vdso.c
> index 721f38d5a3..88d94b19eb 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;
> - }
> - return EXIT_SUCCESS;
> + ret = EXIT_SUCCESS;
> +
> + cleanup:
> + free(buf);
> +
> + if (outf &&
> + fclose(outf) != 0)
> + ret = EXIT_FAILURE;
> + if (inf &&
> + fclose(inf) != 0)
> + ret = EXIT_FAILURE;
> + return ret;
>
> perror_inf:
> perror(inf_name);
> - return EXIT_FAILURE;
> + goto cleanup;
>
> perror_outf:
> perror(outf_name);
> - return EXIT_FAILURE;
> + goto cleanup;
> }
> --
> 2.47.1
>
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 :|
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] linux-user: fix resource leaks in gen-vdso
2025-02-19 10:03 [PATCH] linux-user: fix resource leaks in gen-vdso Daniel P. Berrangé
2025-03-20 17:38 ` Daniel P. Berrangé
@ 2025-03-20 23:16 ` Richard Henderson
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2025-03-20 23:16 UTC (permalink / raw)
To: qemu-devel
On 2/19/25 02:03, Daniel P. Berrangé wrote:
> + cleanup:
> + free(buf);
> +
> + if (outf &&
> + fclose(outf) != 0)
> + ret = EXIT_FAILURE;
> + if (inf &&
> + fclose(inf) != 0)
> + ret = EXIT_FAILURE;
> + return ret;
Modulo the lack of braces here, the patch looks fine.
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-20 23:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-19 10:03 [PATCH] linux-user: fix resource leaks in gen-vdso Daniel P. Berrangé
2025-03-20 17:38 ` Daniel P. Berrangé
2025-03-20 23:16 ` Richard Henderson
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).