From d7f0662d369478e2d9c05be18a6a565786415d61 Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Fri, 4 Jul 2014 15:05:51 +0100 Subject: [PATCH] Stop listening on DBusServer sockets when reaching max_incomplete_connections https://bugs.freedesktop.org/show_bug.cgi?id=80919 --- bus/bus.c | 37 +++++++++++++++++++++++++++++++++++++ bus/bus.h | 1 + bus/connection.c | 34 ++++++++++------------------------ bus/connection.h | 3 ++- dbus/dbus-server-protected.h | 6 +++--- dbus/dbus-server.c | 19 +++++-------------- dbus/dbus-server.h | 1 - dbus/dbus-watch.c | 21 +++++++++++++++++++++ dbus/dbus-watch.h | 2 ++ 9 files changed, 81 insertions(+), 43 deletions(-) diff --git a/bus/bus.c b/bus/bus.c index a514e31..a3dce24 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -39,6 +39,7 @@ #include #include #include +#include #ifdef DBUS_CYGWIN #include @@ -68,6 +69,7 @@ struct BusContext unsigned int keep_umask : 1; unsigned int allow_anonymous : 1; unsigned int systemd_activation : 1; + dbus_bool_t watches_enabled; }; static dbus_int32_t server_data_slot = -1; @@ -758,6 +760,8 @@ bus_context_new (const DBusString *config_file, goto failed; } + context->watches_enabled = TRUE; + context->registry = bus_registry_new (context); if (context->registry == NULL) { @@ -1658,3 +1662,36 @@ bus_context_check_security_policy (BusContext *context, _dbus_verbose ("security policy allowing message\n"); return TRUE; } + +void +bus_context_check_all_watches (BusContext *context) +{ + DBusList *link; + dbus_bool_t enabled = TRUE; + + if (bus_connections_get_n_incomplete (context->connections) >= + bus_context_get_max_incomplete_connections (context)) + { + enabled = FALSE; + } + + if (context->watches_enabled == enabled) + return; + + context->watches_enabled = enabled; + + for (link = _dbus_list_get_first_link (&context->servers); + link != NULL; + link = _dbus_list_get_next_link (&context->servers, link)) + { + /* A BusContext might contains several DBusServer (if there are + * several configuration items) and a DBusServer might + * contain several DBusWatch in its DBusWatchList (if getaddrinfo + * returns several addresses on a dual IPv4-IPv6 stack or if + * systemd passes several fds). + * We want to enable/disable them all. + */ + DBusServer *server = link->data; + _dbus_server_toggle_all_watches (server, enabled); + } +} diff --git a/bus/bus.h b/bus/bus.h index 3597884..400c9d0 100644 --- a/bus/bus.h +++ b/bus/bus.h @@ -125,5 +125,6 @@ dbus_bool_t bus_context_check_security_policy (BusContext DBusConnection *proposed_recipient, DBusMessage *message, DBusError *error); +void bus_context_check_all_watches (BusContext *context); #endif /* BUS_BUS_H */ diff --git a/bus/connection.c b/bus/connection.c index ea2d155..d7c6426 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -293,6 +293,7 @@ bus_connection_disconnected (DBusConnection *connection) _dbus_list_remove_link (&d->connections->incomplete, d->link_in_connection_list); d->link_in_connection_list = NULL; d->connections->n_incomplete -= 1; + bus_context_check_all_watches (d->connections->context); } _dbus_assert (d->connections->n_incomplete >= 0); @@ -688,31 +689,14 @@ bus_connections_setup_connection (BusConnections *connections, dbus_connection_ref (connection); - /* Note that we might disconnect ourselves here, but it only takes - * effect on return to the main loop. We call this to free up - * expired connections if possible, and to queue the timeout for our - * own expiration. - */ bus_connections_expire_incomplete (connections); - /* And we might also disconnect ourselves here, but again it - * only takes effect on return to main loop. - */ - if (connections->n_incomplete > - bus_context_get_max_incomplete_connections (connections->context)) - { - _dbus_verbose ("Number of incomplete connections exceeds max, dropping oldest one\n"); - - _dbus_assert (connections->incomplete != NULL); - /* Disconnect the oldest unauthenticated connection. FIXME - * would it be more secure to drop a *random* connection? This - * algorithm seems to mean that if someone can create new - * connections quickly enough, they can keep anyone else from - * completing authentication. But random may or may not really - * help with that, a more elaborate solution might be required. - */ - dbus_connection_close (connections->incomplete->data); - } + /* The listening sockets are removed from the poll while n_incomplete + * reaches the max */ + _dbus_assert (connections->n_incomplete <= + bus_context_get_max_incomplete_connections (connections->context)); + + bus_context_check_all_watches (d->connections->context); retval = TRUE; @@ -1419,6 +1403,8 @@ bus_connection_complete (DBusConnection *connection, _dbus_assert (d->connections->n_incomplete >= 0); _dbus_assert (d->connections->n_completed > 0); + bus_context_check_all_watches (d->connections->context); + /* See if we can remove the timeout */ bus_connections_expire_incomplete (d->connections); @@ -2348,7 +2334,6 @@ bus_transaction_add_cancel_hook (BusTransaction *transaction, return TRUE; } -#ifdef DBUS_ENABLE_STATS int bus_connections_get_n_active (BusConnections *connections) { @@ -2361,6 +2346,7 @@ bus_connections_get_n_incomplete (BusConnections *connections) return connections->n_incomplete; } +#ifdef DBUS_ENABLE_STATS int bus_connections_get_total_match_rules (BusConnections *connections) { diff --git a/bus/connection.h b/bus/connection.h index 9f4f9ae..6fbcd38 100644 --- a/bus/connection.h +++ b/bus/connection.h @@ -139,9 +139,10 @@ dbus_bool_t bus_transaction_add_cancel_hook (BusTransaction * void *data, DBusFreeFunction free_data_function); -/* called by stats.c, only present if DBUS_ENABLE_STATS */ int bus_connections_get_n_active (BusConnections *connections); int bus_connections_get_n_incomplete (BusConnections *connections); + +/* called by stats.c, only present if DBUS_ENABLE_STATS */ int bus_connections_get_total_match_rules (BusConnections *connections); int bus_connections_get_peak_match_rules (BusConnections *connections); int bus_connections_get_peak_match_rules_per_conn (BusConnections *connections); diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index dd5234b..60102d1 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -99,9 +99,8 @@ dbus_bool_t _dbus_server_add_watch (DBusServer *server, DBusWatch *watch); void _dbus_server_remove_watch (DBusServer *server, DBusWatch *watch); -void _dbus_server_toggle_watch (DBusServer *server, - DBusWatch *watch, - dbus_bool_t enabled); +void _dbus_server_toggle_all_watches (DBusServer *server, + dbus_bool_t enabled); dbus_bool_t _dbus_server_add_timeout (DBusServer *server, DBusTimeout *timeout); void _dbus_server_remove_timeout (DBusServer *server, @@ -110,6 +109,7 @@ void _dbus_server_toggle_timeout (DBusServer *server, DBusTimeout *timeout, dbus_bool_t enabled); + void _dbus_server_ref_unlocked (DBusServer *server); void _dbus_server_unref_unlocked (DBusServer *server); diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index 19d8590..c1d5f6e 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -312,26 +312,17 @@ _dbus_server_remove_watch (DBusServer *server, } /** - * Toggles a watch and notifies app via server's - * DBusWatchToggledFunction if available. It's an error to call this - * function on a watch that was not previously added. + * Toggles all watch and notifies app via server's + * DBusWatchToggledFunction if available. * * @param server the server. - * @param watch the watch to toggle. * @param enabled whether to enable or disable */ void -_dbus_server_toggle_watch (DBusServer *server, - DBusWatch *watch, - dbus_bool_t enabled) +_dbus_server_toggle_all_watches (DBusServer *server, + dbus_bool_t enabled) { - _dbus_assert (watch != NULL); - - HAVE_LOCK_CHECK (server); - protected_change_watch (server, watch, - NULL, NULL, - _dbus_watch_list_toggle_watch, - enabled); + _dbus_watch_list_toggle_all_watches (server->watches, enabled); } /** Function to be called in protected_change_timeout() with refcount held */ diff --git a/dbus/dbus-server.h b/dbus/dbus-server.h index bdbefa0..57e14b8 100644 --- a/dbus/dbus-server.h +++ b/dbus/dbus-server.h @@ -98,7 +98,6 @@ dbus_bool_t dbus_server_set_data (DBusServer *server, DBUS_EXPORT void* dbus_server_get_data (DBusServer *server, int slot); - /** @} */ DBUS_END_DECLS diff --git a/dbus/dbus-watch.c b/dbus/dbus-watch.c index b82c57d..76a5d64 100644 --- a/dbus/dbus-watch.c +++ b/dbus/dbus-watch.c @@ -455,6 +455,27 @@ _dbus_watch_list_toggle_watch (DBusWatchList *watch_list, } /** + * Sets all watches to the given enabled state, invoking the + * application's DBusWatchToggledFunction if appropriate. + * + * @param watch_list the watch list. + * @param enabled #TRUE to enable + */ +void +_dbus_watch_list_toggle_all_watches (DBusWatchList *watch_list, + dbus_bool_t enabled) +{ + DBusList *link; + + for (link = _dbus_list_get_first_link (&watch_list->watches); + link != NULL; + link = _dbus_list_get_next_link (&watch_list->watches, link)) + { + _dbus_watch_list_toggle_watch (watch_list, link->data, enabled); + } +} + +/** * Sets the handler for the watch. * * @todo this function only exists because of the weird diff --git a/dbus/dbus-watch.h b/dbus/dbus-watch.h index c583214..321740e 100644 --- a/dbus/dbus-watch.h +++ b/dbus/dbus-watch.h @@ -76,6 +76,8 @@ void _dbus_watch_list_remove_watch (DBusWatchList *watch_li void _dbus_watch_list_toggle_watch (DBusWatchList *watch_list, DBusWatch *watch, dbus_bool_t enabled); +void _dbus_watch_list_toggle_all_watches (DBusWatchList *watch_list, + dbus_bool_t enabled); dbus_bool_t _dbus_watch_get_enabled (DBusWatch *watch); dbus_bool_t _dbus_watch_get_oom_last_time (DBusWatch *watch); -- 1.8.5.3