connection: Do not busy-loop if a message exceeds the buffer size

If the length of a message exceeds the maximum length of the buffer, the
buffer size will reach its maximum value and stay there forever, with no
message ever being successfully processed.  Since libwayland uses
level-triggered epoll, this will cause the compositor to loop forever
and consume CPU time.  In libwayland 1.22 and below, there was an
explicit check that caused messages exceeding 4096 bytes to result in an
EOVERFLOW error, preventing the loop.  However, this check was removed
between d074d5290263 ("connection: Dynamically resize connection buffers").

To prevent this problem, always limit the size of messages to 4096 bytes.
Since the default and minimum buffer size is 4096 bytes, this ensures
that a single message will always fit in the buffer.  It would be
possible to allow larger messages if the buffer size was larger, but the
maximum size of a message should not depend on the buffer size chosen by
the compositor.

Rejecting messages that exceed 4092 bytes seems to have the advantage of
reserving 4 bits, not 3, in the size field for future use.  However,
message sizes in the range [0x0, 0x7] are invalid, so one can obtain a
fourth bit by negating the meaning of bit 12 if bits 0 through 11
(inclusive) are 0.  Allowing 4096-byte messages provides the far more
important advantage that regressions compared to 1.22 are impossible
and regressions compared to 1.23 are extremely unlikely.  The only case
where a regression is possible is:

- The receiving side is using libwayland 1.23.
- The sending side is either using libwayland 1.23 or is not using
  libwayland.
- The sender sends a message exceeding 4096 bytes.
- If the sender of the large message is the client, the server has
  increased the buffer size from the default value.

This combination is considered extremely unlikely, as libwayland 1.22
and below would disconnect upon receiving such a large message.
4096-byte messages, however, have always worked, so there was no reason
to avoid sending them.

Fixes: d074d5290263 ("connection: Dynamically resize connection buffers").
Fixes: #494
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>

1 files changed, 23 insertions, 0 deletions

diff --git a/src/wayland-server.c b/src/wayland-server.c
index 1566764..482743b 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -398,6 +398,29 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 		wl_connection_copy(connection, p, sizeof p);
 		opcode = p[1] & 0xffff;
 		size = p[1] >> 16;
+
+		/*
+		 * If the message is larger than the maximum size of the
+		 * connection buffer, the connection buffer will fill to
+		 * its max size and stay there, with no message ever
+		 * successfully being processed.  Since libwayland-server
+		 * uses level-triggered epoll, it will cause the server to
+		 * enter a loop that consumes CPU.  To avoid this,
+		 * immediately disconnect the client with a protocol
+		 * error.  Since the maximum size of a message should not
+		 * depend on the buffer size chosen by the compositor,
+		 * always compare the message size against the
+		 * limit enforced by libwayland 1.22 and below (4096),
+		 * rather than the actual value the compositor chose.
+		 */
+		if (size > WL_MAX_MESSAGE_SIZE) {
+			wl_resource_post_error(client->display_resource,
+			                       WL_DISPLAY_ERROR_INVALID_METHOD,
+			                       "message length %u exceeds %d",
+			                       size, WL_MAX_MESSAGE_SIZE);
+			break;
+		}
+
 		if (len < size)
 			break;