From 2633d6ba9696e1229ff95338075454ec0956d969 Mon Sep 17 00:00:00 2001 From: Petr Malat Date: Wed, 4 Dec 2024 10:30:24 +0100 Subject: [PATCH 01/12] _dbus_loop_iterate: Fix OOM retry timeout handling If there is a pending OOM watch and at the same time there is no timeout, poll is entered with infinite timeout, because infinite is expressed with a negative number, which is smaller than any actual timeout. Introduce min_poll_timeout(), which returns the smaller non-negative number of the two, or the larger negative number if both numbers are negative. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/536 Signed-off-by: Petr Malat [smcv: adjust whitespace] Signed-off-by: Simon McVittie (cherry picked from commit a6023f49acfda099a7ccac9ebd804d553ec9d666) --- dbus/dbus-mainloop.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/dbus/dbus-mainloop.c b/dbus/dbus-mainloop.c index 91030e39..4de5a7ae 100644 --- a/dbus/dbus-mainloop.c +++ b/dbus/dbus-mainloop.c @@ -562,6 +562,20 @@ _dbus_loop_queue_dispatch (DBusLoop *loop, return FALSE; } +/* Returns the smaller non-negative number of the two, or the larger negative + * number if both numbers are negative. Poll interprets negative timeout as + * infinity, which makes it longer than any actual timeout. + */ +static int +min_poll_timeout (int a, + int b) +{ + if (a < b) + return a < 0 ? b : a; + else + return b < 0 ? a : b; +} + /* Returns TRUE if we invoked any timeouts or have ready file * descriptors, which is just used in test code as a debug hack */ @@ -620,10 +634,7 @@ _dbus_loop_iterate (DBusLoop *loop, check_timeout (tv_sec, tv_usec, tcb, &msecs_remaining); - if (timeout < 0) - timeout = msecs_remaining; - else - timeout = MIN (msecs_remaining, timeout); + timeout = min_poll_timeout (msecs_remaining, timeout); #if MAINLOOP_SPEW _dbus_verbose (" timeout added, %d remaining, aggregate timeout %ld\n", @@ -656,7 +667,7 @@ _dbus_loop_iterate (DBusLoop *loop, * wait to re-enable it */ if (loop->oom_watch_pending) - timeout = MIN (timeout, _dbus_get_oom_wait ()); + timeout = min_poll_timeout (timeout, _dbus_get_oom_wait ()); #if MAINLOOP_SPEW _dbus_verbose (" polling on %d descriptors timeout %ld\n", _DBUS_N_ELEMENTS (ready_fds), timeout); From 642a22a0bd2759ad95869e30099fa0856e7c945e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 17:04:39 +0000 Subject: [PATCH 02/12] release-checklist: Fix sequencing `meson dist` requires the version you intend to release to have been committed already, and does not create any generated files in the `${srcdir}` that are intended to be committed to git. Signed-off-by: Simon McVittie (cherry picked from commit b97b083f9a0600c51f702cfd452755e8c0625de2) --- maint/release-checklist.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/maint/release-checklist.md b/maint/release-checklist.md index 73eb83ba..c9247382 100644 --- a/maint/release-checklist.md +++ b/maint/release-checklist.md @@ -25,13 +25,13 @@ To make a release of D-Bus, do the following: - CMake takes the version number from `meson.build` and so should not need updating + - When ready to release, `git commit -a`. This is the version + of the tree that corresponds exactly to the released tarball. + - `meson dist -C ${builddir)` (this is the equivalent of Autotools `make distcheck`) - - if `meson dist` failed, fix it. - - - once dist succeeds, `git commit -a`. This is the version - of the tree that corresponds exactly to the released tarball. + - if `meson dist` failed, fix it, commit, retry until successful - tag the tree with `git tag -s -m 'Released X.Y.Z' dbus-X.Y.Z` where X.Y.Z is the version of the release. If you can't sign From cb7f88f8a98d140f42c1e3a41f84d986e271a3e0 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 17:06:06 +0000 Subject: [PATCH 03/12] release-checklist: Give a better reference for deprecation warnings Commit 4ebb275ab7 disabled deprecation warnings in the Autotools build system, which we no longer have. Future stable-branches will want to disable deprecation warnings in Meson instead. Signed-off-by: Simon McVittie (cherry picked from commit f651834427a603e6ac81758e5a300dca3cbe077a) --- maint/release-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maint/release-checklist.md b/maint/release-checklist.md index c9247382..101be53c 100644 --- a/maint/release-checklist.md +++ b/maint/release-checklist.md @@ -70,7 +70,7 @@ changes on a stable branch should be limited to significant bug fixes. Because we won't make minor changes like keeping up with the latest deprecations on a stable branch, stable branches should turn off the -gcc warning for deprecated declarations (e.g. see commit 4ebb275ab7). +gcc warning for deprecated declarations (e.g. see commit 76a68867). Be extra-careful not to merge master (or any branch based on master) into a stable branch. From 80eac5c99efbcf6340fb09836b3ccbb380776cf6 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 17:06:29 +0000 Subject: [PATCH 04/12] release-checklist: Use a more copy-paste'able scp/rsync destination Signed-off-by: Simon McVittie (cherry picked from commit 7dc12d17c8c6908205e2f220e579c88c0068d56e) --- maint/release-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/maint/release-checklist.md b/maint/release-checklist.md index 101be53c..fe0c6183 100644 --- a/maint/release-checklist.md +++ b/maint/release-checklist.md @@ -49,7 +49,7 @@ To make a release of D-Bus, do the following: `git push origin master dbus-X.Y dbus-X.Y.Z` - scp your tarball to freedesktop.org server and copy it to - `dbus.freedesktop.org:/srv/dbus.freedesktop.org/www/releases/dbus/dbus-X.Y.Z.tar.xz`. + `dbus.freedesktop.org:/srv/dbus.freedesktop.org/www/releases/dbus/`. This should be possible if you're in group "dbus" - Update the online documentation with From 9907deedb1cef5ca5f17ff514f8f587b6be9069c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 18:25:27 +0000 Subject: [PATCH 05/12] memory: Remove redundant guard around _dbus_decrement_fail_alloc_counter() This function is already inside `#ifdef DBUS_ENABLE_EMBEDDED_TESTS` and doesn't need a second layer of the same guard. Signed-off-by: Simon McVittie (cherry picked from commit 6e61173d6462b2793c213aa13d4392f232d1c6ec) --- dbus/dbus-memory.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c index be7e4ef6..cf3dd881 100644 --- a/dbus/dbus-memory.c +++ b/dbus/dbus-memory.c @@ -244,7 +244,6 @@ _dbus_get_fail_alloc_failures (void) return n_failures_per_failure; } -#ifdef DBUS_ENABLE_EMBEDDED_TESTS /** * Called when about to alloc some memory; if * it returns #TRUE, then the allocation should @@ -286,7 +285,6 @@ _dbus_decrement_fail_alloc_counter (void) return FALSE; } } -#endif /* DBUS_ENABLE_EMBEDDED_TESTS */ /** * Get the number of outstanding malloc()'d blocks. From d440173062dd866e5509d218fbe0b79bd39f4979 Mon Sep 17 00:00:00 2001 From: Ralf Habacker Date: Mon, 1 Nov 2021 13:01:59 +0100 Subject: [PATCH 06/12] cmake: In client configuration file get DBus1_xxx variables from cmake target The DBus1_xxx variables defined in DBusConfig.cmake for Windows builds are currently hard-coded values and independent of those of the underlying cmake target. To avoid this, these values are retrieved from the corresponding cmake target. In addition, the cmake allows the construction of the resulting relocatable runtime paths. (cherry picked from commit 29c2e9141a0c0dd850c89b5196fba9fa8eb66d89) --- cmake/DBus1Config.cmake.in | 5 +++-- dbus/CMakeLists.txt | 2 ++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmake/DBus1Config.cmake.in b/cmake/DBus1Config.cmake.in index 1775b939..e8d9bc34 100644 --- a/cmake/DBus1Config.cmake.in +++ b/cmake/DBus1Config.cmake.in @@ -26,9 +26,10 @@ if(EXISTS "${CMAKE_CURRENT_LIST_DIR}/DBus1Targets.cmake") include("${CMAKE_CURRENT_LIST_DIR}/DBus1Targets.cmake") endif() - set(DBus1_DEFINITIONS) + # for compatibility, get settings from cmake target + get_target_property(DBus1_DEFINITIONS dbus-1 INTERFACE_COMPILE_DEFINITIONS) get_target_property(DBus1_INCLUDE_DIRS dbus-1 INTERFACE_INCLUDE_DIRECTORIES) - set(DBus1_LIBRARY dbus-1) + get_target_property(DBus1_LIBRARY dbus-1 IMPORTED_IMPLIB) else() message(FATAL_ERROR "Incomplete cmake support in DBus1 find_package configuration") endif() diff --git a/dbus/CMakeLists.txt b/dbus/CMakeLists.txt index 1f545ba6..aaa5a52d 100644 --- a/dbus/CMakeLists.txt +++ b/dbus/CMakeLists.txt @@ -299,7 +299,9 @@ else(WIN32) endif() endif() +# target definitions passed to the clients target_include_directories(dbus-1 INTERFACE $;$) +target_compile_definitions(dbus-1 INTERFACE "") # Assume that Linux has -Wl,--version-script and other platforms do not if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") From 53bcfe2de12ec8536a457f83104f11269d7c0e7c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 17:45:32 +0000 Subject: [PATCH 07/12] Rename "embedded tests" to "intrusive tests" This hopefully helps to get across the point that enabling these tests adds instrumentation to libdbus and dbus-daemon, with a potentially significant impact on code size, performance and security. To avoid a huge diffstat which would be difficult to review, the cpp macro that is checked by most of the C code is still DBUS_ENABLE_EMBEDDED_TESTS, which is defined or undefined under exactly the same conditions as the new DBUS_ENABLE_INTRUSIVE_TESTS. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/537 Co-authored-by: Philip Withnall Signed-off-by: Simon McVittie (cherry picked from commit 9c5b5838f5ff667225a913f97006816e3e401d55) --- CMakeLists.txt | 2 +- cmake/config.h.cmake | 7 ++++++- dbus/CMakeLists.txt | 2 +- dbus/dbus-macros-internal.h | 7 ++++--- dbus/meson.build | 2 +- meson.build | 16 +++++++++------- meson_options.txt | 4 ++-- test/CMakeLists.txt | 4 ++-- test/meson.build | 8 +++++--- test/name-test/meson.build | 2 +- test/use-as-subproject/meson.build | 2 +- tools/ci-build.sh | 2 +- 12 files changed, 34 insertions(+), 24 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d3ec71be..3f9f37f1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -443,7 +443,7 @@ enable_testing() ########### command line options ############### if(DBUS_BUILD_TESTS) - set(DBUS_ENABLE_EMBEDDED_TESTS ON) + set(DBUS_ENABLE_INTRUSIVE_TESTS ON) set(DBUS_ENABLE_MODULAR_TESTS ON) endif() diff --git a/cmake/config.h.cmake b/cmake/config.h.cmake index 1cf57286..17e0def0 100644 --- a/cmake/config.h.cmake +++ b/cmake/config.h.cmake @@ -59,10 +59,15 @@ #ifndef DBUS_DISABLE_CHECKS # define DBUS_ENABLE_CHECKS 1 #endif -#cmakedefine DBUS_ENABLE_EMBEDDED_TESTS 1 +#cmakedefine DBUS_ENABLE_INTRUSIVE_TESTS 1 #cmakedefine DBUS_ENABLE_MODULAR_TESTS 1 #cmakedefine DBUS_USE_OUTPUT_DEBUG_STRING 1 +/* Compatibility with the old name for this functionality */ +#ifdef DBUS_ENABLE_INTRUSIVE_TESTS +# define DBUS_ENABLE_EMBEDDED_TESTS 1 +#endif + /* xmldocs */ /* doxygen */ #cmakedefine DBUS_GCOV_ENABLED 1 diff --git a/dbus/CMakeLists.txt b/dbus/CMakeLists.txt index aaa5a52d..04f9ca22 100644 --- a/dbus/CMakeLists.txt +++ b/dbus/CMakeLists.txt @@ -143,7 +143,7 @@ set(DBUS_SHARED_HEADERS dbus-sysdeps.h ) -if(DBUS_ENABLE_EMBEDDED_TESTS) +if(DBUS_ENABLE_INTRUSIVE_TESTS) set(DBUS_SHARED_SOURCES ${DBUS_SHARED_SOURCES} dbus-test-tap.c) set(DBUS_SHARED_HEADERS ${DBUS_SHARED_HEADERS} dbus-test-tap.h) # ... else they are in the test library instead diff --git a/dbus/dbus-macros-internal.h b/dbus/dbus-macros-internal.h index 3d7c0683..474a84b5 100644 --- a/dbus/dbus-macros-internal.h +++ b/dbus/dbus-macros-internal.h @@ -29,11 +29,12 @@ #include -#ifdef DBUS_ENABLE_EMBEDDED_TESTS -# define DBUS_EMBEDDED_TESTS_EXPORT DBUS_PRIVATE_EXPORT +#ifdef DBUS_ENABLE_INTRUSIVE_TESTS +# define DBUS_INTRUSIVE_TESTS_EXPORT DBUS_PRIVATE_EXPORT #else -# define DBUS_EMBEDDED_TESTS_EXPORT /* nothing */ +# define DBUS_INTRUSIVE_TESTS_EXPORT /* nothing */ #endif +#define DBUS_EMBEDDED_TESTS_EXPORT DBUS_INTRUSIVE_TESTS_EXPORT #if defined(DBUS_PRIVATE_EXPORT) /* value forced by compiler command line, don't redefine */ diff --git a/dbus/meson.build b/dbus/meson.build index 905255c2..87f18d91 100644 --- a/dbus/meson.build +++ b/dbus/meson.build @@ -79,7 +79,7 @@ dbus_shared_sources = [ 'dbus-sysdeps.c', ] -if embedded_tests +if intrusive_tests dbus_shared_sources += 'dbus-test-tap.c' endif diff --git a/meson.build b/meson.build index 1a1e8088..fed1b7f2 100644 --- a/meson.build +++ b/meson.build @@ -879,10 +879,12 @@ config.set('DBUS_USE_OUTPUT_DEBUG_STRING', windows_output_debug) # Controls whether the tools are built. tools = get_option('tools') -# DBUS_ENABLE_EMBEDDED_TESTS controls unit tests built in to .c files +# DBUS_ENABLE_INTRUSIVE_TESTS controls unit tests built in to .c files # and some stuff in the test/ subdir. -embedded_tests = get_option('embedded_tests') -config.set('DBUS_ENABLE_EMBEDDED_TESTS', embedded_tests) +intrusive_tests = get_option('intrusive_tests') +config.set('DBUS_ENABLE_INTRUSIVE_TESTS', intrusive_tests) +# An older name for the same thing +config.set('DBUS_ENABLE_EMBEDDED_TESTS', intrusive_tests) # DBUS_ENABLE_MODULAR_TESTS controls tests that work based on public API. @@ -1344,7 +1346,7 @@ summary_dict += { 'gcc coverage': get_option('b_coverage'), 'gcc profiling': get_option('b_pgo'), - 'Building embedded tests': embedded_tests, + 'Building intrusive tests': intrusive_tests, 'Building modular tests': dbus_enable_modular_tests, '- with GLib': use_glib, 'Installing tests': get_option('installed_tests'), @@ -1385,10 +1387,10 @@ endif summary(summary_dict, bool_yn: true) -if embedded_tests - warning('building with unit tests increases the size of the installed library and renders it insecure.') +if intrusive_tests + warning('building with intrusive tests increases the size of the installed library and renders it insecure.') if not asserts - warning('building with embedded tests but without assertions means tests may not properly report failures (this configuration is only useful when doing something like profiling the tests)') + warning('building with intrusive tests but without assertions means tests may not properly report failures (this configuration is only useful when doing something like profiling the tests)') endif endif diff --git a/meson_options.txt b/meson_options.txt index e63f4bd6..1fc8a2ad 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -82,10 +82,10 @@ option( ) option( - 'embedded_tests', + 'intrusive_tests', type: 'boolean', value: false, - description: 'Enable unit test code in the library and binaries' + description: 'Enable tests that require insecure extra code in the library and binaries' ) option( diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index b3c593e5..032ba1f7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -13,7 +13,7 @@ add_library(dbus-testutils STATIC ) target_link_libraries(dbus-testutils ${DBUS_INTERNAL_LIBRARIES}) -if(DBUS_ENABLE_EMBEDDED_TESTS) +if(DBUS_ENABLE_INTRUSIVE_TESTS) add_subdirectory( name-test ) endif() @@ -100,7 +100,7 @@ if(WIN32) add_helper_executable(manual-paths ${manual-paths_SOURCES} dbus-testutils) endif() -if(DBUS_ENABLE_EMBEDDED_TESTS) +if(DBUS_ENABLE_INTRUSIVE_TESTS) add_test_executable(test-atomic ${test-atomic_SOURCES} dbus-testutils) add_test_executable(test-hash internals/hash.c dbus-testutils) set_target_properties(test-hash PROPERTIES COMPILE_FLAGS ${DBUS_INTERNAL_CLIENT_DEFINITIONS}) diff --git a/test/meson.build b/test/meson.build index 1e61679a..e99196b8 100644 --- a/test/meson.build +++ b/test/meson.build @@ -254,7 +254,9 @@ endif tests = [] -if embedded_tests +if intrusive_tests + # These tests require special instrumentation in libdbus and/or + # dbus-daemon, which is not safe to enable in production builds. tests += [ { @@ -682,10 +684,10 @@ if message_bus and tools and platform_unix and use_glib ] # Testing dbus-launch relies on special code in that binary. - if embedded_tests + if intrusive_tests scripts += { 'name': 'test-dbus-launch-eval.sh' } endif - if embedded_tests and use_x11_autolaunch + if intrusive_tests and use_x11_autolaunch scripts += { 'name': 'test-dbus-launch-x11.sh' } endif endif diff --git a/test/name-test/meson.build b/test/name-test/meson.build index 293a9d36..a99c0e9f 100644 --- a/test/name-test/meson.build +++ b/test/name-test/meson.build @@ -20,7 +20,7 @@ # SOFTWARE. -if embedded_tests +if intrusive_tests tests = [ {'name': 'test-ids'}, diff --git a/test/use-as-subproject/meson.build b/test/use-as-subproject/meson.build index 8111e9ee..e2a67d4a 100644 --- a/test/use-as-subproject/meson.build +++ b/test/use-as-subproject/meson.build @@ -22,7 +22,7 @@ libdbus_dep = dependency( fallback: ['dbus', 'libdbus_dep'], default_options: [ 'default_library=static', - 'embedded_tests=false', + 'intrusive_tests=false', 'message_bus=false', 'modular_tests=disabled', 'tools=false', diff --git a/tools/ci-build.sh b/tools/ci-build.sh index c971415c..55a3c1a4 100755 --- a/tools/ci-build.sh +++ b/tools/ci-build.sh @@ -354,7 +354,7 @@ case "$ci_buildsys" in case "$ci_variant" in (debug) set -- -Dasserts=true "$@" - set -- -Dembedded_tests=true "$@" + set -- -Dintrusive_tests=true "$@" set -- -Dverbose_mode=true "$@" case "$ci_host" in From dcdc638ea4832471ce833d2ac222b380c75e7bed Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 18:16:46 +0000 Subject: [PATCH 08/12] cmake: Make intrusive (formerly embedded) tests into a separate option Previously, the CMake build enabled tests by default, and enabled both modular and intrusive (embedded) tests with a single option. This is a really bad idea if anyone is using CMake-built binaries in production. DBUS_BUILD_TESTS now enables only the modular tests, which are safe to enable in production builds. A new DBUS_ENABLE_INTRUSIVE_TESTS option enables the intrusive test instrumentation. To preserve existing test coverage, explicitly enable the intrusive tests in most CMake-based Gitlab-CI jobs (Debian native, openSUSE native, Windows). In jobs that have a mirrored pair of production/debug builds (openSUSE and Debian mingw32/mingw64 cmake), instead we leave the production build as-is and only build full test coverage in the debug build. Co-authored-by: Philip Withnall Signed-off-by: Simon McVittie (cherry picked from commit 41c7570e1ea803e9635d9bcabba5fc221c94e7e6) --- .gitlab-ci.yml | 11 +++++++---- CMakeLists.txt | 13 +++++++------ README.cmake | 3 +++ cmake/modules/Macros.cmake | 2 +- tools/ci-build.sh | 8 ++++++++ 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a9928968..45b8407b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -167,13 +167,14 @@ debian image: - .unix-host-build stage: build -debian cmake: +debian cmake debug: extends: - .cmake-common - .debian-build when: manual variables: ci_buildsys: "cmake" + ci_variant: "debug" debian meson: extends: @@ -287,12 +288,13 @@ opensuse image: - .unix-host-build stage: build -opensuse cmake: +opensuse cmake debug: extends: - .cmake-common - .suse-build variables: ci_local_packages: "no" + ci_variant: "debug" # TODO: https://gitlab.freedesktop.org/dbus/dbus/-/issues/520 opensuse mingw32 cmake: @@ -360,14 +362,14 @@ windows msys64 ucrt64 cmake: - $env:MSYS2_PATH_TYPE = "inherit" - $env:PATH += ";C:\msys64\usr\bin" # FIXME: glib from msys2 has issues, disable it for now - - C:\msys64\usr\bin\bash -lc 'cmake -G \"MinGW Makefiles\" -S . -B build -DDBUS_WITH_GLIB=OFF && cmake --build build --config Release' + - C:\msys64\usr\bin\bash -lc 'cmake -G \"MinGW Makefiles\" -S . -B build -DDBUS_WITH_GLIB=OFF -DDBUS_ENABLE_INTRUSIVE_TESTS=ON && cmake --build build --config Release' windows vs15-64 cmake: extends: - .cmake-common - .win-build script: - - cmake -DCMAKE_PREFIX_PATH=C:/ -G "Visual Studio 15 2017 Win64" -DCMAKE_BUILD_TYPE=Debug -DDBUS_ENABLE_VERBOSE_MODE=OFF -S . -B build + - cmake -DCMAKE_PREFIX_PATH=C:/ -G "Visual Studio 15 2017 Win64" -DCMAKE_BUILD_TYPE=Debug -DDBUS_ENABLE_VERBOSE_MODE=OFF -DDBUS_ENABLE_INTRUSIVE_TESTS=ON -S . -B build - cmake --build build --config Debug - cmake --install build --config Debug # FIXME: a few tests timeout on gitlab runner for unknown reason @@ -469,6 +471,7 @@ freebsd cmake debug: - .cmake-common - .build-env-freebsd variables: + ci_variant: "debug" # Don't build doxygen documentation since installing the required tools # massively increases the VM image (and therefore container) size. CI_BUILD_ARGS: "-DDBUS_ENABLE_DOXYGEN_DOCS=OFF -DDBUS_ENABLE_XML_DOCS=ON -DCMAKE_BUILD_TYPE=Debug" diff --git a/CMakeLists.txt b/CMakeLists.txt index 3f9f37f1..c2f28800 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,6 +25,7 @@ project(dbus ) option(DBUS_BUILD_TESTS "enable unit test code" ON) +option(DBUS_ENABLE_INTRUSIVE_TESTS "enable tests that require insecure extra code in the library and binaries" OFF) # replacement for AC_C_BIGENDIAN include (TestBigEndian) @@ -443,7 +444,6 @@ enable_testing() ########### command line options ############### if(DBUS_BUILD_TESTS) - set(DBUS_ENABLE_INTRUSIVE_TESTS ON) set(DBUS_ENABLE_MODULAR_TESTS ON) endif() @@ -639,7 +639,7 @@ add_definitions(-DHAVE_CONFIG_H) add_definitions(${DBUS_BUS_CFLAGS}) -if(DBUS_BUILD_TESTS) +if(DBUS_ENABLE_MODULAR_TESTS OR DBUS_ENABLE_INTRUSIVE_TESTS) # set variables used for the .in files (substituted by configure_file) in test/data: set(DBUS_TEST_EXEC ${Z_DRIVE_IF_WINE}${CMAKE_RUNTIME_OUTPUT_DIRECTORY}${IDE_BIN}) # Working directory for build-time tests, so that they'll pick up @@ -700,7 +700,7 @@ endif() add_subdirectory( dbus ) add_subdirectory( bus ) -if(DBUS_BUILD_TESTS) +if(DBUS_ENABLE_MODULAR_TESTS OR DBUS_ENABLE_INTRUSIVE_TESTS) add_subdirectory( test ) add_custom_target(check COMMAND ctest -R ^test-.* @@ -752,6 +752,7 @@ if(MSVC) message(" MSVC code analyze mode: ${DBUS_MSVC_ANALYZE} ") endif() message(" Building unit tests: ${DBUS_BUILD_TESTS} ") +message(" Building intrusive tests: ${DBUS_ENABLE_INTRUSIVE_TESTS} ") message(" Building with GLib: ${DBUS_WITH_GLIB} ") message(" Building verbose mode: ${DBUS_ENABLE_VERBOSE_MODE} ") message(" Building w/o assertions: ${DBUS_DISABLE_ASSERT} ") @@ -786,11 +787,11 @@ message(" build timestamp: ${DBUS_BUILD_TIMESTAMP} " endif() message(" ") -if(DBUS_BUILD_TESTS) - message("NOTE: building with unit tests increases the size of the installed library and renders it insecure.") +if(DBUS_ENABLE_INTRUSIVE_TESTS) + message("NOTE: building with intrusive test code increases the size of the installed library and renders it insecure.") endif() -if(DBUS_BUILD_TESTS AND DBUS_DISABLE_ASSERT) +if(DBUS_ENABLE_INTRUSIVE_TESTS AND DBUS_DISABLE_ASSERT) message("NOTE: building with unit tests but without assertions means tests may not properly report failures (this configuration is only useful when doing something like profiling the tests)") endif() diff --git a/README.cmake b/README.cmake index 03e5f27b..81cac84b 100644 --- a/README.cmake +++ b/README.cmake @@ -124,6 +124,9 @@ CMAKE_INSTALL_PREFIX:PATH=C:/Program Files/dbus // enable unit test code DBUS_BUILD_TESTS:BOOL=ON +// embed intrusive test code in the library and binaries +DBUS_ENABLE_INTRUSIVE_TESTS:BOOL=ON + // The name of the dbus daemon executable DBUS_DAEMON_NAME:STRING=dbus-daemon diff --git a/cmake/modules/Macros.cmake b/cmake/modules/Macros.cmake index 044e2a70..d952b5f3 100644 --- a/cmake/modules/Macros.cmake +++ b/cmake/modules/Macros.cmake @@ -1,6 +1,6 @@ option(DBUS_USE_WINE "set to 1 or ON to support running test cases with Wine" OFF) -if(DBUS_BUILD_TESTS AND CMAKE_CROSSCOMPILING AND CMAKE_SYSTEM_NAME STREQUAL "Windows") +if((DBUS_ENABLE_MODULAR_TESTS OR DBUS_ENABLE_INTRUSIVE_TESTS) AND CMAKE_CROSSCOMPILING AND CMAKE_SYSTEM_NAME STREQUAL "Windows") if(CMAKE_HOST_SYSTEM_NAME STREQUAL "Linux") find_file(WINE_EXECUTABLE NAMES wine diff --git a/tools/ci-build.sh b/tools/ci-build.sh index 55a3c1a4..8752904d 100755 --- a/tools/ci-build.sh +++ b/tools/ci-build.sh @@ -269,6 +269,14 @@ case "$ci_buildsys" in ;; esac + set -- "$@" -D DBUS_BUILD_TESTS=ON + + case "$ci_variant" in + (debug) + set -- "$@" -D DBUS_ENABLE_INTRUSIVE_TESTS=ON + ;; + esac + $cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DENABLE_WERROR=ON -S "$srcdir" -B "$ci_builddir" "$@" ${make} From fd25d0da1ab703890b0351d0527e271a69781651 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 17:12:18 +0000 Subject: [PATCH 09/12] internals: Use negative numbers to indicate no malloc failure simulation If we set the countdown to simulating a failed allocation to _DBUS_INT_MAX, then it will decrement every time we allocate memory, eventually reaching 0 and triggering a simulated malloc failure. In practice this does not happen during unit testing, because all of our tests are (intentionally!) short enough that this can't happen, but it can happen if a build of dbus with embedded tests enabled is used for the "real" dbus-daemon or a "real" D-Bus service, either during debugging or unintentionally, as noted on dbus/dbus!493. We cannot simply special-case `_DBUS_INT_MAX` to never be decremented, because _dbus_test_oom_handling() relies on the counter being decremented even while we are not simulating malloc failure, as a way to count the number of allocations as an upper bound for how long to set the countdown during subsequent test runs. Instead, reserve all negative numbers to represent the absence of malloc failure simulation, while still being able to count allocations by comparing two different negative numbers. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/535 Signed-off-by: Simon McVittie (cherry picked from commit 0735c401a7b3c9e56096fcde8139e773067302bd) --- dbus/dbus-internals.c | 10 +++++++--- dbus/dbus-internals.h | 2 +- dbus/dbus-memory.c | 36 +++++++++++++++++++++++++++--------- dbus/dbus-mempool.c | 2 +- 4 files changed, 36 insertions(+), 14 deletions(-) diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index 6a23ff5b..dd581372 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -1097,7 +1097,7 @@ run_failing_each_malloc (int n_mallocs, n_mallocs -= 1; } - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + _dbus_set_fail_alloc_counter (-1); return TRUE; } @@ -1127,14 +1127,18 @@ _dbus_test_oom_handling (const char *description, /* Run once to see about how many mallocs are involved */ - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + _dbus_set_fail_alloc_counter (-1); _dbus_test_diag ("Running \"%s\" once to count mallocs", description); if (!(* func) (data, TRUE)) return FALSE; - approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter (); + /* We have decremented the counter once per allocation, so for example + * if there were 10 allocations, it will have changed from -1 to -11. + * Subtract from -1 to get the positive number of allocations that took + * place. */ + approx_mallocs = -1 - _dbus_get_fail_alloc_counter (); _dbus_test_diag ("\"%s\" has about %d mallocs in total", description, approx_mallocs); diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index 4aef24ca..0035a0c2 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -384,7 +384,7 @@ dbus_bool_t _dbus_test_oom_handling (const char *description, void *data); #else #define _dbus_set_fail_alloc_counter(n) -#define _dbus_get_fail_alloc_counter _DBUS_INT_MAX +#define _dbus_get_fail_alloc_counter (-1) /* These are constant expressions so that blocks * they protect should be optimized away diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c index cf3dd881..7a11b385 100644 --- a/dbus/dbus-memory.c +++ b/dbus/dbus-memory.c @@ -105,7 +105,7 @@ static dbus_bool_t debug_initialized = FALSE; static int fail_nth = -1; static size_t fail_size = 0; -static int fail_alloc_counter = _DBUS_INT_MAX; +static int fail_alloc_counter = -1; static int n_failures_per_failure = 1; static int n_failures_this_failure = 0; static dbus_bool_t guards = FALSE; @@ -190,7 +190,11 @@ _dbus_disable_mem_pools (void) * Sets the number of allocations until we simulate a failed * allocation. If set to 0, the next allocation to run * fails; if set to 1, one succeeds then the next fails; etc. - * Set to _DBUS_INT_MAX to not fail anything. + * + * Set to a negative number to not fail anything. + * In this case, the counter is still decremented every time we allocate + * memory (until it reaches _DBUS_INT_MIN), which can be used to estimate + * the number of allocations in a particular unit test case. * * @param until_next_fail number of successful allocs before one fails */ @@ -208,7 +212,11 @@ _dbus_set_fail_alloc_counter (int until_next_fail) /** * Gets the number of successful allocs until we'll simulate - * a failed alloc. + * a failed alloc, or negative if we will not simulate a failed alloc. + * + * If negative, the counter indicates the number of allocations since + * it was set to a known value, which can be used to count allocations: + * see _dbus_set_fail_alloc_counter() for details. * * @returns current counter value */ @@ -257,8 +265,20 @@ _dbus_decrement_fail_alloc_counter (void) { _dbus_initialize_malloc_debug (); - if (fail_alloc_counter <= 0) + if (fail_alloc_counter < 0) { + /* We never fail in this case, but we still decrement the counter, + * so that _dbus_test_oom_handling() can use it to count allocations. + * Saturate at _DBUS_INT_MIN to avoid undefined integer overflow + * (or in this case underflow). */ + if (fail_alloc_counter > _DBUS_INT_MIN) + fail_alloc_counter -= 1; + + return FALSE; + } + else if (fail_alloc_counter == 0) + { + /* It's time to pretend we ran out of memory. */ if (backtrace_on_fail_alloc) _dbus_print_backtrace (); @@ -267,11 +287,7 @@ _dbus_decrement_fail_alloc_counter (void) n_failures_this_failure += 1; if (n_failures_this_failure >= n_failures_per_failure) { - if (fail_nth >= 0) - fail_alloc_counter = fail_nth; - else - fail_alloc_counter = _DBUS_INT_MAX; - + fail_alloc_counter = fail_nth; n_failures_this_failure = 0; _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter); @@ -281,6 +297,8 @@ _dbus_decrement_fail_alloc_counter (void) } else { + /* Don't fail this allocation, but count down to the next time we + * will fail an allocation. */ fail_alloc_counter -= 1; return FALSE; } diff --git a/dbus/dbus-mempool.c b/dbus/dbus-mempool.c index 5bc4a97b..c817c049 100644 --- a/dbus/dbus-mempool.c +++ b/dbus/dbus-mempool.c @@ -317,7 +317,7 @@ _dbus_mem_pool_alloc (DBusMemPool *pool) * malloc here for purposes of failed alloc simulation. */ saved_counter = _dbus_get_fail_alloc_counter (); - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + _dbus_set_fail_alloc_counter (-1); #endif if (pool->zero_elements) From 08d854af6958ccdcc8b26fbd0cbd0f047842bf97 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 9 Dec 2024 23:22:17 +0000 Subject: [PATCH 10/12] sysdeps: Don't check for PROC_SUPER_MAGIC if it isn't defined Debian GNU/Hurd has fstatfs() but not PROC_SUPER_MAGIC. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/539 Bug-Debian: https://bugs.debian.org/1089641 Signed-off-by: Simon McVittie (cherry picked from commit 5d7b87496f3bb094b926692036ae656c31efdd8e) --- dbus/dbus-file-unix.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-file-unix.c b/dbus/dbus-file-unix.c index d5fa029e..19228a95 100644 --- a/dbus/dbus-file-unix.c +++ b/dbus/dbus-file-unix.c @@ -66,7 +66,7 @@ _dbus_file_get_contents (DBusString *str, { int fd; struct stat sb; -#ifdef HAVE_FSTATFS +#if defined(HAVE_FSTATFS) && defined(PROC_SUPER_MAGIC) struct statfs sfs; #endif int orig_len; @@ -119,7 +119,7 @@ _dbus_file_get_contents (DBusString *str, /* procfs has different semantics - most files are 0 size, * we can do only one read, and at most we can read 4M. */ -#ifdef HAVE_FSTATFS +#if defined(HAVE_FSTATFS) && defined(PROC_SUPER_MAGIC) if (sb.st_size == 0) { if (fstatfs(fd, &sfs) < 0) From 7c65efde7a1b1720c23e5c884a5dfba2a18b2978 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 10 Dec 2024 14:31:59 +0000 Subject: [PATCH 11/12] tests: Exercise NSS group lookup before running tests Similar to #256, NSS plugins might open file descriptors the first time they look up a system group, and leave them open. To avoid detecting this as a leak, do one group lookup (which we expect to fail) before starting testing, so that the fd is already open the first time we call _dbus_check_fdleaks_enter(), and therefore is not considered to have been leaked in _dbus_check_fdleaks_leave(). Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/540 Signed-off-by: Simon McVittie (cherry picked from commit 7cbb7b75dd2803fcb0c6edb18d5c43e4eaeee704) --- test/test-utils.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/test-utils.c b/test/test-utils.c index 17febc8b..05df7b95 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -788,14 +788,20 @@ _dbus_test_main (int argc, * show up as having been "leaked" by the first module of code under * test that happens to do a NSS lookup. */ { - DBusString username; + DBusString lookup; dbus_uid_t ignored_uid = DBUS_UID_UNSET; + dbus_gid_t ignored_gid = DBUS_GID_UNSET; - _dbus_string_init_const (&username, "dbus-no-user-with-this-name"); + _dbus_string_init_const (&lookup, "dbus-no-user-with-this-name"); /* We use a username that almost certainly doesn't exist, because * if we used something like root it might get handled early in the * NSS search order, before we get as far as asking sssd or LDAP. */ - _dbus_parse_unix_user_from_config (&username, &ignored_uid); + _dbus_parse_unix_user_from_config (&lookup, &ignored_uid); + + /* Same for groups */ + _dbus_string_init_const (&lookup, "dbus-no-group-with-this-name"); + _dbus_parse_unix_group_from_config (&lookup, &ignored_gid); + _dbus_test_check_memleaks ("initial nss query"); } From 51b9fadf2002e5acadfa83f74eb65d48b4b31fab Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 10 Dec 2024 16:00:26 +0000 Subject: [PATCH 12/12] Update NEWS for 1.16.x Signed-off-by: Simon McVittie --- NEWS | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 19bf3b25..eed36bb9 100644 --- a/NEWS +++ b/NEWS @@ -62,7 +62,38 @@ New features and significant bug fixes: dbus 1.15.91 (UNRELEASED) ========================= -... +Build-time configuration changes: + +• When building with Meson, the embedded_tests option has been renamed + to intrusive_tests. This option adds test instrumentation in libdbus + and dbus-daemon, which reduces performance and is not secure. + For production builds of dbus in OS distributions, it must be false + (-Dintrusive_tests=false, which is the default) + During development, it should be set true (-Dintrusive_tests=true) + for full test coverage. (dbus#537, Simon McVittie) + +• Similarly, when building with CMake, the DBUS_BUILD_TESTS option no + longer enables intrusive test instrumentation. A new option + -DDBUS_ENABLE_INTRUSIVE_TESTS=ON is equivalent to the Meson build + system's -Dintrusive_tests=true. + +Bug fixes: + +• If a DBusWatch callback fails because there is insufficient memory, + make sure to retry it within a finite time (dbus#536, Petr Malat) + +• If intrusive test instrumentation is enabled, older versions of dbus + would simulate an out-of-memory condition once per 2**32 allocations, + even if not specifically requested. This is no longer done. + (dbus#535, Simon McVittie) + +• Fix compilation on non-Linux platforms with glibc, such as + Debian GNU/Hurd (dbus#539, Simon McVittie) + +• Avoid test failures with non-trivial NSS modules, similar to dbus#256 + (dbus#540, Simon McVittie) + +• Make paths in DBus1Config relocatable (dbus!499, Ralf Habacker) dbus 1.15.90 (2024-12-06) =========================