From 3872a9362e3f943e0a4cac8a4aee6b05b15e0d45 Mon Sep 17 00:00:00 2001 From: Demi Marie Obenour Date: Wed, 14 Aug 2024 21:14:49 -0400 Subject: [PATCH 1/2] tests: Test that an overlong message is rejected If one runs the test case without the fix for d074d5290263 ("connection: Dynamically resize connection buffers"), the server spins and never processes the message. The test eventually times out. With the fix, a protocol error is delivered to the client and the test finishes immediately. The test covers both the default 4096-byte buffer size and the case where the buffer size is large enough for the message, but the message itself exceeds the 4096-byte limit. Both are rejected. The test relies on the definition of struct wl_proxy, so this definition is moved to a client-specific private header. The definition itself is not changed in any way. Signed-off-by: Demi Marie Obenour --- src/wayland-client-private.h | 83 ++++++++++++++++++++++++++++++++++++ src/wayland-client.c | 53 +---------------------- tests/connection-test.c | 45 +++++++++++++++++++ tests/test-compositor.c | 69 +++++++++++++++++++++++++++--- tests/test-compositor.h | 2 + 5 files changed, 195 insertions(+), 57 deletions(-) create mode 100644 src/wayland-client-private.h diff --git a/src/wayland-client-private.h b/src/wayland-client-private.h new file mode 100644 index 00000000..5bb9b918 --- /dev/null +++ b/src/wayland-client-private.h @@ -0,0 +1,83 @@ +/* + * Copyright © 2008-2012 Kristian Høgsberg + * Copyright © 2010-2012 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining + * a copy of this software and associated documentation files (the + * "Software"), to deal in the Software without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +/* Struct definintions shared by the client code and test suite. */ + +/** \cond */ + +struct wl_event_queue { + struct wl_list event_list; + struct wl_list proxy_list; /**< struct wl_proxy::queue_link */ + struct wl_display *display; + char *name; +}; + +struct wl_proxy { + struct wl_object object; + struct wl_display *display; + struct wl_event_queue *queue; + uint32_t flags; + int refcount; + void *user_data; + wl_dispatcher_func_t dispatcher; + uint32_t version; + const char * const *tag; + struct wl_list queue_link; /**< in struct wl_event_queue::proxy_list */ +}; + +struct wl_display { + struct wl_proxy proxy; + struct wl_connection *connection; + + /* errno of the last wl_display error */ + int last_error; + + /* When display gets an error event from some object, it stores + * information about it here, so that client can get this + * information afterwards */ + struct { + /* Code of the error. It can be compared to + * the interface's errors enumeration. */ + uint32_t code; + /* interface (protocol) in which the error occurred */ + const struct wl_interface *interface; + /* id of the proxy that caused the error. There's no warranty + * that the proxy is still valid. It's up to client how it will + * use it */ + uint32_t id; + } protocol_error; + int fd; + struct wl_map objects; + struct wl_event_queue display_queue; + struct wl_event_queue default_queue; + pthread_mutex_t mutex; + + int reader_count; + uint32_t read_serial; + pthread_cond_t reader_cond; +}; + +/** \endcond */ diff --git a/src/wayland-client.c b/src/wayland-client.c index c8633046..b0b64748 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -46,6 +46,7 @@ #include "wayland-client.h" #include "wayland-private.h" #include "timespec-util.h" +#include "wayland-client-private.h" /** \cond */ @@ -60,58 +61,6 @@ struct wl_zombie { int *fd_count; }; -struct wl_proxy { - struct wl_object object; - struct wl_display *display; - struct wl_event_queue *queue; - uint32_t flags; - int refcount; - void *user_data; - wl_dispatcher_func_t dispatcher; - uint32_t version; - const char * const *tag; - struct wl_list queue_link; /**< in struct wl_event_queue::proxy_list */ -}; - -struct wl_event_queue { - struct wl_list event_list; - struct wl_list proxy_list; /**< struct wl_proxy::queue_link */ - struct wl_display *display; - char *name; -}; - -struct wl_display { - struct wl_proxy proxy; - struct wl_connection *connection; - - /* errno of the last wl_display error */ - int last_error; - - /* When display gets an error event from some object, it stores - * information about it here, so that client can get this - * information afterwards */ - struct { - /* Code of the error. It can be compared to - * the interface's errors enumeration. */ - uint32_t code; - /* interface (protocol) in which the error occurred */ - const struct wl_interface *interface; - /* id of the proxy that caused the error. There's no warranty - * that the proxy is still valid. It's up to client how it will - * use it */ - uint32_t id; - } protocol_error; - int fd; - struct wl_map objects; - struct wl_event_queue display_queue; - struct wl_event_queue default_queue; - pthread_mutex_t mutex; - - int reader_count; - uint32_t read_serial; - pthread_cond_t reader_cond; -}; - /** \endcond */ static int debug_client = 0; diff --git a/tests/connection-test.c b/tests/connection-test.c index aed97a0a..3a9e28e2 100644 --- a/tests/connection-test.c +++ b/tests/connection-test.c @@ -807,6 +807,51 @@ leak_after_error(void) free(c); } +static void +too_long_message(void *arg) +{ +#define TOO_LONG ((size_t)(4096 - 12 + 1)) + void *buf = malloc(TOO_LONG); + assert(buf); + memset(buf, 0, TOO_LONG); + struct client *c = client_connect(); + struct wl_array arr = { TOO_LONG - 1, TOO_LONG, buf }; + set_buffer_size(c, *(uint32_t *)arg); + long_request(c, &arr); + wl_display_roundtrip(c->wl_display); + assert(wl_display_get_error(c->wl_display) == 0); + arr.size += 1; + long_request(c, &arr); + wl_display_dispatch(c->wl_display); + assert(wl_display_get_error(c->wl_display) == EINVAL); + wl_proxy_destroy((struct wl_proxy *) c->tc); + wl_display_disconnect(c->wl_display); + free(c); + free(buf); +} + +TEST(overlong_message_small_buffer) +{ + struct display *d = display_create(); + + uint32_t size = 4096; + client_create(d, too_long_message, &size); + display_run(d); + + display_destroy(d); +} + +TEST(overlong_message_long_buffer) +{ + struct display *d = display_create(); + + uint32_t size = 8192; + client_create(d, too_long_message, &size); + display_run(d); + + display_destroy(d); +} + TEST(closure_leaks_after_error) { struct display *d = display_create(); diff --git a/tests/test-compositor.c b/tests/test-compositor.c index 8ec0631b..fc3058e1 100644 --- a/tests/test-compositor.c +++ b/tests/test-compositor.c @@ -34,8 +34,8 @@ #include #include #include - -#define WL_HIDE_DEPRECATED +#include "wayland-private.h" +#include "wayland-client-private.h" #include "test-runner.h" #include "test-compositor.h" @@ -49,6 +49,8 @@ static const struct wl_message tc_requests[] = { /* this request serves as a barrier for synchronizing*/ { "stop_display", "u", NULL }, { "noop", "", NULL }, + { "long_request", "a", NULL }, + { "set_buffer_size", "u", NULL }, }; static const struct wl_message tc_events[] = { @@ -57,7 +59,7 @@ static const struct wl_message tc_events[] = { const struct wl_interface test_compositor_interface = { "test", 1, - 2, tc_requests, + 4, tc_requests, 1, tc_events }; @@ -67,6 +69,12 @@ struct test_compositor_interface { uint32_t num); void (*noop)(struct wl_client *client, struct wl_resource *resource); + void (*handle_long_request)(struct wl_client *client, + struct wl_resource *resource, + struct wl_array *array); + void (*handle_set_buffer_size)(struct wl_client *client, + struct wl_resource *resource, + uint32_t buffer_size); }; struct test_compositor_listener { @@ -76,11 +84,13 @@ struct test_compositor_listener { enum { STOP_DISPLAY = 0, - TEST_NOOP = 1 + TEST_NOOP = 1, + LONG_REQUEST = 2, + SET_BUFFER_SIZE = 3, }; enum { - DISPLAY_RESUMED = 0 + DISPLAY_RESUMED = 0, }; /* Since tests can run parallelly, we need unique socket names @@ -338,9 +348,29 @@ handle_noop(struct wl_client *client, struct wl_resource *resource) (void)resource; } +static void +handle_long_request(struct wl_client *client, struct wl_resource *resource, struct wl_array *array) +{ + (void)client; + (void)resource; + (void)array; + /* This request should be rejected before the handler is called. */ + assert(array->size <= 4096 - 12 && + "overlong message not rejected sooner"); +} + +static void +handle_set_buffer_size(struct wl_client *client, struct wl_resource *resource, uint32_t buffer_size) +{ + (void)resource; + wl_client_set_max_buffer_size(client, buffer_size); +} + static const struct test_compositor_interface tc_implementation = { handle_stop_display, handle_noop, + handle_long_request, + handle_set_buffer_size, }; static void @@ -583,3 +613,32 @@ noop_request(struct client *c) { wl_proxy_marshal((struct wl_proxy *) c->tc, TEST_NOOP); } + +void +set_buffer_size(struct client *c, uint32_t size) +{ + wl_proxy_marshal((struct wl_proxy *) c->tc, SET_BUFFER_SIZE, size); +} + +void +long_request(struct client *c, struct wl_array *array) +{ + /* 12 is 8 bytes for the header and 4 bytes for the array length. */ + + /* Ensure that the array can validly be encoded in a message. */ + assert(array->size <= (UINT16_MAX & ~UINT16_C(3)) - 12 && "overlong wl_array"); + + struct wl_proxy *proxy = (struct wl_proxy *)c->tc; + if (array->size > WL_MAX_MESSAGE_SIZE - 12) { + size_t size = 12 + ((array->size + 3) & ~3); + uint32_t buf[2]; + + /* The server will post an error after getting + * the header, so don't send the request body. */ + buf[0] = wl_proxy_get_id(proxy); + buf[1] = size << 16 | LONG_REQUEST; + assert(wl_connection_write(proxy->display->connection, buf, sizeof(buf)) == 0); + } else { + wl_proxy_marshal(proxy, LONG_REQUEST, array); + } +} diff --git a/tests/test-compositor.h b/tests/test-compositor.h index 662a81c3..09f543ae 100644 --- a/tests/test-compositor.h +++ b/tests/test-compositor.h @@ -75,6 +75,8 @@ struct client *client_connect(void); void client_disconnect(struct client *); int stop_display(struct client *, int); void noop_request(struct client *); +void long_request(struct client *c, struct wl_array *a); +void set_buffer_size(struct client *c, uint32_t size); /** * Usual workflow: From d81525a235e48cc5de3e4005a16ddb1fbdfd9d7c Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Wed, 2 Jul 2025 12:15:33 +0200 Subject: [PATCH 2/2] client: add wl_display_dispatch_pending_single As well as wl_display_dispatch_queue_pending_single. The motivation is writing libwayland bindings for a dynamic language with exceptions/non-local returns. Since it is invalid for a wl_dispatcher_func_t callback provided to libwayland to not return, there is no way to prevent dispatching of further events in the case of an exception in the dynamic language event handler. Furthermore, since creating/destroying Wayland objects in an event handler affects the dispatching of subsequent events by libwayland, it is not possible to collect Wayland events in a queue outside libwayland and dispatch them one-by-one after wl_display_dispatch_pending() returns. Adding libwayland API to dispatch at most one pending event solves this problem cleanly. The bindings can have libwayland dispatch a single event, wait for wl_display_dispatch_pending_single() to return, run the dynamic language event handler (which may longjmp away), and continue the loop for as long as there are more events to dispatch. References: https://codeberg.org/ifreund/janet-wayland Signed-off-by: Isaac Freund --- src/wayland-client-core.h | 7 ++++ src/wayland-client.c | 75 +++++++++++++++++++++++++++++++++++++++ tests/display-test.c | 69 +++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+) diff --git a/src/wayland-client-core.h b/src/wayland-client-core.h index 970e6254..e0523e49 100644 --- a/src/wayland-client-core.h +++ b/src/wayland-client-core.h @@ -268,9 +268,16 @@ int wl_display_dispatch_queue_pending(struct wl_display *display, struct wl_event_queue *queue); +int +wl_display_dispatch_queue_pending_single(struct wl_display *display, + struct wl_event_queue *queue); + int wl_display_dispatch_pending(struct wl_display *display); +int +wl_display_dispatch_pending_single(struct wl_display *display); + int wl_display_get_error(struct wl_display *display); diff --git a/src/wayland-client.c b/src/wayland-client.c index c0b361f0..ed686b5c 100644 --- a/src/wayland-client.c +++ b/src/wayland-client.c @@ -1882,6 +1882,34 @@ err: return -1; } + +static int +dispatch_queue_single(struct wl_display *display, struct wl_event_queue *queue) +{ + if (display->last_error) + goto err; + + while (!wl_list_empty(&display->display_queue.event_list)) { + dispatch_event(display, &display->display_queue); + if (display->last_error) + goto err; + } + + if (!wl_list_empty(&queue->event_list)) { + dispatch_event(display, queue); + if (display->last_error) + goto err; + return 1; + } else { + return 0; + } + +err: + errno = display->last_error; + + return -1; +} + /** Prepare to read events from the display's file descriptor to a queue * * \param display The display context object @@ -2212,6 +2240,34 @@ wl_display_dispatch_queue_pending(struct wl_display *display, return ret; } +/** Dispatch at most one pending event in an event queue + * + * \param display The display context object + * \param queue The event queue to dispatch + * \return The number of dispatched events (0 or 1) on success or -1 on failure + * + * Dispatch at most one pending event for objects assigned to the given + * event queue. On failure -1 is returned and errno set appropriately. + * If there are no events queued, this function returns immediately. + * + * \memberof wl_display + * \since 1.25.0 + */ +WL_EXPORT int +wl_display_dispatch_queue_pending_single(struct wl_display *display, + struct wl_event_queue *queue) +{ + int ret; + + pthread_mutex_lock(&display->mutex); + + ret = dispatch_queue_single(display, queue); + + pthread_mutex_unlock(&display->mutex); + + return ret; +} + /** Process incoming events * * \param display The display context object @@ -2272,6 +2328,25 @@ wl_display_dispatch_pending(struct wl_display *display) &display->default_queue); } +/** Dispatch at most one pending event in the default event queue. + * + * \param display The display context object + * \return The number of dispatched events (0 or 1) on success or -1 on failure + * + * Dispatch at most one pending event for objects assigned to the default + * event queue. On failure -1 is returned and errno set appropriately. + * If there are no events queued, this function returns immediately. + * + * \memberof wl_display + * \since 1.25.0 + */ +WL_EXPORT int +wl_display_dispatch_pending_single(struct wl_display *display) +{ + return wl_display_dispatch_queue_pending_single(display, + &display->default_queue); +} + /** Retrieve the last error that occurred on a display * * \param display The display context object diff --git a/tests/display-test.c b/tests/display-test.c index 89606c73..fe78b521 100644 --- a/tests/display-test.c +++ b/tests/display-test.c @@ -1695,6 +1695,75 @@ TEST(global_remove) display_destroy(d); } +static void +dispatch_single_read_events(struct wl_display *d) +{ + if (wl_display_prepare_read(d) < 0) { + return; + } + + int ret = 0; + do { + ret = wl_display_flush(d); + } while (ret < 0 && (errno == EINTR || errno == EAGAIN)); + assert(ret >= 0); + + struct pollfd pfd[1]; + pfd[0].fd = wl_display_get_fd(d); + pfd[0].events = POLLIN; + + do { + ret = poll(pfd, 1, -1); + } while (ret < 0 && errno == EINTR); + assert(ret > 0); + + wl_display_read_events(d); +} + +static void +dispatch_single_client(void) +{ + struct client *c = client_connect(); + + assert(wl_display_dispatch_pending_single(c->wl_display) == 0); + + struct wl_registry *registry = wl_display_get_registry(c->wl_display); + + dispatch_single_read_events(c->wl_display); + + // [1815110.061] {Default Queue} wl_registry#3.global(1, "test", 1) + assert(wl_display_dispatch_pending_single(c->wl_display) == 1); + + dispatch_single_read_events(c->wl_display); + + // [1815110.067] {Default Queue} wl_registry#3.global(2, "wl_seat", 1) + assert(wl_display_dispatch_pending_single(c->wl_display) == 1); + + // No more events + assert(wl_display_dispatch_pending_single(c->wl_display) == 0); + + wl_registry_destroy(registry); + + client_disconnect(c); +} + +TEST(dispatch_single) +{ + struct display *d = display_create(); + + struct wl_global *global = wl_global_create(d->wl_display, + &wl_seat_interface, + 1, d, bind_seat); + + client_create_noarg(d, dispatch_single_client); + + display_run(d); + + wl_global_destroy(global); + + display_destroy(d); +} + static void terminate_display(void *arg) {