Don't let dbus-daemon and its subprocesses inherit unnecessary fds

This should avoid test failures under CMake in which the
dbus-daemon inherits an unwanted fd from CMake's test framework, causing
the close-on-exec check before executing activated services to fail.

The dbus-daemon now marks all fds that it inherits, except for its
stdin, stdout and stderr, to be closed on exec. For completeness, the
dbus-daemons run by dbus-run-session and dbus-launch also now inherit
stdin, stdout, stderr and the pipes used to communicate with their
callers, but nothing else.

Signed-off-by: Simon McVittie <smcv@collabora.com>

diff --git a/bus/main.c b/bus/main.c
index 6e8859aae19a3c037b4026387b6dab2fbd281d29..84f601b3086fbaba543d1065c0454d26672f8102 100644 (file)
--- a/bus/main.c
+++ b/bus/main.c
@@ -423,6 +423,10 @@ main (int argc, char **argv)
                error_str, _dbus_strerror (errno));
       return 1;
     }
+
+  /* Set all fds >= 3 close-on-execute. We don't want activated services
+   * to inherit fds we might have inherited from our caller. */
+  _dbus_fd_set_all_close_on_exec ();
 #endif
 
   if (!_dbus_string_init (&config_file))

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 026f6b20799a297915967df17a867ee8f27bcb1c..245bbc4b0b870671dab469b8a726b56a04e009a8 100644 (file)
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -3500,6 +3500,28 @@ _dbus_fd_set_close_on_exec (int fd)
   fcntl (fd, F_SETFD, val);
 }
 
+/**
+ * Sets the file descriptor to *not* be close-on-exec. This can be called
+ * after _dbus_fd_set_all_close_on_exec() to make exceptions for pipes
+ * used to communicate with child processes.
+ *
+ * @param fd the file descriptor
+ */
+void
+_dbus_fd_clear_close_on_exec (int fd)
+{
+  int val;
+
+  val = fcntl (fd, F_GETFD, 0);
+
+  if (val < 0)
+    return;
+
+  val &= ~FD_CLOEXEC;
+
+  fcntl (fd, F_SETFD, val);
+}
+
 /**
  * Closes a file descriptor.
  *
@@ -4672,12 +4694,18 @@ _dbus_socket_can_pass_unix_fd (DBusSocket fd)
 #endif
 }
 
-/**
- * Closes all file descriptors except the first three (i.e. stdin,
- * stdout, stderr).
+static void
+close_ignore_error (int fd)
+{
+  close (fd);
+}
+
+/*
+ * Similar to Solaris fdwalk(3), but without the ability to stop iteration,
+ * and may call func for integers that are not actually valid fds.
  */
-void
-_dbus_close_all (void)
+static void
+act_on_fds_3_and_up (void (*func) (int fd))
 {
   int maxfds, i;
 
@@ -4716,7 +4744,7 @@ _dbus_close_all (void)
           if (fd == dirfd (d))
             continue;
 
-          close (fd);
+          func (fd);
         }
 
       closedir (d);
@@ -4734,7 +4762,27 @@ _dbus_close_all (void)
 
   /* close all inherited fds */
   for (i = 3; i < maxfds; i++)
-    close (i);
+    func (i);
+}
+
+/**
+ * Closes all file descriptors except the first three (i.e. stdin,
+ * stdout, stderr).
+ */
+void
+_dbus_close_all (void)
+{
+  act_on_fds_3_and_up (close_ignore_error);
+}
+
+/**
+ * Sets all file descriptors except the first three (i.e. stdin,
+ * stdout, stderr) to be close-on-execute.
+ */
+void
+_dbus_fd_set_all_close_on_exec (void)
+{
+  act_on_fds_3_and_up (_dbus_fd_set_close_on_exec);
 }
 
 /**

diff --git a/dbus/dbus-sysdeps-unix.h b/dbus/dbus-sysdeps-unix.h
index 8d3df2d672881709d8cf5bf860372c2a568b17c4..8523f8cbd57a095e19f9d15a86b9bfaada114b5a 100644 (file)
--- a/dbus/dbus-sysdeps-unix.h
+++ b/dbus/dbus-sysdeps-unix.h
@@ -146,6 +146,10 @@ dbus_bool_t _dbus_parse_uid (const DBusString  *uid_str,
 
 DBUS_PRIVATE_EXPORT
 void _dbus_close_all (void);
+DBUS_PRIVATE_EXPORT
+void _dbus_fd_set_all_close_on_exec (void);
+DBUS_PRIVATE_EXPORT
+void _dbus_fd_clear_close_on_exec (int fd);
 
 dbus_bool_t _dbus_append_address_from_socket (DBusSocket  fd,
                                               DBusString *address,

diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c
index 7217d5b32c6a0fe96b96b39cb942a6623d1c8495..f1a5b9708a99e8142a1ab167fc447581aa0b7179 100644 (file)
--- a/tools/dbus-launch.c
+++ b/tools/dbus-launch.c
@@ -1226,7 +1226,17 @@ main (int argc, char **argv)
                "%d", bus_address_to_launcher_pipe[WRITE_END]);
 
       verbose ("Calling exec()\n");
- 
+
+      /* Set all fds >= 3 close-on-execute, except for the ones that
+       * can't be. We don't want dbus-daemon to inherit random fds we
+       * might have inherited from our caller. (Note that in the
+       * deprecated form "dbus-launch myapp", we *do* let "myapp" inherit
+       * them, in an attempt to be as close as possible to being a
+       * transparent wrapper.) */
+      _dbus_fd_set_all_close_on_exec ();
+      _dbus_fd_clear_close_on_exec (bus_address_to_launcher_pipe[WRITE_END]);
+      _dbus_fd_clear_close_on_exec (bus_pid_to_launcher_pipe[WRITE_END]);
+
 #ifdef DBUS_ENABLE_EMBEDDED_TESTS
       {
         /* exec from testdir */

diff --git a/tools/dbus-run-session.c b/tools/dbus-run-session.c
index face95a1bc706845be708a669cd69eb2ede1de11..f608a55cb8b04d3a1e1c217b66e57e8522335c76 100644 (file)
--- a/tools/dbus-run-session.c
+++ b/tools/dbus-run-session.c
@@ -39,6 +39,7 @@
 #ifdef DBUS_UNIX
 #include <sys/wait.h>
 #include <signal.h>
+#include <dbus/dbus-sysdeps-unix.h>
 #else
 #include <dbus/dbus-internals.h>
 #include <dbus/dbus-sysdeps-win.h>
@@ -193,6 +194,14 @@ exec_dbus_daemon (const char *dbus_daemon,
 
   close (bus_address_pipe[PIPE_READ_END]);
 
+  /* Set all fds >= 3 close-on-execute, except for the one that can't be.
+   * We don't want dbus-daemon to inherit random fds we might have
+   * inherited from our caller. (Note that we *do* let the wrapped process
+   * inherit them in exec_app(), in an attempt to be as close as possible
+   * to being a transparent wrapper.) */
+  _dbus_fd_set_all_close_on_exec ();
+  _dbus_fd_clear_close_on_exec (bus_address_pipe[PIPE_WRITE_END]);
+
   sprintf (write_address_fd_as_string, "%d", bus_address_pipe[PIPE_WRITE_END]);
 
   execlp (dbus_daemon,