Merge branch 'dbus-daemon-inherited-fds' into 'master'

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

See merge request dbus/dbus!50

Reviewed-by: rhabacker
Reviewed-by: pwithnall

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))

dbus/dbus-spawn-unix.c

@@ -1063,7 +1063,22 @@ do_exec (int                       child_err_report_fd,
       retval = fcntl (i, F_GETFD);
 
       if (retval != -1 && !(retval & FD_CLOEXEC))
-        _dbus_warn ("Fd %d did not have the close-on-exec flag set!", i);
+        {
+          char description[256] = { 0 };
+          char proc_self_fd[256] = { 0 };
+          size_t description_length = sizeof (description) - 1;
+
+          snprintf (proc_self_fd, sizeof (proc_self_fd) - 1,
+                    "/proc/self/fd/%d", i);
+          proc_self_fd[sizeof (proc_self_fd) - 1] = '\0';
+
+          if (readlink (proc_self_fd, description, description_length) <= 0)
+            snprintf (description, sizeof (description) - 1, "(unknown)");
+
+          description[sizeof (description) - 1] = '\0';
+          _dbus_warn ("Fd %d \"%s\" did not have the close-on-exec flag set!",
+                      i, description);
+        }
     }
 #endif
 

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);
 }
 
 /**

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,

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 */

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,