[PATCH libnbd] tests: Use libc_malloc_debug.so.0 instead of MALLOC_CHECK_ for glibc 2.34
This is the nbdkit change transplanted to libnbd. Because there are several subdirectory Makefiles which define the MALLOC_CHECK_ environment variables, I turned it into a macro which is placed in the top level subdir-rules.mk. Note this currently breaks valgrinding because of: https://sourceware.org/bugzilla/show_bug.cgi?id=28256 and because there's not a good place to unset GLIBC_TUNABLES in libnbd. Rich.
glibc 2.34 changes the way that malloc checking works. For more context see these nbdkit commits: https://gitlab.com/nbdkit/nbdkit/-/commit/362e0fdcae37db876e13b944102a5c152e... https://gitlab.com/nbdkit/nbdkit/-/commit/8972831aa2a32d4b5820465d37c1827eb7... Thanks: Siddhesh Poyarekar and Eric Blake --- configure.ac | 12 ++++++++++++ copy/Makefile.am | 3 +-- fuse/Makefile.am | 3 +-- golang/Makefile.am | 3 +-- info/Makefile.am | 3 +-- ocaml/tests/Makefile.am | 3 +-- python/Makefile.am | 3 +-- sh/Makefile.am | 3 +-- subdir-rules.mk | 16 ++++++++++++++++ tests/Makefile.am | 3 +-- 10 files changed, 36 insertions(+), 16 deletions(-) diff --git a/configure.ac b/configure.ac index 734d2730..3e7f367e 100644 --- a/configure.ac +++ b/configure.ac @@ -121,6 +121,18 @@ AC_CHECK_HEADERS([dlfcn.h],[have_dlfcn=yes],[have_dlfcn=no]) AM_CONDITIONAL([HAVE_LIBDL], [test "x$have_libdl" = "xyes" && test "x$have_dlfcn" = "xyes"]) +dnl Does this platform require libc_malloc_debug.so.0 (glibc >= 2.34)? +AC_MSG_CHECKING([if this is glibc >= 2.34]) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[ +#include <limits.h> +#if !defined(__GLIBC__) || __GLIBC__ < 2 || (__GLIBC__ == 2 && __GLIBC_MINOR__ < 34) +#error "not glibc 2.34" +#endif + ]])], [is_glibc_234=yes], [is_glibc_234=no] +) +AC_MSG_RESULT([$is_glibc_234]) +AM_CONDITIONAL([HAVE_GLIBC_234], [test "x$is_glibc_234" = "xyes"]) + dnl Check for GnuTLS (optional, for TLS support). AC_ARG_WITH([gnutls], [AS_HELP_STRING([--without-gnutls], diff --git a/copy/Makefile.am b/copy/Makefile.am index d35abe46..f2100853 100644 --- a/copy/Makefile.am +++ b/copy/Makefile.am @@ -53,8 +53,7 @@ EXTRA_DIST = \ TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ EXPECTED_VERSION=$(VERSION) \ PSKTOOL=$(PSKTOOL) \ QEMU_NBD=$(QEMU_NBD) \ diff --git a/fuse/Makefile.am b/fuse/Makefile.am index d8691862..26ea0b0f 100644 --- a/fuse/Makefile.am +++ b/fuse/Makefile.am @@ -37,8 +37,7 @@ EXTRA_DIST = \ TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ EXPECTED_VERSION=$(VERSION) \ $(NULL) LOG_COMPILER = $(top_builddir)/run diff --git a/golang/Makefile.am b/golang/Makefile.am index a8771190..77c0e0cf 100644 --- a/golang/Makefile.am +++ b/golang/Makefile.am @@ -89,8 +89,7 @@ all-local: $(source_files) TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ pkg=$(pkg) \ abs_top_srcdir=$(abs_top_srcdir) \ $(NULL) diff --git a/info/Makefile.am b/info/Makefile.am index a5708ec5..332ef80d 100644 --- a/info/Makefile.am +++ b/info/Makefile.am @@ -58,8 +58,7 @@ EXTRA_DIST = \ TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ EXPECTED_VERSION=$(VERSION) \ QEMU_NBD=$(QEMU_NBD) \ $(NULL) diff --git a/ocaml/tests/Makefile.am b/ocaml/tests/Makefile.am index e0e42e72..6fac8b7c 100644 --- a/ocaml/tests/Makefile.am +++ b/ocaml/tests/Makefile.am @@ -121,8 +121,7 @@ endif TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ OCAMLRUNPARAM=b \ $(NULL) LOG_COMPILER = $(top_builddir)/run diff --git a/python/Makefile.am b/python/Makefile.am index a05c016a..f51d40e1 100644 --- a/python/Makefile.am +++ b/python/Makefile.am @@ -80,8 +80,7 @@ if HAVE_NBDKIT TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ PYTHON="$(PYTHON)" \ $(NULL) LOG_COMPILER = $(top_builddir)/run diff --git a/sh/Makefile.am b/sh/Makefile.am index 4bf0e1a4..1e732d4d 100644 --- a/sh/Makefile.am +++ b/sh/Makefile.am @@ -50,8 +50,7 @@ endif HAVE_POD TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ NBDKIT_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ EXPECTED_VERSION=$(VERSION) \ $(NULL) LOG_COMPILER = $(top_builddir)/run diff --git a/subdir-rules.mk b/subdir-rules.mk index afb3e022..6d382c6d 100644 --- a/subdir-rules.mk +++ b/subdir-rules.mk @@ -31,3 +31,19 @@ endif $(top_builddir)/podwrapper.pl: $(top_srcdir)/podwrapper.pl.in $(MAKE) -C $(top_builddir) podwrapper.pl + +# In tests, include $(MALLOC_CHECKS) in TESTS_ENVIRONMENT to find some +# use-after-free and uninitialized read problems when using glibc. +# This doesn't affect other libc. +random = $(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') +if HAVE_GLIBC_234 +MALLOC_CHECKS = \ + LD_PRELOAD="$${LD_PRELOAD:+"$$LD_PRELOAD:"}libc_malloc_debug.so.0" \ + GLIBC_TUNABLES=glibc.malloc.check=1:glibc.malloc.perturb=$(random) \ + $(NULL) +else +MALLOC_CHECKS = \ + MALLOC_CHECK_=1 \ + MALLOC_PERTURB_=$(random) \ + $(NULL) +endif diff --git a/tests/Makefile.am b/tests/Makefile.am index aad55130..f2e89081 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -51,8 +51,7 @@ TESTS = # Enable debug in all tests. TESTS_ENVIRONMENT = \ LIBNBD_DEBUG=1 \ - MALLOC_CHECK_=1 \ - MALLOC_PERTURB_=$(shell bash -c 'echo $$(( 1 + (RANDOM & 255) ))') \ + $(MALLOC_CHECKS) \ srcdir=$(srcdir) \ $(NULL) # Use the ./run script so we're always using the local library and tools. -- 2.32.0
On Tue, Aug 24, 2021 at 05:13:24PM +0100, Richard W.M. Jones wrote:
This is the nbdkit change transplanted to libnbd. Because there are several subdirectory Makefiles which define the MALLOC_CHECK_ environment variables, I turned it into a macro which is placed in the top level subdir-rules.mk.
Nice refactoring to avoid duplication.
Note this currently breaks valgrinding because of: https://sourceware.org/bugzilla/show_bug.cgi?id=28256 and because there's not a good place to unset GLIBC_TUNABLES in libnbd.
Patch looks good to apply (I almost tackled the same project myself before seeing your email). Could we modify run.in to define _VG to start with 'env -u GLIBC_TUNABLES valgrind' on systems where 'env -u' is supported? (Sadly, POSIX doesn't require 'env -u') -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On Wed, Aug 25, 2021 at 01:26:08PM -0500, Eric Blake wrote:
On Tue, Aug 24, 2021 at 05:13:24PM +0100, Richard W.M. Jones wrote:
This is the nbdkit change transplanted to libnbd. Because there are several subdirectory Makefiles which define the MALLOC_CHECK_ environment variables, I turned it into a macro which is placed in the top level subdir-rules.mk.
Nice refactoring to avoid duplication.
Note this currently breaks valgrinding because of: https://sourceware.org/bugzilla/show_bug.cgi?id=28256 and because there's not a good place to unset GLIBC_TUNABLES in libnbd.
Patch looks good to apply (I almost tackled the same project myself before seeing your email). Could we modify run.in to define _VG to start with 'env -u GLIBC_TUNABLES valgrind' on systems where 'env -u' is supported? (Sadly, POSIX doesn't require 'env -u')
I actually thought of another way to fix this but forgot to send a follow-up email: diff --git a/valgrind/glibc.suppressions b/valgrind/glibc.suppressions index a03332d9..d0f8ac52 100644 --- a/valgrind/glibc.suppressions +++ b/valgrind/glibc.suppressions @@ -57,3 +57,14 @@ ... fun:__printf_chk } + +# Conditional jump or move depends on uninitialised value(s) in +# __GI___tunables_init +# https://sourceware.org/bugzilla/show_bug.cgi?id=28256 +# Bug present in glibc 2.34 and fixed in 2.35. +{ + glibc_6 + Memcheck:Cond + fun:parse_tunables + fun:__GI___tunables_init +} This obviously only suppresses the error rather than fixing it but I tested it and it allows make check-valgrind to pass partially. There is actually a further error revealed by valgrinding (but disguised by the above problem) which I'm looking into. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
I pushed this which fixes everything: https://gitlab.com/nbdkit/libnbd/-/commit/45e329178c41c3cea61b879b08ba0884a6... However I think we should in the long term find a way to unset GLIBC_TUNABLES when running under valgrind. (I couldn't work out how to use env -u). Anyway that's for another day. Also: https://bugzilla.redhat.com/show_bug.cgi?id=1985048#c4 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Wed, Aug 25, 2021 at 10:05:39PM +0100, Richard W.M. Jones wrote:
I pushed this which fixes everything:
https://gitlab.com/nbdkit/libnbd/-/commit/45e329178c41c3cea61b879b08ba0884a6...
However I think we should in the long term find a way to unset GLIBC_TUNABLES when running under valgrind. (I couldn't work out how to use env -u). Anyway that's for another day.
Actually we can do this in the ./run script since all the tests use that. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
participants (2)
-
Eric Blake -
Richard W.M. Jones