From 6df392436a092a364c6afa64af7ef19305526563 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 1 Jul 2016 14:46:02 +0100 Subject: [PATCH] WiP: only read one message at a time if there are fds pending --- dbus/dbus-message-internal.h | 4 ++- dbus/dbus-message-util.c | 10 +++--- dbus/dbus-message.c | 86 ++++++++++++++++++++++++++++++++++++++++++-- dbus/dbus-transport-socket.c | 20 ++++++++--- dbus/dbus-transport.c | 8 +++-- 5 files changed, 113 insertions(+), 15 deletions(-) diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index 4bb4d8b..386bc50 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -73,7 +73,9 @@ void _dbus_message_loader_unref (DBusMessageLoader DBUS_PRIVATE_EXPORT void _dbus_message_loader_get_buffer (DBusMessageLoader *loader, - DBusString **buffer); + DBusString **buffer, + int *max_to_read, + dbus_bool_t *may_read_unix_fds); DBUS_PRIVATE_EXPORT void _dbus_message_loader_return_buffer (DBusMessageLoader *loader, DBusString *buffer); diff --git a/dbus/dbus-message-util.c b/dbus/dbus-message-util.c index a9d4a51..e226223 100644 --- a/dbus/dbus-message-util.c +++ b/dbus/dbus-message-util.c @@ -489,7 +489,7 @@ dbus_internal_do_not_use_try_message_data (const DBusString *data, { DBusString *buffer; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); _dbus_string_append_byte (buffer, _dbus_string_get_byte (data, i)); _dbus_message_loader_return_buffer (loader, buffer); @@ -508,7 +508,7 @@ dbus_internal_do_not_use_try_message_data (const DBusString *data, { DBusString *buffer; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); _dbus_string_copy (data, 0, buffer, _dbus_string_get_length (buffer)); _dbus_message_loader_return_buffer (loader, buffer); @@ -529,7 +529,7 @@ dbus_internal_do_not_use_try_message_data (const DBusString *data, { DBusString *buffer; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); _dbus_string_append_byte (buffer, _dbus_string_get_byte (data, i)); if ((i+1) < len) @@ -1497,7 +1497,7 @@ _dbus_message_test (const char *test_data_dir) { DBusString *buffer; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); _dbus_string_append_byte (buffer, data[i]); _dbus_message_loader_return_buffer (loader, buffer); } @@ -1508,7 +1508,7 @@ _dbus_message_test (const char *test_data_dir) { DBusString *buffer; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); _dbus_string_append_byte (buffer, data[i]); _dbus_message_loader_return_buffer (loader, buffer); } diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index e22fe51..44640fe 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -4033,13 +4033,95 @@ _dbus_message_loader_unref (DBusMessageLoader *loader) */ void _dbus_message_loader_get_buffer (DBusMessageLoader *loader, - DBusString **buffer) + DBusString **buffer, + int *max_to_read, + dbus_bool_t *may_read_fds) { _dbus_assert (!loader->buffer_outstanding); *buffer = &loader->data; loader->buffer_outstanding = TRUE; + + if (max_to_read != NULL) + { + int offset = 0; + int remain; + int byte_order; + int fields_array_len; + int header_len; + int body_len; + + *max_to_read = DBUS_MAXIMUM_MESSAGE_LENGTH; + *may_read_fds = TRUE; + + /* If we aren't holding onto any fds, we can read as much as we want + * (fast path). */ + if (loader->n_unix_fds == 0) + return; + + /* Slow path: we have a message with some fds in it. We don't want + * to start on the next message until this one is out of the way; + * otherwise a legitimate sender can keep us processing messages + * containing fds, until we disconnect it for having had fds pending + * for too long, a limit that is in place to stop malicious senders + * from setting up recursive fd-passing that takes up our quota and + * will never go away. */ + + remain = _dbus_string_get_length (&loader->data); + + while (remain > 0) + { + DBusValidity validity = DBUS_VALIDITY_UNKNOWN; + int needed; + + /* If 0 < remain < DBUS_MINIMUM_HEADER_SIZE, then we've had at + * least the first byte of a message, but we don't know how + * much more to read. Only read the rest of the + * DBUS_MINIMUM_HEADER_SIZE for now; then we'll know. */ + if (remain < DBUS_MINIMUM_HEADER_SIZE) + { + *max_to_read = DBUS_MINIMUM_HEADER_SIZE - remain; + *may_read_fds = FALSE; + return; + } + + if (!_dbus_header_have_message_untrusted (loader->max_message_size, + &validity, + &byte_order, + &fields_array_len, + &header_len, + &body_len, + &loader->data, + offset, + remain)) + { + /* If a message in the buffer is invalid, we're going to + * disconnect the sender anyway, so reading an arbitrary amount + * is fine. */ + if (validity != DBUS_VALID) + return; + + /* We have a partial message, with the + * DBUS_MINIMUM_HEADER_SIZE-byte fixed part of the header (which + * lets us work out how much more we need), but no more. Read + * the rest of the message. */ + needed = header_len + body_len; + _dbus_assert (needed > remain); + *max_to_read = needed - remain; + *may_read_fds = FALSE; + return; + } + + /* Skip over entire messages until we have less than a message + * remaining. */ + needed = header_len + body_len; + _dbus_assert (needed > DBUS_MINIMUM_HEADER_SIZE); + _dbus_assert (remain >= needed); + remain -= needed; + offset += needed; + } + } } /** @@ -4868,7 +4950,7 @@ dbus_message_demarshal (const char *str, if (loader == NULL) return NULL; - _dbus_message_loader_get_buffer (loader, &buffer); + _dbus_message_loader_get_buffer (loader, &buffer, NULL, NULL); if (!_dbus_string_append_len (buffer, str, len)) goto fail_oom; diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c index dce5c7d..5b8538e 100644 --- a/dbus/dbus-transport-socket.c +++ b/dbus/dbus-transport-socket.c @@ -794,7 +794,9 @@ do_reading (DBusTransport *transport) if (bytes_read > 0) { _dbus_message_loader_get_buffer (transport->loader, - &buffer); + &buffer, + NULL, + NULL); if (!_dbus_auth_decode_data (transport->auth, &socket_transport->encoded_incoming, @@ -817,11 +819,19 @@ do_reading (DBusTransport *transport) } else { + int max_to_read = DBUS_MAXIMUM_MESSAGE_LENGTH; + dbus_bool_t may_read_unix_fds = TRUE; + _dbus_message_loader_get_buffer (transport->loader, - &buffer); + &buffer, + &max_to_read, + &may_read_unix_fds); + + if (max_to_read > socket_transport->max_bytes_read_per_iteration) + max_to_read = socket_transport->max_bytes_read_per_iteration; #ifdef HAVE_UNIX_FD_PASSING - if (DBUS_TRANSPORT_CAN_SEND_UNIX_FD(transport)) + if (DBUS_TRANSPORT_CAN_SEND_UNIX_FD(transport) && may_read_unix_fds) { int *fds; unsigned int n_fds; @@ -836,7 +846,7 @@ do_reading (DBusTransport *transport) bytes_read = _dbus_read_socket_with_unix_fds(socket_transport->fd, buffer, - socket_transport->max_bytes_read_per_iteration, + max_to_read, fds, &n_fds); saved_errno = _dbus_save_socket_errno (); @@ -849,7 +859,7 @@ do_reading (DBusTransport *transport) #endif { bytes_read = _dbus_read_socket (socket_transport->fd, - buffer, socket_transport->max_bytes_read_per_iteration); + buffer, max_to_read); saved_errno = _dbus_save_socket_errno (); } diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index 31586b1..ca31aa3 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -1029,7 +1029,9 @@ recover_unused_bytes (DBusTransport *transport) } _dbus_message_loader_get_buffer (transport->loader, - &buffer); + &buffer, + NULL, + NULL); orig_len = _dbus_string_get_length (buffer); @@ -1059,7 +1061,9 @@ recover_unused_bytes (DBusTransport *transport) dbus_bool_t succeeded; _dbus_message_loader_get_buffer (transport->loader, - &buffer); + &buffer, + NULL, + NULL); orig_len = _dbus_string_get_length (buffer); -- 2.8.1