From 326ae65ef3077a0806a5c9ec2d5f7fb1bf61f218 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 4 Nov 2014 14:41:54 +0000 Subject: [PATCH 1/3] Set fd rlimit to 64k for the system dbus-daemon Restore the original rlimit for activated services. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=85105 Reviewed-by: Lennart Poettering [Alban Crequy: make failure to restore original rlimit non-fatal] Reviewed-by: Simon McVittie [Alban Crequy: use new DBUS_SYSTEM_LOG_WARNING; skip if !DBUS_UNIX] [Simon McVittie: adjust DBUS_UNIX, add stub _dbus_rlimit_free on Windows] Signed-off-by: Simon McVittie --- bus/activation.c | 28 +++++++- bus/bus.c | 50 ++++++++++++--- bus/bus.h | 1 + dbus/dbus-sysdeps-util-unix.c | 145 +++++++++++++++++++++++++++++++++--------- dbus/dbus-sysdeps-util-win.c | 35 +++++++++- dbus/dbus-sysdeps.h | 11 +++- 6 files changed, 227 insertions(+), 43 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 149cca8..ecd19bb 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -1688,6 +1688,31 @@ out: return retval; } +static void +child_setup (void *user_data) +{ +#ifdef DBUS_UNIX + 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_INFO, + "Failed to reset fd limit before activating " + "service: %s: %s", + error.name, error.message); + } +#endif +} + + dbus_bool_t bus_activation_activate_service (BusActivation *activation, DBusConnection *connection, @@ -2121,7 +2146,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..47cc345 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; @@ -659,19 +660,38 @@ oom: static void raise_file_descriptor_limit (BusContext *context) { +#ifdef DBUS_UNIX + 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); + + if (context->initial_fd_limit == NULL) + { + bus_context_log (context, DBUS_SYSTEM_LOG_INFO, + "%s: %s", error.name, error.message); + dbus_error_free (&error); + return; + } - _dbus_request_file_descriptor_limit (limit); + /* 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_INFO, + "%s: %s", error.name, error.message); + dbus_error_free (&error); + return; + } +#endif } static dbus_bool_t @@ -1130,6 +1150,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 +1318,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 0d8a66c..199eb3d 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -378,53 +378,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 4678b11..ec9afbb 100644 --- a/dbus/dbus-sysdeps-util-win.c +++ b/dbus/dbus-sysdeps-util-win.c @@ -258,9 +258,42 @@ _dbus_change_to_daemon_user (const char *user, return TRUE; } +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 -_dbus_request_file_descriptor_limit (unsigned int limit) +_dbus_rlimit_free (DBusRLimit *lim) { + /* _dbus_rlimit_save_fd_limit() cannot return non-NULL on Windows + * so there cannot be anything to free */ + _dbus_assert (lim == NULL); } void diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 47ba2f4..5465128 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -546,8 +546,6 @@ dbus_bool_t _dbus_change_to_daemon_user (const char *user, void _dbus_flush_caches (void); -void _dbus_request_file_descriptor_limit (unsigned int limit); - /* * replaces the term DBUS_PREFIX in configure_time_path by the * current dbus installation directory. On unix this function is a noop @@ -566,6 +564,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 -- 2.1.1