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 <smcv@collabora.com>
(cherry picked from commit c42bb64457c3b31e561ad9885c618e051af1171a)

1 files 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;