From b047a62a14d21ba9125e9fdd1de3aa138d568ec3 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 16 Oct 2014 15:09:47 +0100 Subject: [PATCH 2/2] Set fd rlimit to 64k for the system dbus-daemon Restore the original rlimit for activated services. [make restore fd limit non-fatal -alban] [use LOG_WARNING -alban] [use #ifdef DBUS_UNIX -alban] https://bugs.freedesktop.org/show_bug.cgi?id=85105 --- bus/activation.c | 26 +++++++- bus/bus.c | 52 ++++++++++++--- bus/bus.h | 1 + dbus/dbus-sysdeps-util-unix.c | 145 +++++++++++++++++++++++++++++++++--------- dbus/dbus-sysdeps-util-win.c | 29 ++++++++- dbus/dbus-sysdeps.h | 9 +++ 6 files changed, 220 insertions(+), 42 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index bdf9cf3..7116784 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1688,6 +1688,29 @@ out: return retval; } +static void +child_setup (void *user_data) +{ + BusActivation *activation = user_data; + DBusRLimit *initial_fd_limit; + DBusError error; + + dbus_error_init (&error); + initial_fd_limit = bus_context_get_initial_fd_limit (activation->context); + + if (initial_fd_limit != NULL && + !_dbus_rlimit_restore_fd_limit (initial_fd_limit, &error)) + { + /* unfortunately we don't actually know the service name here */ + bus_context_log (activation->context, + DBUS_SYSTEM_LOG_WARNING, + "Failed to reset fd limit before activating " + "service: %s: %s", + error.name, error.message); + } +} + + dbus_bool_t bus_activation_activate_service (BusActivation *activation, DBusConnection *connection, @@ -2121,7 +2144,8 @@ bus_activation_activate_service (BusActivation *activation, service_name, argv, envp, - NULL, activation, + child_setup, + activation, &tmp_error)) { _dbus_verbose ("Failed to spawn child\n"); diff --git a/bus/bus.c b/bus/bus.c index 35d4075..a5db6b2 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -64,6 +64,7 @@ struct BusContext BusPolicy *policy; BusMatchmaker *matchmaker; BusLimits limits; + DBusRLimit *initial_fd_limit; unsigned int fork : 1; unsigned int syslog : 1; unsigned int keep_umask : 1; @@ -656,23 +657,42 @@ oom: return FALSE; } +#ifdef DBUS_UNIX static void raise_file_descriptor_limit (BusContext *context) { + DBusError error = DBUS_ERROR_INIT; - /* I just picked this out of thin air; we need some extra - * descriptors for things like any internal pipes we create, - * inotify, connections to SELinux, etc. - */ - unsigned int arbitrary_extra_fds = 32; - unsigned int limit; + /* we only do this once */ + if (context->initial_fd_limit != NULL) + return; - limit = context->limits.max_completed_connections + - context->limits.max_incomplete_connections - + arbitrary_extra_fds; + context->initial_fd_limit = _dbus_rlimit_save_fd_limit (&error); - _dbus_request_file_descriptor_limit (limit); + if (context->initial_fd_limit == NULL) + { + bus_context_log (context, DBUS_SYSTEM_LOG_WARNING, + "%s: %s", error.name, error.message); + dbus_error_free (&error); + return; + } + + /* We used to compute a suitable rlimit based on the configured number + * of connections, but that breaks down as soon as we allow fd-passing, + * because each connection is allowed to pass 64 fds to us, and if + * they all did, we'd hit kernel limits. We now hard-code 64k as a + * good limit, like systemd does: that's enough to avoid DoS from + * anything short of multiple uids conspiring against us. + */ + if (!_dbus_rlimit_raise_fd_limit_if_privileged (65536, &error)) + { + bus_context_log (context, DBUS_SYSTEM_LOG_WARNING, + "%s: %s", error.name, error.message); + dbus_error_free (&error); + return; + } } +#endif static dbus_bool_t process_config_postinit (BusContext *context, @@ -682,7 +702,9 @@ process_config_postinit (BusContext *context, DBusHashTable *service_context_table; DBusList *watched_dirs = NULL; +#ifdef DBUS_UNIX raise_file_descriptor_limit (context); +#endif service_context_table = bus_config_parser_steal_service_context_table (parser); if (!bus_registry_set_service_context_table (context->registry, @@ -1130,6 +1152,10 @@ bus_context_unref (BusContext *context) dbus_free (context->pidfile); } + + if (context->initial_fd_limit) + _dbus_rlimit_free (context->initial_fd_limit); + dbus_free (context); dbus_server_free_data_slot (&server_data_slot); @@ -1294,6 +1320,12 @@ bus_context_get_reply_timeout (BusContext *context) return context->limits.reply_timeout; } +DBusRLimit * +bus_context_get_initial_fd_limit (BusContext *context) +{ + return context->initial_fd_limit; +} + void bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, ...) _DBUS_GNUC_PRINTF (3, 4); diff --git a/bus/bus.h b/bus/bus.h index 7d0b369..dac6ea5 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -116,6 +116,7 @@ int bus_context_get_max_services_per_connection (BusContext int bus_context_get_max_match_rules_per_connection (BusContext *context); int bus_context_get_max_replies_per_connection (BusContext *context); int bus_context_get_reply_timeout (BusContext *context); +DBusRLimit * bus_context_get_initial_fd_limit (BusContext *context); void bus_context_log (BusContext *context, DBusSystemLogSeverity severity, const char *msg, diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index f410806..9b724cc 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -380,53 +380,140 @@ _dbus_change_to_daemon_user (const char *user, } #endif /* !HAVE_LIBAUDIT */ +#ifdef HAVE_SETRLIMIT -/** - * Attempt to ensure that the current process can open - * at least @p limit file descriptors. - * - * If @p limit is lower than the current, it will not be - * lowered. No error is returned if the request can - * not be satisfied. - * - * @param limit number of file descriptors +/* We assume that if we have setrlimit, we also have getrlimit and + * struct rlimit. */ -void -_dbus_request_file_descriptor_limit (unsigned int limit) + +struct DBusRLimit { + struct rlimit lim; +}; + +DBusRLimit * +_dbus_rlimit_save_fd_limit (DBusError *error) +{ + DBusRLimit *self; + + self = dbus_new0 (DBusRLimit, 1); + + if (self == NULL) + { + _DBUS_SET_OOM (error); + return NULL; + } + + if (getrlimit (RLIMIT_NOFILE, &self->lim) < 0) + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to get fd limit: %s", _dbus_strerror (errno)); + dbus_free (self); + return NULL; + } + + return self; +} + +dbus_bool_t +_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int desired, + DBusError *error) { -#ifdef HAVE_SETRLIMIT struct rlimit lim; - struct rlimit target_lim; /* No point to doing this practically speaking * if we're not uid 0. We expect the system * bus to use this before we change UID, and - * the session bus takes the Linux default - * of 1024 for both cur and max. + * the session bus takes the Linux default, + * currently 1024 for cur and 4096 for max. */ if (getuid () != 0) - return; + { + /* not an error, we're probably the session bus */ + return TRUE; + } if (getrlimit (RLIMIT_NOFILE, &lim) < 0) - return; + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to get fd limit: %s", _dbus_strerror (errno)); + return FALSE; + } - if (lim.rlim_cur >= limit) - return; + if (lim.rlim_cur == RLIM_INFINITY || lim.rlim_cur >= desired) + { + /* not an error, everything is fine */ + return TRUE; + } /* Ignore "maximum limit", assume we have the "superuser" * privileges. On Linux this is CAP_SYS_RESOURCE. */ - target_lim.rlim_cur = target_lim.rlim_max = limit; - /* Also ignore errors; if we fail, we will at least work - * up to whatever limit we had, which seems better than - * just outright aborting. - * - * However, in the future we should probably log this so OS builders - * have a chance to notice any misconfiguration like dbus-daemon - * being started without CAP_SYS_RESOURCE. - */ - setrlimit (RLIMIT_NOFILE, &target_lim); + lim.rlim_cur = lim.rlim_max = desired; + + if (setrlimit (RLIMIT_NOFILE, &lim) < 0) + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to set fd limit to %u: %s", + desired, _dbus_strerror (errno)); + return FALSE; + } + + return TRUE; +} + +dbus_bool_t +_dbus_rlimit_restore_fd_limit (DBusRLimit *saved, + DBusError *error) +{ + if (setrlimit (RLIMIT_NOFILE, &saved->lim) < 0) + { + dbus_set_error (error, _dbus_error_from_errno (errno), + "Failed to restore old fd limit: %s", + _dbus_strerror (errno)); + return FALSE; + } + + return TRUE; +} + +#else /* !HAVE_SETRLIMIT */ + +static void +fd_limit_not_supported (DBusError *error) +{ + dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, + "cannot change fd limit on this platform"); +} + +DBusRLimit * +_dbus_rlimit_save_fd_limit (DBusError *error) +{ + fd_limit_not_supported (error); + return NULL; +} + +dbus_bool_t +_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int desired, + DBusError *error) +{ + fd_limit_not_supported (error); + return FALSE; +} + +dbus_bool_t +_dbus_rlimit_restore_fd_limit (DBusRLimit *saved, + DBusError *error) +{ + fd_limit_not_supported (error); + return FALSE; +} + #endif + +void +_dbus_rlimit_free (DBusRLimit *lim) +{ + dbus_free (lim); } void diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c index dd4b233..1850aae 100644 --- a/dbus/dbus-sysdeps-util-win.c +++ b/dbus/dbus-sysdeps-util-win.c @@ -258,9 +258,34 @@ _dbus_change_to_daemon_user (const char *user, return TRUE; } -void -_dbus_request_file_descriptor_limit (unsigned int limit) +static void +fd_limit_not_supported (DBusError *error) +{ + dbus_set_error (error, DBUS_ERROR_NOT_SUPPORTED, + "cannot change fd limit on this platform"); +} + +DBusRLimit * +_dbus_rlimit_save_fd_limit (DBusError *error) { + fd_limit_not_supported (error); + return NULL; +} + +dbus_bool_t +_dbus_rlimit_raise_fd_limit_if_privileged (unsigned int desired, + DBusError *error) +{ + fd_limit_not_supported (error); + return FALSE; +} + +dbus_bool_t +_dbus_rlimit_restore_fd_limit (DBusRLimit *saved, + DBusError *error) +{ + fd_limit_not_supported (error); + return FALSE; } void diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index a78390e..f122f1b 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -566,6 +566,15 @@ _dbus_replace_install_prefix (const char *configure_time_path); */ #define DBUS_DEFAULT_MESSAGE_UNIX_FDS 16 +typedef struct DBusRLimit DBusRLimit; + +DBusRLimit *_dbus_rlimit_save_fd_limit (DBusError *error); +dbus_bool_t _dbus_rlimit_raise_fd_limit_if_privileged (unsigned int desired, + DBusError *error); +dbus_bool_t _dbus_rlimit_restore_fd_limit (DBusRLimit *saved, + DBusError *error); +void _dbus_rlimit_free (DBusRLimit *lim); + /** @} */ DBUS_END_DECLS -- 1.8.5.3