From 78a84bfedbfca2cbaa095cf3da0ce491b13a6b0f Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Tue, 24 Jun 2014 17:57:14 +0100 Subject: [PATCH] Handle ETOOMANYREFS https://bugs.freedesktop.org/show_bug.cgi?id=80163 --- dbus/dbus-connection-internal.h | 3 ++- dbus/dbus-connection.c | 60 ++++++++++++++++++++++++++++++++++++++--- dbus/dbus-connection.h | 16 +++++++++++ dbus/dbus-protocol.h | 4 +++ dbus/dbus-sysdeps.c | 10 +++++++ dbus/dbus-sysdeps.h | 1 + dbus/dbus-transport-socket.c | 43 +++++++++++++++++++++++++++-- 7 files changed, 130 insertions(+), 7 deletions(-) diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index 2842f2f..2c7bc73 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -53,7 +53,8 @@ void _dbus_connection_queue_received_message_link (DBusConnection dbus_bool_t _dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection); DBusMessage* _dbus_connection_get_message_to_send (DBusConnection *connection); void _dbus_connection_message_sent_unlocked (DBusConnection *connection, - DBusMessage *message); + DBusMessage *message, + DBusError *error); dbus_bool_t _dbus_connection_add_watch_unlocked (DBusConnection *connection, DBusWatch *watch); void _dbus_connection_remove_watch_unlocked (DBusConnection *connection, diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index f0b6871..0c8b314 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -299,6 +299,10 @@ struct DBusConnection void *dispatch_status_data; /**< Application data for dispatch_status_function */ DBusFreeFunction free_dispatch_status_data; /**< free dispatch_status_data */ + DBusUnableToSendFunction unable_to_send_function; /**< Function to notify failure to send a message */ + void *unable_to_send_data; /**< Application data for unable_to_send_function */ + DBusFreeFunction free_unable_to_send_data; /**< free unable_to_send_data */ + DBusDispatchStatus last_dispatch_status; /**< The last dispatch status we reported to the application. */ DBusObjectTree *objects; /**< Object path handlers registered with this connection */ @@ -614,16 +618,18 @@ _dbus_connection_get_message_to_send (DBusConnection *connection) } /** - * Notifies the connection that a message has been sent, so the + * Notifies an attempt to send the message on the connection, so the * message can be removed from the outgoing queue. * Called with the connection lock held. * * @param connection the connection. * @param message the message that was sent. + * @param error error when the message failed to be sent, or NULL. */ void _dbus_connection_message_sent_unlocked (DBusConnection *connection, - DBusMessage *message) + DBusMessage *message, + DBusError *error) { DBusList *link; @@ -659,6 +665,15 @@ _dbus_connection_message_sent_unlocked (DBusConnection *connection, dbus_message_get_signature (message), connection, connection->n_outgoing); + /* Notify if the message failed to be sent. It can happen if the socket was + * closed while we were writing, or with highly recursive fds */ + if (error) + { + if (connection->unable_to_send_function) + connection->unable_to_send_function (connection, message, error, + connection->unable_to_send_data); + } + /* It's OK that in principle we call the notify function, because for the * outgoing limit, there isn't one */ _dbus_message_remove_counter (message, connection->outgoing_counter); @@ -4181,13 +4196,17 @@ notify_disconnected_unlocked (DBusConnection *connection) if (connection->n_outgoing > 0) { DBusList *link; + DBusError error = DBUS_ERROR_INIT; _dbus_verbose ("Dropping %d outgoing messages since we're disconnected\n", connection->n_outgoing); - + + dbus_set_error_const (&error, DBUS_ERROR_CONNECTION_CLOSED, + "Dropping message because the connection is closed"); + while ((link = _dbus_list_get_last_link (&connection->outgoing_messages))) { - _dbus_connection_message_sent_unlocked (connection, link->data); + _dbus_connection_message_sent_unlocked (connection, link->data, &error); } } } @@ -5587,6 +5606,39 @@ dbus_connection_remove_filter (DBusConnection *connection, } /** + * Set a callback to be called when a message couldn't be send + * + * @param connection the connection + * @param function the handler + * @param user_data user data for the handler + * + */ +void +dbus_connection_set_unable_to_send_function (DBusConnection *connection, + DBusUnableToSendFunction function, + void *user_data, + DBusFreeFunction free_data_function) +{ + void *old_data; + DBusFreeFunction old_free_data; + + _dbus_return_if_fail (connection != NULL); + + CONNECTION_LOCK (connection); + old_data = connection->unable_to_send_data; + old_free_data = connection->free_unable_to_send_data; + + connection->unable_to_send_function = function; + connection->unable_to_send_data = user_data; + connection->free_unable_to_send_data = free_data_function; + + CONNECTION_UNLOCK (connection); + + if (old_free_data != NULL) + (* old_free_data) (old_data); +} + +/** * Registers a handler for a given path or subsection in the object * hierarchy. The given vtable handles messages sent to exactly the * given path or also for paths bellow that, depending on fallback diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h index fe4d04e..9b1b5f2 100644 --- a/dbus/dbus-connection.h +++ b/dbus/dbus-connection.h @@ -169,6 +169,16 @@ typedef void (* DBusPendingCallNotifyFunction) (DBusPendingCall *pending, typedef DBusHandlerResult (* DBusHandleMessageFunction) (DBusConnection *connection, DBusMessage *message, void *user_data); + +/** + * Called when a message cannot be sent by the underlying transport. This is + * only for permanent errors such as ECONNRESET or ETOOMANYREFS + */ +typedef void (* DBusUnableToSendFunction) (DBusConnection *connection, + DBusMessage *message, + DBusError *error, + void *user_data); + DBUS_EXPORT DBusConnection* dbus_connection_open (const char *address, DBusError *error); @@ -288,6 +298,12 @@ DBUS_EXPORT void dbus_connection_set_route_peer_messages (DBusConnection *connection, dbus_bool_t value); +DBUS_EXPORT +void dbus_connection_set_unable_to_send_function (DBusConnection *connection, + DBusUnableToSendFunction function, + void *data, + DBusFreeFunction free_data_function); + /* Filters */ diff --git a/dbus/dbus-protocol.h b/dbus/dbus-protocol.h index 60605ab..7c13dcb 100644 --- a/dbus/dbus-protocol.h +++ b/dbus/dbus-protocol.h @@ -446,6 +446,10 @@ extern "C" { /** The message meta data does not match the payload. e.g. expected number of file descriptors were not sent over the socket this message was received on. */ #define DBUS_ERROR_INCONSISTENT_MESSAGE "org.freedesktop.DBus.Error.InconsistentMessage" +/** The message cannot be sent because of too highly recursive file descriptors passed. */ +#define DBUS_ERROR_CANNOT_SEND_MESSAGE "org.freedesktop.DBus.Error.CannotSendMessage" +/** The message cannot be sent because the connection was closed. */ +#define DBUS_ERROR_CONNECTION_CLOSED "org.freedesktop.DBus.Error.ConnectionClosed" /* XML introspection format */ diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index de3a18c..11d7c12 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -762,6 +762,16 @@ _dbus_get_is_errno_epipe (void) } /** + * See if errno is ETOOMANYREFS + * @returns #TRUE if errno == ETOOMANYREFS + */ +dbus_bool_t +_dbus_get_is_errno_toomanyrefs (void) +{ + return errno == ETOOMANYREFS; +} + +/** * Get error message from errno * @returns _dbus_strerror(errno) */ diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index e586946..4f069d3 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -384,6 +384,7 @@ dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (void); dbus_bool_t _dbus_get_is_errno_enomem (void); dbus_bool_t _dbus_get_is_errno_eintr (void); dbus_bool_t _dbus_get_is_errno_epipe (void); +dbus_bool_t _dbus_get_is_errno_toomanyrefs (void); const char* _dbus_strerror_from_errno (void); void _dbus_disable_sigpipe (void); diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c index 774f459..117517c 100644 --- a/dbus/dbus-transport-socket.c +++ b/dbus/dbus-transport-socket.c @@ -645,12 +645,51 @@ do_writing (DBusTransport *transport) { /* EINTR already handled for us */ - /* For some discussion of why we also ignore EPIPE here, see + /* If the other end closed the socket with close() or shutdown(), we + * receive EPIPE here but we must not close the socket yet: there + * might still be some data to read. See: * http://lists.freedesktop.org/archives/dbus/2008-March/009526.html */ if (_dbus_get_is_errno_eagain_or_ewouldblock () || _dbus_get_is_errno_epipe ()) goto out; + + /* Since Linux commit 25888e (from 2.6.37-rc4, Nov 2010), sendmsg() + * on Unix sockets returns -1 errno=ETOOMANYREFS when the passfd + * mechanism (SCM_RIGHTS) is used recursively with a recursion level + * of maximum 4. The kernel does not have an API to check whether + * the passed fds can be forwarded and it can change asynchronously. + * See: + * https://bugs.freedesktop.org/show_bug.cgi?id=80163 + */ + + else if (_dbus_get_is_errno_toomanyrefs ()) + { + DBusError error = DBUS_ERROR_INIT; + + /* We only send fds in the first byte of the message. + * ETOOMANYREFS cannot happen after. + */ + _dbus_assert (socket_transport->message_bytes_written == 0); + + _dbus_verbose (" discard message of %d bytes\n", + total_bytes_to_write); + + total += bytes_written; + + socket_transport->message_bytes_written = 0; + _dbus_string_set_length (&socket_transport->encoded_outgoing, 0); + _dbus_string_compact (&socket_transport->encoded_outgoing, 2048); + + dbus_set_error_const (&error, DBUS_ERROR_CANNOT_SEND_MESSAGE, + "Cannot send message with highly recursive fdpassing"); + + /* The message was not actually sent but it needs to be removed + * from the outgoing queue + */ + _dbus_connection_message_sent_unlocked (transport->connection, + message, &error); + } else { _dbus_verbose ("Error writing to remote app: %s\n", @@ -677,7 +716,7 @@ do_writing (DBusTransport *transport) _dbus_string_compact (&socket_transport->encoded_outgoing, 2048); _dbus_connection_message_sent_unlocked (transport->connection, - message); + message, NULL); } } } -- 1.8.5.3