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

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