diff --git a/bus/activation-helper.c b/bus/activation-helper.c index 8172b6cf..8a4fd732 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 #include #include #include @@ -43,6 +44,7 @@ #include #include #include +#include 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/dbus/dbus-spawn-unix.c b/dbus/dbus-spawn-unix.c index 5550ffac..f3577e70 100644 --- a/dbus/dbus-spawn-unix.c +++ b/dbus/dbus-spawn-unix.c @@ -1443,27 +1443,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; + { + /* 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); -#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); - } - - 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 e86de6d9..a051b542 100644 --- a/dbus/dbus-sysdeps-unix.h +++ b/dbus/dbus-sysdeps-unix.h @@ -174,6 +174,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 878ca2ea..314ce64b 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1603,3 +1603,118 @@ _dbus_daemon_report_stopping (void) sd_notify (0, "STOPPING=1"); #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 +}