From f349e6b8c50ea6faa48c8261198cf1b07bf59a79 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Sun, 13 Feb 2005 17:16:25 +0000 Subject: 2005-02-13 Havoc Pennington * dbus/dbus-object-tree.c (handle_default_introspect_and_unlock): fix a double-unlock * dbus/dbus-connection.c (_dbus_connection_detach_pending_call_unlocked): add this Initial semi-correct pass through to fix thread locking; there are still some issues with the condition variable paths I'm pretty sure * dbus/dbus-server.c: add a mutex on DBusServer and appropriate lock/unlock calls * dbus/dbus-connection.c (_dbus_connection_do_iteration_unlocked): rename to add _unlocked (struct DBusConnection): move "dispatch_acquired" and "io_path_acquired" to use only one bit each. (CONNECTION_LOCK, CONNECTION_UNLOCK): add checks with !DBUS_DISABLE_CHECKS (dbus_connection_set_watch_functions): hacky fix to reentrancy (_dbus_connection_add_watch, _dbus_connection_remove_watch) (_dbus_connection_toggle_watch, _dbus_connection_add_timeout) (_dbus_connection_remove_timeout) (_dbus_connection_toggle_timeout): drop lock when calling out to user functions; done in a hacky/bad way. (_dbus_connection_send_and_unlock): add a missing unlock (_dbus_connection_block_for_reply): add a missing unlock * dbus/dbus-transport.c (_dbus_transport_get_is_authenticated): drop lock in a hacky probably unsafe way to call out to user function --- dbus/dbus-transport.c | 59 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 13 deletions(-) (limited to 'dbus/dbus-transport.c') diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index 94d4a8a8..c08573d8 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -388,10 +388,12 @@ _dbus_transport_unref (DBusTransport *transport) { _dbus_assert (transport != NULL); _dbus_assert (transport->refcount > 0); - + transport->refcount -= 1; if (transport->refcount == 0) { + _dbus_verbose ("%s: finalizing\n", _DBUS_FUNCTION_NAME); + _dbus_assert (transport->vtable->finalize != NULL); (* transport->vtable->finalize) (transport); @@ -409,14 +411,18 @@ _dbus_transport_unref (DBusTransport *transport) void _dbus_transport_disconnect (DBusTransport *transport) { + _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME); + _dbus_assert (transport->vtable->disconnect != NULL); - + if (transport->disconnected) return; (* transport->vtable->disconnect) (transport); transport->disconnected = TRUE; + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } /** @@ -437,7 +443,8 @@ _dbus_transport_get_is_connected (DBusTransport *transport) * Returns #TRUE if we have been authenticated. Will return #TRUE * even if the transport is disconnected. * - * @todo needs to drop connection->mutex when calling the unix_user_function + * @todo we drop connection->mutex when calling the unix_user_function, + * which may not be safe really. * * @param transport the transport * @returns whether we're authenticated @@ -453,6 +460,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) if (transport->disconnected) return FALSE; + + /* paranoia ref since we call user callbacks sometimes */ + _dbus_connection_ref_unlocked (transport->connection); maybe_authenticated = (!(transport->send_credentials_pending || @@ -486,21 +496,40 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) if (transport->unix_user_function != NULL) { - /* FIXME we hold the connection lock here and should drop it */ - if (!(* transport->unix_user_function) (transport->connection, - auth_identity.uid, - transport->unix_user_data)) + dbus_bool_t allow; + DBusConnection *connection; + DBusAllowUnixUserFunction unix_user_function; + void *unix_user_data; + + /* Dropping the lock here probably isn't that safe. */ + + connection = transport->connection; + unix_user_function = transport->unix_user_function; + unix_user_data = transport->unix_user_data; + + _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME); + _dbus_connection_unlock (connection); + + allow = (* unix_user_function) (connection, + auth_identity.uid, + unix_user_data); + + _dbus_verbose ("lock %s post unix user function\n", _DBUS_FUNCTION_NAME); + _dbus_connection_lock (connection); + + if (allow) + { + _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid); + } + else { _dbus_verbose ("Client UID "DBUS_UID_FORMAT " was rejected, disconnecting\n", auth_identity.uid); _dbus_transport_disconnect (transport); + _dbus_connection_unref_unlocked (connection); return FALSE; } - else - { - _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid); - } } else { @@ -515,6 +544,7 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) " but our UID is "DBUS_UID_FORMAT", disconnecting\n", auth_identity.uid, our_identity.uid); _dbus_transport_disconnect (transport); + _dbus_connection_unref_unlocked (transport->connection); return FALSE; } else @@ -527,8 +557,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport) } transport->authenticated = maybe_authenticated; - - return transport->authenticated; + + _dbus_connection_unref_unlocked (transport->connection); + return maybe_authenticated; } } @@ -670,6 +701,8 @@ _dbus_transport_do_iteration (DBusTransport *transport, (* transport->vtable->do_iteration) (transport, flags, timeout_milliseconds); _dbus_transport_unref (transport); + + _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME); } static dbus_bool_t -- cgit