From 4ed9f00a7cbff498f74b7c5f4861280a50497c20 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 21 Feb 2022 15:53:38 +0000 Subject: spawn-unix: On Linux, don't try to increase OOM-killer protection The oom_score_adj parameter is a signed integer, with increasingly positive values being more likely to be killed by the OOM-killer, and increasingly negative values being less likely. Previously, we assumed that oom_score_adj would be negative or zero, and reset it to zero, which does not require privileges because it meant we're voluntarily giving up our OOM-killer protection. In particular, bus/dbus.service.in has OOMScoreAdjust=-900, which we don't want system services to inherit. However, systemd >= 250 has started putting a positive oom_score_adj on user processes, to make it more likely that the OOM killer will kill a user process rather than a system process. Changing from a positive oom_score_adj to zero is increasing protection from the OOM-killer, which only a privileged process is allowed to do, resulting in warnings whenever we carry out traditional (non-systemd) service activation on the session bus. To avoid this, do the equivalent of: if (oom_score_adj < 0) oom_score_adj = 0; which is always allowed. Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/374 Signed-off-by: Simon McVittie (cherry picked from commit c42bb64457c3b31e561ad9885c618e051af1171a) --- dbus/dbus-sysdeps-util-unix.c | 55 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 6 deletions(-) diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index dcf6dcff..ffbc7aea 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1595,29 +1595,68 @@ _dbus_reset_oom_score_adj (const char **error_str_p) const char *error_str = NULL; #ifdef O_CLOEXEC - fd = open ("/proc/self/oom_score_adj", O_WRONLY | 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_WRONLY); + fd = open ("/proc/self/oom_score_adj", O_RDWR); _dbus_fd_set_close_on_exec (fd); } if (fd >= 0) { - if (write (fd, "0", sizeof (char)) < 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 = "writing oom_score_adj error"; + error_str = "failed to read from /proc/self/oom_score_adj"; saved_errno = errno; + goto out; } - else + + /* 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; } - _dbus_close (fd, NULL); + 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 { @@ -1626,6 +1665,10 @@ _dbus_reset_oom_score_adj (const char **error_str_p) ret = TRUE; } +out: + if (fd >= 0) + _dbus_close (fd, NULL); + if (error_str_p != NULL) *error_str_p = error_str; -- cgit v1.2.1