Commit 94806fb2 authored by Simon McVittie's avatar Simon McVittie

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's avatarSimon McVittie <smcv@collabora.com>
parent ebf487ef
......@@ -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))
......
......@@ -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);
}
/**
......
......@@ -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,
......
......@@ -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 */
......
......@@ -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,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment