From 3eb4fa15ee4c9297cc77ce69fcfcd5d7192462f4 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 28 Dec 2017 09:44:35 +0100 Subject: [PATCH 1/5] ENOMEM checks: consistently check wl_array_add return --- types/wlr_data_device.c | 3 +++ types/wlr_primary_selection.c | 3 +++ types/wlr_seat.c | 5 +++++ types/wlr_xdg_shell_v6.c | 24 ++++++++++++++++++++---- xwayland/selection.c | 9 +++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/types/wlr_data_device.c b/types/wlr_data_device.c index af81d861..58893129 100644 --- a/types/wlr_data_device.c +++ b/types/wlr_data_device.c @@ -907,6 +907,9 @@ static void data_source_offer(struct wl_client *client, *p = strdup(mime_type); } if (!p || !*p){ + if (p) { + source->mime_types.size -= sizeof *p; + } wl_resource_post_no_memory(resource); } } diff --git a/types/wlr_primary_selection.c b/types/wlr_primary_selection.c index 8163d2e5..1228e94e 100644 --- a/types/wlr_primary_selection.c +++ b/types/wlr_primary_selection.c @@ -127,6 +127,9 @@ static void source_handle_offer(struct wl_client *client, *p = strdup(mime_type); } if (p == NULL || *p == NULL) { + if (p) { + source->mime_types.size -= sizeof(*p); + } wl_resource_post_no_memory(resource); } } diff --git a/types/wlr_seat.c b/types/wlr_seat.c index 156ac142..2a79f784 100644 --- a/types/wlr_seat.c +++ b/types/wlr_seat.c @@ -876,6 +876,11 @@ void wlr_seat_keyboard_enter(struct wlr_seat *seat, wl_array_init(&keys); for (size_t i = 0; i < keyboard->num_keycodes; ++i) { uint32_t *p = wl_array_add(&keys, sizeof(uint32_t)); + if (!p) { + wlr_log(L_ERROR, "Cannot allocate memory, skipping keycode: %d\n", + keyboard->keycodes[i]); + continue; + } *p = keyboard->keycodes[i]; } uint32_t serial = wl_display_next_serial(seat->display); diff --git a/types/wlr_xdg_shell_v6.c b/types/wlr_xdg_shell_v6.c index ead5a73e..87dac3ff 100644 --- a/types/wlr_xdg_shell_v6.c +++ b/types/wlr_xdg_shell_v6.c @@ -922,19 +922,35 @@ static void wlr_xdg_toplevel_v6_send_configure( wl_array_init(&states); if (surface->toplevel_state->pending.maximized) { s = wl_array_add(&states, sizeof(uint32_t)); - *s = ZXDG_TOPLEVEL_V6_STATE_MAXIMIZED; + if (!s) { + wlr_log(L_ERROR, "Could not allocate state for maximized xdg_toplevel"); + } else { + *s = ZXDG_TOPLEVEL_V6_STATE_MAXIMIZED; + } } if (surface->toplevel_state->pending.fullscreen) { s = wl_array_add(&states, sizeof(uint32_t)); - *s = ZXDG_TOPLEVEL_V6_STATE_FULLSCREEN; + if (!s) { + wlr_log(L_ERROR, "Could not allocate state for fullscreen xdg_toplevel"); + } else { + *s = ZXDG_TOPLEVEL_V6_STATE_FULLSCREEN; + } } if (surface->toplevel_state->pending.resizing) { s = wl_array_add(&states, sizeof(uint32_t)); - *s = ZXDG_TOPLEVEL_V6_STATE_RESIZING; + if (!s) { + wlr_log(L_ERROR, "Could not allocate state for resizing xdg_toplevel"); + } else { + *s = ZXDG_TOPLEVEL_V6_STATE_RESIZING; + } } if (surface->toplevel_state->pending.activated) { s = wl_array_add(&states, sizeof(uint32_t)); - *s = ZXDG_TOPLEVEL_V6_STATE_ACTIVATED; + if (!s) { + wlr_log(L_ERROR, "Could not allocate state for activated xdg_toplevel"); + } else { + *s = ZXDG_TOPLEVEL_V6_STATE_ACTIVATED; + } } uint32_t width = surface->toplevel_state->pending.width; diff --git a/xwayland/selection.c b/xwayland/selection.c index 0d7f1588..280df583 100644 --- a/xwayland/selection.c +++ b/xwayland/selection.c @@ -55,6 +55,15 @@ static int xwm_read_data_source(int fd, uint32_t mask, void *data) { int current = selection->source_data.size; if (selection->source_data.size < incr_chunk_size) { p = wl_array_add(&selection->source_data, incr_chunk_size); + if (!p){ + wlr_log(L_ERROR, "Could not allocate selection source_data to read into, throwing away some input"); + /* if we just return now, we'll just be called + * again right away - force read something. + * 1K on stack is probably fine? */ + char junk[1024]; + read(fd, junk, sizeof(junk)); + return 1; + } } else { p = (char *) selection->source_data.data + selection->source_data.size; } From e5dd98c7f54241cb5eb15ba16ef6cb276d24271a Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 28 Dec 2017 12:17:57 +0100 Subject: [PATCH 2/5] xwayland/selection: handle wl_array_add failure better Just abort and deregister instead of trying to throw some input out, which would have lead to inconsistent paste --- xwayland/selection.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/xwayland/selection.c b/xwayland/selection.c index 280df583..00e45182 100644 --- a/xwayland/selection.c +++ b/xwayland/selection.c @@ -56,13 +56,8 @@ static int xwm_read_data_source(int fd, uint32_t mask, void *data) { if (selection->source_data.size < incr_chunk_size) { p = wl_array_add(&selection->source_data, incr_chunk_size); if (!p){ - wlr_log(L_ERROR, "Could not allocate selection source_data to read into, throwing away some input"); - /* if we just return now, we'll just be called - * again right away - force read something. - * 1K on stack is probably fine? */ - char junk[1024]; - read(fd, junk, sizeof(junk)); - return 1; + wlr_log(L_ERROR, "Could not allocate selection source_data"); + goto error_out; } } else { p = (char *) selection->source_data.data + selection->source_data.size; @@ -73,11 +68,7 @@ static int xwm_read_data_source(int fd, uint32_t mask, void *data) { int len = read(fd, p, available); if (len == -1) { wlr_log(L_ERROR, "read error from data source: %m"); - xwm_selection_send_notify(selection, XCB_ATOM_NONE); - wl_event_source_remove(selection->property_source); - selection->property_source = NULL; - close(fd); - wl_array_release(&selection->source_data); + goto error_out; } wlr_log(L_DEBUG, "read %d (available %d, mask 0x%x) bytes: \"%.*s\"", @@ -149,6 +140,14 @@ static int xwm_read_data_source(int fd, uint32_t mask, void *data) { } return 1; + +error_out: + xwm_selection_send_notify(selection, XCB_ATOM_NONE); + wl_event_source_remove(selection->property_source); + selection->property_source = NULL; + close(fd); + wl_array_release(&selection->source_data); + return 0; } static void xwm_selection_source_send(struct wlr_xwm_selection *selection, From da3ef46daf6ce678ca9e12ef184d281009b9f91f Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 28 Dec 2017 16:08:45 +0100 Subject: [PATCH 3/5] xdg_toplevel send_configure: abort on ENOMEM instead of sending partial configure --- types/wlr_xdg_shell_v6.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/types/wlr_xdg_shell_v6.c b/types/wlr_xdg_shell_v6.c index 87dac3ff..213d1030 100644 --- a/types/wlr_xdg_shell_v6.c +++ b/types/wlr_xdg_shell_v6.c @@ -924,33 +924,33 @@ static void wlr_xdg_toplevel_v6_send_configure( s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for maximized xdg_toplevel"); - } else { - *s = ZXDG_TOPLEVEL_V6_STATE_MAXIMIZED; + goto out; } + *s = ZXDG_TOPLEVEL_V6_STATE_MAXIMIZED; } if (surface->toplevel_state->pending.fullscreen) { s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for fullscreen xdg_toplevel"); - } else { - *s = ZXDG_TOPLEVEL_V6_STATE_FULLSCREEN; + goto out; } + *s = ZXDG_TOPLEVEL_V6_STATE_FULLSCREEN; } if (surface->toplevel_state->pending.resizing) { s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for resizing xdg_toplevel"); - } else { - *s = ZXDG_TOPLEVEL_V6_STATE_RESIZING; + goto out; } + *s = ZXDG_TOPLEVEL_V6_STATE_RESIZING; } if (surface->toplevel_state->pending.activated) { s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for activated xdg_toplevel"); - } else { - *s = ZXDG_TOPLEVEL_V6_STATE_ACTIVATED; + goto out; } + *s = ZXDG_TOPLEVEL_V6_STATE_ACTIVATED; } uint32_t width = surface->toplevel_state->pending.width; @@ -964,6 +964,7 @@ static void wlr_xdg_toplevel_v6_send_configure( zxdg_toplevel_v6_send_configure(surface->toplevel_state->resource, width, height, &states); +out: wl_array_release(&states); } From 8e24aaa3c6959089f0459e03f01c7b1f638d6027 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 28 Dec 2017 16:28:19 +0100 Subject: [PATCH 4/5] style: fix sizeof() calls without parentheses --- backend/drm/util.c | 2 +- examples/screenshot.c | 2 +- types/wlr_data_device.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/drm/util.c b/backend/drm/util.c index 25256343..37d99aa8 100644 --- a/backend/drm/util.c +++ b/backend/drm/util.c @@ -223,7 +223,7 @@ static bool match_obj_(struct match_state *st, size_t skips, size_t score, size_ if (score > st->score || (score == st->score && replaced < st->replaced)) { st->score = score; st->replaced = replaced; - memcpy(st->best, st->res, sizeof st->best[0] * st->num_res); + memcpy(st->best, st->res, sizeof(st->best[0]) * st->num_res); if (st->score == st->num_objs && st->replaced == 0) { st->exit_early = true; diff --git a/examples/screenshot.c b/examples/screenshot.c index a887d1d7..66e5c75c 100644 --- a/examples/screenshot.c +++ b/examples/screenshot.c @@ -95,7 +95,7 @@ static void handle_global(void *data, struct wl_registry *registry, static struct screenshooter_output *output; if (strcmp(interface, "wl_output") == 0) { - output = calloc(1, sizeof *output); + output = calloc(1, sizeof(*output)); output->output = wl_registry_bind(registry, name, &wl_output_interface, 1); wl_list_insert(&output_list, &output->link); diff --git a/types/wlr_data_device.c b/types/wlr_data_device.c index 58893129..a9f54a0b 100644 --- a/types/wlr_data_device.c +++ b/types/wlr_data_device.c @@ -901,14 +901,14 @@ static void data_source_offer(struct wl_client *client, wl_resource_get_user_data(resource); char **p; - p = wl_array_add(&source->mime_types, sizeof *p); + p = wl_array_add(&source->mime_types, sizeof(*p)); if (p) { *p = strdup(mime_type); } if (!p || !*p){ if (p) { - source->mime_types.size -= sizeof *p; + source->mime_types.size -= sizeof(*p); } wl_resource_post_no_memory(resource); } From b0e440b5b1e1de0030947c68cdc34b0fd902c5e8 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Thu, 28 Dec 2017 16:33:08 +0100 Subject: [PATCH 5/5] xdg_toplevel send_configure: also post no_memory to resource on ENOMEM --- types/wlr_xdg_shell_v6.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/types/wlr_xdg_shell_v6.c b/types/wlr_xdg_shell_v6.c index 213d1030..1c46197e 100644 --- a/types/wlr_xdg_shell_v6.c +++ b/types/wlr_xdg_shell_v6.c @@ -924,7 +924,7 @@ static void wlr_xdg_toplevel_v6_send_configure( s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for maximized xdg_toplevel"); - goto out; + goto error_out; } *s = ZXDG_TOPLEVEL_V6_STATE_MAXIMIZED; } @@ -932,7 +932,7 @@ static void wlr_xdg_toplevel_v6_send_configure( s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for fullscreen xdg_toplevel"); - goto out; + goto error_out; } *s = ZXDG_TOPLEVEL_V6_STATE_FULLSCREEN; } @@ -940,7 +940,7 @@ static void wlr_xdg_toplevel_v6_send_configure( s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for resizing xdg_toplevel"); - goto out; + goto error_out; } *s = ZXDG_TOPLEVEL_V6_STATE_RESIZING; } @@ -948,7 +948,7 @@ static void wlr_xdg_toplevel_v6_send_configure( s = wl_array_add(&states, sizeof(uint32_t)); if (!s) { wlr_log(L_ERROR, "Could not allocate state for activated xdg_toplevel"); - goto out; + goto error_out; } *s = ZXDG_TOPLEVEL_V6_STATE_ACTIVATED; } @@ -964,8 +964,12 @@ static void wlr_xdg_toplevel_v6_send_configure( zxdg_toplevel_v6_send_configure(surface->toplevel_state->resource, width, height, &states); -out: wl_array_release(&states); + return; + +error_out: + wl_array_release(&states); + wl_resource_post_no_memory(surface->toplevel_state->resource); } static void wlr_xdg_surface_send_configure(void *user_data) {