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.

(cherry picked from commit 0bb0316f5eb2c8d30e91feac571404687c6a0dc2)

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 6e4935d555d3b9fccd011a67546d520e97e51519..518f4d92c5da3102226dd05e796e49bb85888b9e 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..1146104
--- /dev/null
+++ b/src/shared/log-assert-critical.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include "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 768f446d20196d0c326d31fd28eb0fb04c4200ff..6954490a46cbb19a943810ef2c01cc29c9a4e731 100644 (file)
--- a/src/shared/meson.build
+++ b/src/shared/meson.build
@@ -115,6 +115,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 5c933c48010ce4cac1776491bbd4275ff194b6b8..81666cb60efe2814162f9aca4a8f980f55337394 100644 (file)
--- a/src/shared/tests.h
+++ b/src/shared/tests.h
@@ -7,6 +7,7 @@
 #include "errno-util.h"
 #include "forward.h"
 #include "log.h"
+#include "log-assert-critical.h"
 #include "static-destruct.h"
 #include "signal-util.h"
 #include "stdio-util.h"