From 524f79e86144b3e1b722f102f89ee070a9586278 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 21 Nov 2017 14:38:13 +0000 Subject: [PATCH 05/12] _dbus_server_new_for_socket: Properly disconnect during error unwinding _dbus_server_finalize_base() asserts that the socket has been disconnected, but in some OOM code paths we would call it without officially disconnecting. Do so. This means we need to be a bit more careful about what is socket_disconnect()'s responsibility to clean up, what is _dbus_server_new_for_socket()'s responsibility, and what is the caller's responsibility. Signed-off-by: Simon McVittie --- dbus/dbus-server-protected.h | 1 + dbus/dbus-server-socket.c | 27 ++++++++++++++++++++++----- dbus/dbus-server.c | 24 +++++++++++++++--------- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h index f613bf34..5664b2b4 100644 --- a/dbus/dbus-server-protected.h +++ b/dbus/dbus-server-protected.h @@ -96,6 +96,7 @@ dbus_bool_t _dbus_server_init_base (DBusServer *server, const DBusString *address, DBusError *error); void _dbus_server_finalize_base (DBusServer *server); +void _dbus_server_disconnect_unlocked (DBusServer *server); dbus_bool_t _dbus_server_add_watch (DBusServer *server, DBusWatch *watch); void _dbus_server_remove_watch (DBusServer *server, diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c index b5179be6..460b535c 100644 --- a/dbus/dbus-server-socket.c +++ b/dbus/dbus-server-socket.c @@ -241,8 +241,11 @@ socket_disconnect (DBusServer *server) socket_server->watch[i] = NULL; } - _dbus_close_socket (socket_server->fds[i], NULL); - _dbus_socket_invalidate (&socket_server->fds[i]); + if (_dbus_socket_is_valid (socket_server->fds[i])) + { + _dbus_close_socket (socket_server->fds[i], NULL); + _dbus_socket_invalidate (&socket_server->fds[i]); + } } if (socket_server->socket_name != NULL) @@ -336,10 +339,24 @@ _dbus_server_new_for_socket (DBusSocket *fds, socket_server->watch[i])) { int j; - for (j = 0 ; j < i ; j++) - _dbus_server_remove_watch (server, - socket_server->watch[j]); + /* The caller is still responsible for closing the fds until + * we return successfully, so don't let socket_disconnect() + * close them */ + for (j = 0; j < n_fds; j++) + _dbus_socket_invalidate (&socket_server->fds[i]); + + /* socket_disconnect() will try to remove all the watches; + * make sure it doesn't see the ones that weren't even added + * yet */ + for (j = i; j < n_fds; j++) + { + _dbus_watch_invalidate (socket_server->watch[i]); + _dbus_watch_unref (socket_server->watch[i]); + socket_server->watch[i] = NULL; + } + + _dbus_server_disconnect_unlocked (server); SERVER_UNLOCK (server); _dbus_server_finalize_base (&socket_server->base); goto failed_2; diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index ea9aff2d..d91df832 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -764,6 +764,20 @@ dbus_server_unref (DBusServer *server) } } +void +_dbus_server_disconnect_unlocked (DBusServer *server) +{ + _dbus_assert (server->vtable->disconnect != NULL); + + if (!server->disconnected) + { + /* this has to be first so recursive calls to disconnect don't happen */ + server->disconnected = TRUE; + + (* server->vtable->disconnect) (server); + } +} + /** * Releases the server's address and stops listening for * new clients. If called more than once, only the first @@ -780,15 +794,7 @@ dbus_server_disconnect (DBusServer *server) dbus_server_ref (server); SERVER_LOCK (server); - _dbus_assert (server->vtable->disconnect != NULL); - - if (!server->disconnected) - { - /* this has to be first so recursive calls to disconnect don't happen */ - server->disconnected = TRUE; - - (* server->vtable->disconnect) (server); - } + _dbus_server_disconnect_unlocked (server); SERVER_UNLOCK (server); dbus_server_unref (server); -- 2.15.0