[RFC PATCH] scripts/lsan-suppressions: Add a LeakSanitizer suppressions file

[RFC PATCH] scripts/lsan-suppressions: Add a LeakSanitizer suppressions file

[Peter Maydell]

Add a LeakSanitizer suppressions file that documents and suppresses
known false-positive leaks in either QEMU or its dependencies.
To use it you'll need to set
  LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
when running a QEMU built with the leak-sanitizer.

The first and currently only entry is for a deliberate leak in glib's
g_set_user_dirs() that otherwise causes false positive leak reports
in the qga-ssh-test because of its use of G_TEST_OPTION_ISOLATE_DIRS:

Direct leak of 321 byte(s) in 5 object(s) allocated from:
    #0 0x5555dd8abd1e in __interceptor_malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x19cd1e) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)
    #1 0x7fb5bc724738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
    #2 0x7fb5bc739583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
    #3 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1659:21
    #4 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1647:1
    #5 0x7fb5bc757a29 in g_set_user_dirs debian/build/deb/../../../glib/gutils.c:1743:9
    #6 0x7fb5bc743d78 in test_do_isolate_dirs debian/build/deb/../../../glib/gtestutils.c:1486:3
    #7 0x7fb5bc743d78 in test_case_run debian/build/deb/../../../glib/gtestutils.c:2917:16
    #8 0x7fb5bc743d78 in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3018:16
    #9 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    #10 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
    #11 0x7fb5bc743fe9 in g_test_run_suite debian/build/deb/../../../glib/gtestutils.c:3112:13
    #12 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2231:7
    #13 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2218:1
    #14 0x5555dd9293b1 in main qga/commands-posix-ssh.c:439:12
    #15 0x7fb5bc3dfd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #16 0x7fb5bc3dfe3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #17 0x5555dd828ed4 in _start (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x119ed4) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)

(Strictly speaking, this is a genuine leak, it's just a deliberate
one by glib; they document it in their valgrind-format suppression
file upstream.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Does this seem like a good idea?  It gives us a place to document
things like this and to suppress them so we could in theory get a
complete clean 'make check' run with the leak sanitizer on.  It might
be nice if there was an easy way to enable all our "recommended
sanitizer settings" (ASAN_OPTIONS="fast_unwind_on_malloc=0 is
pretty much required to get useful backtraces, for instance), but
I'm not sure there's a neat way to do that.

 scripts/lsan-suppressions.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
 create mode 100644 scripts/lsan-suppressions.txt

diff --git a/scripts/lsan-suppressions.txt b/scripts/lsan-suppressions.txt
new file mode 100644
index 00000000000..5c3cffaa5a0
--- /dev/null
+++ b/scripts/lsan-suppressions.txt
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2024 Linaro Limited
+
+# This is a set of suppressions for LeakSanitizer; you can use it
+# by setting
+#   LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
+# when running a QEMU built with the leak-sanitizer.
+
+# g_set_user_dirs() deliberately leaks the previous cached g_get_user_*
+# values. This is documented in upstream glib's valgrind-format
+# suppression file:
+# https://github.com/GNOME/glib/blob/main/tools/glib.supp
+# This avoids false positive leak reports for the qga-ssh-test.
+leak:g_set_user_dirs
-- 
2.34.1

Re: [RFC PATCH] scripts/lsan-suppressions: Add a LeakSanitizer suppressions file

[Peter Maydell]

On Mon, 19 Aug 2024 at 18:07, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Add a LeakSanitizer suppressions file that documents and suppresses
> known false-positive leaks in either QEMU or its dependencies.
> To use it you'll need to set
>   LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
> when running a QEMU built with the leak-sanitizer.
>
> The first and currently only entry is for a deliberate leak in glib's
> g_set_user_dirs() that otherwise causes false positive leak reports
> in the qga-ssh-test because of its use of G_TEST_OPTION_ISOLATE_DIRS:
>
> Direct leak of 321 byte(s) in 5 object(s) allocated from:
>     #0 0x5555dd8abd1e in __interceptor_malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x19cd1e) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)
>     #1 0x7fb5bc724738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
>     #2 0x7fb5bc739583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
>     #3 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1659:21
>     #4 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1647:1
>     #5 0x7fb5bc757a29 in g_set_user_dirs debian/build/deb/../../../glib/gutils.c:1743:9
>     #6 0x7fb5bc743d78 in test_do_isolate_dirs debian/build/deb/../../../glib/gtestutils.c:1486:3
>     #7 0x7fb5bc743d78 in test_case_run debian/build/deb/../../../glib/gtestutils.c:2917:16
>     #8 0x7fb5bc743d78 in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3018:16
>     #9 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
>     #10 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
>     #11 0x7fb5bc743fe9 in g_test_run_suite debian/build/deb/../../../glib/gtestutils.c:3112:13
>     #12 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2231:7
>     #13 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2218:1
>     #14 0x5555dd9293b1 in main qga/commands-posix-ssh.c:439:12
>     #15 0x7fb5bc3dfd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>     #16 0x7fb5bc3dfe3f in __libc_start_main csu/../csu/libc-start.c:392:3
>     #17 0x5555dd828ed4 in _start (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x119ed4) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)
>
> (Strictly speaking, this is a genuine leak, it's just a deliberate
> one by glib; they document it in their valgrind-format suppression
> file upstream.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Does this seem like a good idea?  It gives us a place to document
> things like this and to suppress them so we could in theory get a
> complete clean 'make check' run with the leak sanitizer on.  It might
> be nice if there was an easy way to enable all our "recommended
> sanitizer settings" (ASAN_OPTIONS="fast_unwind_on_malloc=0 is
> pretty much required to get useful backtraces, for instance), but
> I'm not sure there's a neat way to do that.

On the "no" end of the argument: it looks like from glib 2.79
or thereabouts there was support added to glib to make it
dynamically detect whether it's running in a binary that was
built with LSan and explicitly tell lsan to ignore these
deliberate leaks. That fix is less than a year old, though,
and at least my dev machine is still running 2.72.

https://github.com/GNOME/glib/commit/fb58d55187dfe1565d10c0c0ffdbaa85376cf0b8

-- PMM

Re: [RFC PATCH] scripts/lsan-suppressions: Add a LeakSanitizer suppressions file

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Add a LeakSanitizer suppressions file that documents and suppresses
> known false-positive leaks in either QEMU or its dependencies.
> To use it you'll need to set
>   LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
> when running a QEMU built with the leak-sanitizer.
>
> The first and currently only entry is for a deliberate leak in glib's
> g_set_user_dirs() that otherwise causes false positive leak reports
> in the qga-ssh-test because of its use of G_TEST_OPTION_ISOLATE_DIRS:

Shame we can't share with scripts/oss-fuzz/lsan_supressions.tct:

# The tcmalloc on Fedora37 confuses things
leak:/lib64/libtcmalloc_minimal.so.4

# libxkbcommon also leaks in qemu-keymap
leak:/lib64/libxkbcommon.so.0

Or does fuzzing make some things easier to hit?

>
> Direct leak of 321 byte(s) in 5 object(s) allocated from:
>     #0 0x5555dd8abd1e in __interceptor_malloc (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x19cd1e) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)
>     #1 0x7fb5bc724738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
>     #2 0x7fb5bc739583 in g_strdup debian/build/deb/../../../glib/gstrfuncs.c:361:17
>     #3 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1659:21
>     #4 0x7fb5bc757a29 in set_str_if_different debian/build/deb/../../../glib/gutils.c:1647:1
>     #5 0x7fb5bc757a29 in g_set_user_dirs debian/build/deb/../../../glib/gutils.c:1743:9
>     #6 0x7fb5bc743d78 in test_do_isolate_dirs debian/build/deb/../../../glib/gtestutils.c:1486:3
>     #7 0x7fb5bc743d78 in test_case_run debian/build/deb/../../../glib/gtestutils.c:2917:16
>     #8 0x7fb5bc743d78 in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3018:16
>     #9 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
>     #10 0x7fb5bc74380a in g_test_run_suite_internal debian/build/deb/../../../glib/gtestutils.c:3035:18
>     #11 0x7fb5bc743fe9 in g_test_run_suite debian/build/deb/../../../glib/gtestutils.c:3112:13
>     #12 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2231:7
>     #13 0x7fb5bc744055 in g_test_run debian/build/deb/../../../glib/gtestutils.c:2218:1
>     #14 0x5555dd9293b1 in main qga/commands-posix-ssh.c:439:12
>     #15 0x7fb5bc3dfd8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
>     #16 0x7fb5bc3dfe3f in __libc_start_main csu/../csu/libc-start.c:392:3
>     #17 0x5555dd828ed4 in _start (/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qga/qga-ssh-test+0x119ed4) (BuildId: 7991a166007e8206c51bee401722a8335e7990bb)
>
> (Strictly speaking, this is a genuine leak, it's just a deliberate
> one by glib; they document it in their valgrind-format suppression
> file upstream.)
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Does this seem like a good idea?  It gives us a place to document
> things like this and to suppress them so we could in theory get a
> complete clean 'make check' run with the leak sanitizer on.  It might
> be nice if there was an easy way to enable all our "recommended
> sanitizer settings" (ASAN_OPTIONS="fast_unwind_on_malloc=0 is
> pretty much required to get useful backtraces, for instance), but
> I'm not sure there's a neat way to do that.
>
>  scripts/lsan-suppressions.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 scripts/lsan-suppressions.txt
>
> diff --git a/scripts/lsan-suppressions.txt b/scripts/lsan-suppressions.txt
> new file mode 100644
> index 00000000000..5c3cffaa5a0
> --- /dev/null
> +++ b/scripts/lsan-suppressions.txt
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (c) 2024 Linaro Limited
> +
> +# This is a set of suppressions for LeakSanitizer; you can use it
> +# by setting
> +#   LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
> +# when running a QEMU built with the leak-sanitizer.
> +
> +# g_set_user_dirs() deliberately leaks the previous cached g_get_user_*
> +# values. This is documented in upstream glib's valgrind-format
> +# suppression file:
> +# https://github.com/GNOME/glib/blob/main/tools/glib.supp
> +# This avoids false positive leak reports for the qga-ssh-test.
> +leak:g_set_user_dirs

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [RFC PATCH] scripts/lsan-suppressions: Add a LeakSanitizer suppressions file

[Peter Maydell]

On Mon, 19 Aug 2024 at 23:23, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > Add a LeakSanitizer suppressions file that documents and suppresses
> > known false-positive leaks in either QEMU or its dependencies.
> > To use it you'll need to set
> >   LSAN_OPTIONS="suppressions=/path/to/scripts/lsan-suppressions.txt"
> > when running a QEMU built with the leak-sanitizer.
> >
> > The first and currently only entry is for a deliberate leak in glib's
> > g_set_user_dirs() that otherwise causes false positive leak reports
> > in the qga-ssh-test because of its use of G_TEST_OPTION_ISOLATE_DIRS:
>
> Shame we can't share with scripts/oss-fuzz/lsan_supressions.tct:
>
> # The tcmalloc on Fedora37 confuses things
> leak:/lib64/libtcmalloc_minimal.so.4
>
> # libxkbcommon also leaks in qemu-keymap
> leak:/lib64/libxkbcommon.so.0
>
> Or does fuzzing make some things easier to hit?

Ah, I hadn't realised we already had a suppression file.
I think we've just encountered different things.

Plus the fuzzing disables qga-ssh-test in
qga/meson.build rather than suppressing the leak report.
Maybe we could move that to the leak-suppression file,
or maybe it causes some other problem in the fuzzing
context rather than merely a false positive?

(There are also a ton of reported leaks in the GUI libraries
we link against if you run with the GTK GUI, but I haven't
investigated those to see if they're real or something we
should just suppress to reduce the noise.)

thanks
-- PMM