From 94806fb2c711338841ca85b82793b77d95ed0f1c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 20 Nov 2018 18:06:34 +0000 Subject: [PATCH] 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 --- bus/main.c | 4 +++ dbus/dbus-sysdeps-unix.c | 62 +++++++++++++++++++++++++++++++++++----- dbus/dbus-sysdeps-unix.h | 4 +++ tools/dbus-launch.c | 12 +++++++- tools/dbus-run-session.c | 9 ++++++ 5 files changed, 83 insertions(+), 8 deletions(-) diff --git a/bus/main.c b/bus/main.c index 6e8859aae..84f601b30 100644 --- 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 026f6b207..245bbc4b0 100644 --- 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 8d3df2d67..8523f8cbd 100644 --- 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 7217d5b32..f1a5b9708 100644 --- 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 face95a1b..f608a55cb 100644 --- a/tools/dbus-run-session.c +++ b/tools/dbus-run-session.c @@ -39,6 +39,7 @@ #ifdef DBUS_UNIX #include #include +#include #else #include #include @@ -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, -- 2.47.3