bus: Don't crash if bus_context_create_client_policy() fails

If policy creation fails, we can't usefully leave a NULL policy in the
BusConnectionData. If we did, the next attempt to reload policy would
crash with a NULL dereference when we tried to unref it, or with
an assertion failure.

One situation in which we can legitimately fail to create a client policy
is an out-of-memory condition. Another is if we are unable to look up a
connection's supplementary groups with SO_PEERGROUPS, and also unable to
look up the connection's uid's groups in the system user database, for
example because it belongs to a user account that has been deleted (which
is sysadmin error, but can happen, particularly in automated test systems)
or because a service required by a Name Service Switch plugin has failed.

Keeping the last known policy is consistent with what happens to all
the connections that are after this one in iteration order: after we
early-return, all of those connections retain their previous policies
(which doesn't seem ideal either, but that's how this has always worked).

[smcv: Add commit message]
Co-authored-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/-/issues/343

(cherry picked from commit 63522f2887878e6b9e40c9bb6742484679631ea9)

diff --git a/bus/connection.c b/bus/connection.c
index 95bc6c0a5742c3bfc71ac4459b4810fa29558069..c9c0bed7546842081f6faf47b2a0380cc9d1ee3c 100644 (file)
--- a/bus/connection.c
+++ b/bus/connection.c
@@ -1660,22 +1660,26 @@ bus_connections_reload_policy (BusConnections *connections,
        link;
        link = _dbus_list_get_next_link (&(connections->completed), link))
     {
+      BusClientPolicy *policy;
+
       connection = link->data;
       d = BUS_CONNECTION_DATA (connection);
       _dbus_assert (d != NULL);
       _dbus_assert (d->policy != NULL);
 
-      bus_client_policy_unref (d->policy);
-      d->policy = bus_context_create_client_policy (connections->context,
-                                                    connection,
-                                                    error);
-      if (d->policy == NULL)
+      policy = bus_context_create_client_policy (connections->context,
+                                                 connection,
+                                                 error);
+      if (policy == NULL)
         {
           _dbus_verbose ("Failed to create security policy for connection %p\n",
                       connection);
           _DBUS_ASSERT_ERROR_IS_SET (error);
           return FALSE;
         }
+
+      bus_client_policy_unref (d->policy);
+      d->policy = policy;
     }
 
   return TRUE;