diff options
author | Simon McVittie <smcv@collabora.com> | 2022-02-25 15:22:38 +0000 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2022-02-25 15:22:38 +0000 |
commit | b4448e60764d3cb92d0a16842ca7303a0e93a7ec (patch) | |
tree | 369690f8137d52be201d6aab063f21d65b397a68 | |
parent | e5922ee2714bf058ca58c4a3850023562353f869 (diff) | |
parent | c8e781872307294133af3ce18be3c561cc00d129 (diff) | |
download | dbus-b4448e60764d3cb92d0a16842ca7303a0e93a7ec.tar.gz |
Merge branch '1.12-backports' into 'dbus-1.12'
[1.12.x] Backport various fixes to dbus-1.12
See merge request dbus/dbus!258
-rw-r--r-- | bus/activation-helper.c | 8 | ||||
-rw-r--r-- | bus/signals.c | 4 | ||||
-rw-r--r-- | dbus/dbus-spawn.c | 26 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-unix.h | 2 | ||||
-rw-r--r-- | dbus/dbus-sysdeps-util-unix.c | 115 | ||||
-rw-r--r-- | test/integration/transient-services.sh | 15 |
6 files changed, 146 insertions, 24 deletions
diff --git a/bus/activation-helper.c b/bus/activation-helper.c index 5b6a0908..afe57fd3 100644 --- a/bus/activation-helper.c +++ b/bus/activation-helper.c @@ -32,6 +32,7 @@ #include "activation-helper.h" #include "activation-exit-codes.h" +#include <errno.h> #include <stdio.h> #include <stdlib.h> #include <string.h> @@ -43,6 +44,7 @@ #include <dbus/dbus-misc.h> #include <dbus/dbus-shell.h> #include <dbus/dbus-marshal-validate.h> +#include <dbus/dbus-sysdeps-unix.h> static BusDesktopFile * desktop_file_for_name (BusConfigParser *parser, @@ -337,11 +339,17 @@ exec_for_correct_user (char *exec, char *user, DBusError *error) char **argv; int argc; dbus_bool_t retval; + const char *error_str = NULL; argc = 0; retval = TRUE; argv = NULL; + /* Resetting the OOM score adjustment is best-effort, so we don't + * treat a failure to do so as fatal. */ + if (!_dbus_reset_oom_score_adj (&error_str)) + _dbus_warn ("%s: %s", error_str, strerror (errno)); + if (!switch_user (user, error)) return FALSE; diff --git a/bus/signals.c b/bus/signals.c index 6b7a464c..034e6e35 100644 --- a/bus/signals.c +++ b/bus/signals.c @@ -121,7 +121,7 @@ bus_match_rule_unref (BusMatchRule *rule) } } -#if defined(DBUS_ENABLE_VERBOSE_MODE) || defined(DBUS_ENABLE_STATS) +#if defined(DBUS_ENABLE_VERBOSE_MODE) || defined(DBUS_ENABLE_STATS) || defined(DBUS_ENABLE_EMBEDDED_TESTS) static dbus_bool_t append_key_and_escaped_value (DBusString *str, const char *token, const char *value) { @@ -311,7 +311,7 @@ match_rule_to_string (BusMatchRule *rule) _dbus_string_free (&str); return NULL; } -#endif /* defined(DBUS_ENABLE_VERBOSE_MODE) || defined(DBUS_ENABLE_STATS) */ +#endif /* defined(DBUS_ENABLE_VERBOSE_MODE) || defined(DBUS_ENABLE_STATS) || defined(DBUS_ENABLE_EMBEDDED_TESTS) */ dbus_bool_t bus_match_rule_set_message_type (BusMatchRule *rule, diff --git a/dbus/dbus-spawn.c b/dbus/dbus-spawn.c index 8ab529a4..0459dc21 100644 --- a/dbus/dbus-spawn.c +++ b/dbus/dbus-spawn.c @@ -1396,27 +1396,13 @@ _dbus_spawn_async_with_babysitter (DBusBabysitter **sitter_p, _dbus_assert_not_reached ("Got to code after write_err_and_exit()"); } else if (grandchild_pid == 0) - { -#ifdef __linux__ - int fd = -1; - -#ifdef O_CLOEXEC - fd = open ("/proc/self/oom_score_adj", O_WRONLY | O_CLOEXEC); -#endif - - if (fd < 0) - { - fd = open ("/proc/self/oom_score_adj", O_WRONLY); - _dbus_fd_set_close_on_exec (fd); - } + { + /* This might not succeed in a dbus-daemon that started as root + * and dropped privileges, so don't log an error on failure. + * (Also, we can't safely log errors here anyway, because logging + * is not async-signal safe). */ + _dbus_reset_oom_score_adj (NULL); - if (fd >= 0) - { - if (write (fd, "0", sizeof (char)) < 0) - _dbus_warn ("writing oom_score_adj error: %s", strerror (errno)); - _dbus_close (fd, NULL); - } -#endif /* Go back to ignoring SIGPIPE, since it's evil */ signal (SIGPIPE, SIG_IGN); diff --git a/dbus/dbus-sysdeps-unix.h b/dbus/dbus-sysdeps-unix.h index 830d5cd0..5de8b731 100644 --- a/dbus/dbus-sysdeps-unix.h +++ b/dbus/dbus-sysdeps-unix.h @@ -173,6 +173,8 @@ typedef void (* DBusSignalHandler) (int sig); void _dbus_set_signal_handler (int sig, DBusSignalHandler handler); +dbus_bool_t _dbus_reset_oom_score_adj (const char **error_str_p); + /** @} */ DBUS_END_DECLS diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index cc7bbe62..bed6fd3e 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1571,3 +1571,118 @@ _dbus_test_append_different_username (DBusString *username) } #endif + +/** + * If the current process has been protected from the Linux OOM killer + * (the oom_score_adj process parameter is negative), reset it to the + * default level of protection from the OOM killer (set oom_score_adj + * to zero). + * + * This function does not use DBusError, to avoid calling malloc(), so + * that it can be used in contexts where an async-signal-safe function + * is required (for example after fork()). Instead, on failure it sets + * errno and returns something like "Failed to open /dev/null" in + * *error_str_p. Callers are expected to combine *error_str_p + * with _dbus_strerror (errno) to get a full error report. + */ +dbus_bool_t +_dbus_reset_oom_score_adj (const char **error_str_p) +{ +#ifdef __linux__ + int fd = -1; + dbus_bool_t ret = FALSE; + int saved_errno = 0; + const char *error_str = NULL; + +#ifdef O_CLOEXEC + fd = open ("/proc/self/oom_score_adj", O_RDWR | O_CLOEXEC); +#endif + + if (fd < 0) + { + fd = open ("/proc/self/oom_score_adj", O_RDWR); + _dbus_fd_set_close_on_exec (fd); + } + + if (fd >= 0) + { + ssize_t read_result = -1; + /* It doesn't actually matter whether we read the whole file, + * as long as we get the presence or absence of the minus sign */ + char first_char = '\0'; + + read_result = read (fd, &first_char, 1); + + if (read_result < 0) + { + /* This probably can't actually happen in practice: if we can + * open it, then we can hopefully read from it */ + ret = FALSE; + error_str = "failed to read from /proc/self/oom_score_adj"; + saved_errno = errno; + goto out; + } + + /* If we are running with protection from the OOM killer + * (typical for the system dbus-daemon under systemd), then + * oom_score_adj will be negative. Drop that protection, + * returning to oom_score_adj = 0. + * + * Conversely, if we are running with increased susceptibility + * to the OOM killer (as user sessions typically do in + * systemd >= 250), oom_score_adj will be strictly positive, + * and we are not allowed to decrease it to 0 without privileges. + * + * If it's exactly 0 (typical for non-systemd systems, and + * user processes on older systemd) then there's no need to + * alter it. + * + * We shouldn't get an empty result, but if we do, assume it + * means zero and don't try to change it. */ + if (read_result == 0 || first_char != '-') + { + /* Nothing needs to be done: the OOM score adjustment is + * non-negative */ + ret = TRUE; + goto out; + } + + if (pwrite (fd, "0", sizeof (char), 0) < 0) + { + ret = FALSE; + error_str = "writing oom_score_adj error"; + saved_errno = errno; + goto out; + } + + /* Success */ + ret = TRUE; + } + else if (errno == ENOENT) + { + /* If /proc/self/oom_score_adj doesn't exist, assume the kernel + * doesn't support this feature and ignore it. */ + ret = TRUE; + } + else + { + ret = FALSE; + error_str = "open(/proc/self/oom_score_adj)"; + saved_errno = errno; + goto out; + } + +out: + if (fd >= 0) + _dbus_close (fd, NULL); + + if (error_str_p != NULL) + *error_str_p = error_str; + + errno = saved_errno; + return ret; +#else + /* nothing to do on this platform */ + return TRUE; +#endif +} diff --git a/test/integration/transient-services.sh b/test/integration/transient-services.sh index 2d946d9e..40bb8aed 100644 --- a/test/integration/transient-services.sh +++ b/test/integration/transient-services.sh @@ -74,8 +74,19 @@ trap cleanup EXIT echo "1..2" -# This is an integration test, so we expect the dbus-daemon to already be -# running +# If the dbus-daemon is launched on-demand by a systemd socket unit, it +# might not be there yet, even if the socket is +( +dbus-send --session --dest="org.freedesktop.DBus" \ + --type=method_call --print-reply /org/freedesktop/DBus \ + org.freedesktop.DBus.Peer.Ping || touch "$workdir/failed" \ +) 2>&1 | sed -e 's/^/# /' + +if [ -e "$workdir/failed" ]; then + echo "Bail out! Unable to ensure dbus-daemon has started" + exit 1 +fi + if ! test -d "$XDG_RUNTIME_DIR/dbus-1/services"; then echo "Bail out! $XDG_RUNTIME_DIR/dbus-1/services is not a directory" exit 1 |