_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 <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=89104

3 files changed, 38 insertions, 14 deletions

diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h
index f613bf34e..5664b2b4e 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 b5179be6a..460b535ca 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 ea9aff2db..d91df8325 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);