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 d074d52902 ("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: d074d52902 ("connection: Dynamically resize connection buffers").
Fixes: #494
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>

src/wayland-client.c

@@ -1578,6 +1578,28 @@ queue_event(struct wl_display *display, int len)
 	id = p[0];
 	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.  If the user of
+	 * libwayland-client uses a level-triggered event loop,
+	 * this will cause the client to enter a loop that
+	 * consumes CPU.  To avoid this, immediately drop the
+	 * connection.  Since the maximum size of a message should
+	 * not depend on the max buffer size chosen by the client,
+	 * always compare the message size against the
+	 * limit enforced by libwayland 1.22 and below (4096),
+	 * rather than the actual value the client chose.
+	 */
+	if (size > WL_MAX_MESSAGE_SIZE) {
+		wl_log("Message length %u exceeds limit %d\n",
+		       size, WL_MAX_MESSAGE_SIZE);
+		errno = E2BIG;
+		return -1;
+	}
+
 	if (len < size)
 		return 0;
 

src/wayland-private.h

@@ -49,6 +49,9 @@
 #define WL_CLOSURE_MAX_ARGS 20
 #define WL_BUFFER_DEFAULT_SIZE_POT 12
 #define WL_BUFFER_DEFAULT_MAX_SIZE (1 << WL_BUFFER_DEFAULT_SIZE_POT)
+#if WL_BUFFER_DEFAULT_MAX_SIZE < WL_MAX_MESSAGE_SIZE
+# error default buffer cannot hold maximum-sized message
+#endif
 
 #define WL_DEBUG_COLOR_RESET "\e[0m"
 #define WL_DEBUG_COLOR_RED "\e[31m"

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;
 

src/wayland-util.h

@@ -90,6 +90,14 @@ extern "C" {
  */
 struct wl_object;
 
+/**
+ * The maximum size of a protocol message.
+ *
+ * If a message size exceeds this value, the connection will be dropped.
+ * Servers will send an invalid_method error before disconnecting.
+ */
+#define WL_MAX_MESSAGE_SIZE 4096
+
 /**
  * Protocol message signature
  *