Do not use "critical assert_return" in libsystemd or libudev

Previously, when compiled in developer mode, a call into libsystemd with
invalid parameters would result in an abort. This means that it's effectively
impossible to install such libsystemd in a normal system, since various
third-party programs may now abort. A shared library should generally never
abort or exit the calling program.

In python-systemd, the test suite calls into libsystemd, to check if the proper
return values are received and propagated through the Python wrappers.
Obviously with libsystemd compiled from git, the test suite now fails
in a nasty way.

So rework the code to set assert_return_is_critical similarly to how we handle
mempool enablement: the function that returns true is declared as a week
symbol, and we "opt in" by linking a file that provides the function in
libsystemd-shared. Effectively, libsystemd and libudev always have
assert_return_is_critical==false, and our binaries and modules enable it
conditionally.

diff --git a/src/basic/assert-util.c b/src/basic/assert-util.c
index 87b760e8fea8fb2b91d5665d4f69e7307eecfaf0..0a068ee82d99ce932bf27e701e4462d2136bfa1e 100644 (file)
--- a/src/basic/assert-util.c
+++ b/src/basic/assert-util.c
@@ -4,40 +4,13 @@
 #include <stdlib.h>
 
 #include "assert-util.h"
-#include "env-util.h"
 #include "errno-util.h"
 #include "log.h"
 
-static bool assert_return_is_critical = BUILD_MODE_DEVELOPER;
-
 /* Akin to glibc's __abort_msg; which is private and we hence cannot
  * use here. */
 static char *log_abort_msg = NULL;
 
-void log_set_assert_return_is_critical(bool b) {
-        assert_return_is_critical = b;
-}
-
-void log_set_assert_return_is_critical_from_env(void) {
-        static int cached = INT_MIN;
-        int r;
-
-        if (cached == INT_MIN) {
-                r = secure_getenv_bool("SYSTEMD_ASSERT_RETURN_IS_CRITICAL");
-                if (r < 0 && r != -ENXIO)
-                        log_debug_errno(r, "Failed to parse $SYSTEMD_ASSERT_RETURN_IS_CRITICAL, ignoring: %m");
-
-                cached = r;
-        }
-
-        if (cached >= 0)
-                log_set_assert_return_is_critical(cached);
-}
-
-bool log_get_assert_return_is_critical(void) {
-        return assert_return_is_critical;
-}
-
 static void log_assert(
         int level,
         const char *text,
@@ -73,8 +46,8 @@ _noreturn_ void log_assert_failed_unreachable(const char *file, int line, const
 }
 
 void log_assert_failed_return(const char *text, const char *file, int line, const char *func) {
-
-        if (assert_return_is_critical)
+        /* log_get_assert_return_is_critical is a weak symbol. It may be NULL. */
+        if (log_get_assert_return_is_critical && log_get_assert_return_is_critical())
                 log_assert_failed(text, file, line, func);
 
         PROTECT_ERRNO;

diff --git a/src/basic/assert-util.h b/src/basic/assert-util.h
index fa3e13b220a7ef59e539a4b52bc6a4bbb6bfb85c..899a4365601f709acef78b798759c0cc4ed9755b 100644 (file)
--- a/src/basic/assert-util.h
+++ b/src/basic/assert-util.h
@@ -5,9 +5,7 @@
 
 /* Logging for various assertions */
 
-void log_set_assert_return_is_critical(bool b);
-void log_set_assert_return_is_critical_from_env(void);
-bool log_get_assert_return_is_critical(void) _pure_;
+bool log_get_assert_return_is_critical(void) _weak_ _pure_;
 
 void log_assert_failed_return(const char *text, const char *file, int line, const char *func);
 

diff --git a/src/fuzz/fuzz.h b/src/fuzz/fuzz.h
index a4cc7ab67dd56ccf6a0e08775db42a415504a993..be708abfd1bf2ab9651cfe90aebb767b636bc169 100644 (file)
--- a/src/fuzz/fuzz.h
+++ b/src/fuzz/fuzz.h
@@ -7,6 +7,7 @@
 
 #include "fileio.h"
 #include "log.h"
+#include "log-assert-critical.h"
 
 /* The entry point into the fuzzer */
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size);

diff --git a/src/shared/log-assert-critical.c b/src/shared/log-assert-critical.c
new file mode 100644 (file)
index 0000000..e30dc7d
--- /dev/null
+++ b/src/shared/log-assert-critical.c
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "env-util.h"
+#include "log.h"
+#include "log-assert-critical.h"
+
+static bool assert_return_is_critical = BUILD_MODE_DEVELOPER;
+
+void log_set_assert_return_is_critical(bool b) {
+        assert_return_is_critical = b;
+}
+
+void log_set_assert_return_is_critical_from_env(void) {
+        static int cached = INT_MIN;
+        int r;
+
+        if (cached == INT_MIN) {
+                r = secure_getenv_bool("SYSTEMD_ASSERT_RETURN_IS_CRITICAL");
+                if (r < 0 && r != -ENXIO)
+                        log_debug_errno(r, "Failed to parse $SYSTEMD_ASSERT_RETURN_IS_CRITICAL, ignoring: %m");
+
+                cached = r;
+        }
+
+        if (cached >= 0)
+                log_set_assert_return_is_critical(cached);
+}
+
+bool log_get_assert_return_is_critical(void) {
+        return assert_return_is_critical;
+}

diff --git a/src/shared/log-assert-critical.h b/src/shared/log-assert-critical.h
new file mode 100644 (file)
index 0000000..62c7360
--- /dev/null
+++ b/src/shared/log-assert-critical.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include "basic-forward.h"
+
+void log_set_assert_return_is_critical(bool b);
+void log_set_assert_return_is_critical_from_env(void);

diff --git a/src/shared/meson.build b/src/shared/meson.build
index 15763d4c65ea39c94922ff777076b93d1d40028b..38915f38851b464a7e1dbbcfab4818162c64d24c 100644 (file)
--- a/src/shared/meson.build
+++ b/src/shared/meson.build
@@ -119,6 +119,7 @@ shared_sources = files(
         'libmount-util.c',
         'local-addresses.c',
         'locale-setup.c',
+        'log-assert-critical.c',
         'logs-show.c',
         'loop-util.c',
         'loopback-setup.c',

diff --git a/src/shared/nss-util.c b/src/shared/nss-util.c
index 32a36cd57e7a24c0255c45df893ba45f4ca02a29..5a40c40a410dc98aff9d54bec0101d68a4e50e03 100644 (file)
--- a/src/shared/nss-util.c
+++ b/src/shared/nss-util.c
@@ -6,6 +6,7 @@
 
 #include "assert-util.h"
 #include "log.h"
+#include "log-assert-critical.h"
 #include "nss-util.h"
 
 sd_json_dispatch_flags_t nss_json_dispatch_flags = SD_JSON_ALLOW_EXTENSIONS;

diff --git a/src/shared/tests.h b/src/shared/tests.h
index 3f50b3dc54ce5c1507759f717e9a3aeb407bf49e..a408fbd087ce9b0d65410dd5bb77ea86fd21cf1e 100644 (file)
--- a/src/shared/tests.h
+++ b/src/shared/tests.h
@@ -7,6 +7,7 @@
 #include "errno-util.h"
 #include "shared-forward.h"
 #include "log.h"
+#include "log-assert-critical.h"
 #include "static-destruct.h"
 #include "signal-util.h"
 #include "stdio-util.h"