From f0d455f088510bf8a79aaccb2c67fc2a926b5b1a Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 09:59:44 +0900 Subject: drm backend: overflow fixes These operations are done in 32-bit arithmetics before being casted to 64-bit, thus can overflow before the cast. Casting early fixes the issue. Found through static analysis --- backend/drm/atomic.c | 4 ++-- backend/drm/drm.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index acc56e65..41b6424c 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -83,8 +83,8 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_plane *plane, // The src_* properties are in 16.16 fixed point atomic_add(atom, id, props->src_x, 0); atomic_add(atom, id, props->src_y, 0); - atomic_add(atom, id, props->src_w, plane->surf.width << 16); - atomic_add(atom, id, props->src_h, plane->surf.height << 16); + atomic_add(atom, id, props->src_w, (uint64_t)plane->surf.width << 16); + atomic_add(atom, id, props->src_h, (uint64_t)plane->surf.height << 16); atomic_add(atom, id, props->crtc_w, plane->surf.width); atomic_add(atom, id, props->crtc_h, plane->surf.height); atomic_add(atom, id, props->fb_id, fb_id); diff --git a/backend/drm/drm.c b/backend/drm/drm.c index c5db480e..f4a971a2 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -973,7 +973,7 @@ int handle_drm_event(int fd, uint32_t mask, void *data) { } void restore_drm_outputs(struct wlr_drm_backend *drm) { - uint64_t to_close = (1 << wl_list_length(&drm->outputs)) - 1; + uint64_t to_close = (1L << wl_list_length(&drm->outputs)) - 1; struct wlr_drm_connector *conn; wl_list_for_each(conn, &drm->outputs, link) { -- cgit v1.2.3 From 4f7b1382d4ceb1ed308563809d485ea6c047f077 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:03:26 +0900 Subject: wayland backend seat: fix NULL output check The test was done after dereferencing output in pointer_handle_enter, just move it up one line. No reason pointer_handle_leave would not need the check if enter needs it, add it there. Found through static analysis. --- backend/wayland/wl_seat.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/backend/wayland/wl_seat.c b/backend/wayland/wl_seat.c index 8ed61409..d5001a51 100644 --- a/backend/wayland/wl_seat.c +++ b/backend/wayland/wl_seat.c @@ -38,10 +38,8 @@ static void pointer_handle_enter(void *data, struct wl_pointer *wl_pointer, } struct wlr_wl_output *output = wl_surface_get_user_data(surface); + assert(output); struct wlr_wl_pointer *pointer = output_get_pointer(output); - if (output == NULL) { - return; - } output->enter_serial = serial; backend->current_pointer = pointer; @@ -56,6 +54,7 @@ static void pointer_handle_leave(void *data, struct wl_pointer *wl_pointer, } struct wlr_wl_output *output = wl_surface_get_user_data(surface); + assert(output); output->enter_serial = 0; if (backend->current_pointer == NULL || -- cgit v1.2.3 From bcc2c64c1e1a4562699a94deb6f9d57e1e072ed8 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:17:36 +0900 Subject: x11 backend init: fix leak on failed XOpenDisplay Found through static analysis --- backend/x11/backend.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/backend/x11/backend.c b/backend/x11/backend.c index d4793b9c..e35cbed7 100644 --- a/backend/x11/backend.c +++ b/backend/x11/backend.c @@ -245,13 +245,13 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display, x11->xlib_conn = XOpenDisplay(x11_display); if (!x11->xlib_conn) { wlr_log(L_ERROR, "Failed to open X connection"); - return NULL; + goto error_x11; } x11->xcb_conn = XGetXCBConnection(x11->xlib_conn); if (!x11->xcb_conn || xcb_connection_has_error(x11->xcb_conn)) { wlr_log(L_ERROR, "Failed to open xcb connection"); - goto error_x11; + goto error_display; } XSetEventQueueOwner(x11->xlib_conn, XCBOwnsEventQueue); @@ -262,7 +262,7 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display, x11->event_source = wl_event_loop_add_fd(ev, fd, events, x11_event, x11); if (!x11->event_source) { wlr_log(L_ERROR, "Could not create event source"); - goto error_x11; + goto error_display; } x11->screen = xcb_setup_roots_iterator(xcb_get_setup(x11->xcb_conn)).data; @@ -291,8 +291,9 @@ struct wlr_backend *wlr_x11_backend_create(struct wl_display *display, error_event: wl_event_source_remove(x11->event_source); -error_x11: +error_display: XCloseDisplay(x11->xlib_conn); +error_x11: free(x11); return NULL; } -- cgit v1.2.3 From 1e17f4deb6e73880fe135662a483a4fb0af690c7 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:18:58 +0900 Subject: rootston: fix leak in handle_layer_shell_surface Found through static analysis --- rootston/layer_shell.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rootston/layer_shell.c b/rootston/layer_shell.c index db0aeb59..2adf11a5 100644 --- a/rootston/layer_shell.c +++ b/rootston/layer_shell.c @@ -381,12 +381,6 @@ void handle_layer_shell_surface(struct wl_listener *listener, void *data) { layer_surface->client_pending.margin.bottom, layer_surface->client_pending.margin.left); - struct roots_layer_surface *roots_surface = - calloc(1, sizeof(struct roots_layer_surface)); - if (!roots_surface) { - return; - } - if (!layer_surface->output) { struct roots_input *input = desktop->server->input; struct roots_seat *seat = input_last_active_seat(input); @@ -409,6 +403,12 @@ void handle_layer_shell_surface(struct wl_listener *listener, void *data) { } } + struct roots_layer_surface *roots_surface = + calloc(1, sizeof(struct roots_layer_surface)); + if (!roots_surface) { + return; + } + roots_surface->surface_commit.notify = handle_surface_commit; wl_signal_add(&layer_surface->surface->events.commit, &roots_surface->surface_commit); -- cgit v1.2.3 From 266898ca1f1eeabaaff3f951a17e612147153ce5 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:28:41 +0900 Subject: direct session backend: fix closing -1 on error Found through static analysis --- backend/session/direct-ipc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/session/direct-ipc.c b/backend/session/direct-ipc.c index f8ba07f7..2dd777c8 100644 --- a/backend/session/direct-ipc.c +++ b/backend/session/direct-ipc.c @@ -159,7 +159,9 @@ static void communicate(int sock) { } error: send_msg(sock, ret ? -1 : fd, &ret, sizeof(ret)); - close(fd); + if (fd >= 0) { + close(fd); + } break; -- cgit v1.2.3 From efef54ccf56b298e935b8707c6808e7f4eebd030 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:41:10 +0900 Subject: wlr_keyboard: fix mmap leak + logic on close for keymap_fd mmap leak found through static analysis --- types/wlr_keyboard.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/types/wlr_keyboard.c b/types/wlr_keyboard.c index 0f0cb1cf..3afbe517 100644 --- a/types/wlr_keyboard.c +++ b/types/wlr_keyboard.c @@ -144,6 +144,7 @@ void wlr_keyboard_init(struct wlr_keyboard *kb, wl_signal_init(&kb->events.modifiers); wl_signal_init(&kb->events.keymap); wl_signal_init(&kb->events.repeat_info); + kb->keymap_fd = -1; // Sane defaults kb->repeat_info.rate = 25; @@ -156,7 +157,9 @@ void wlr_keyboard_destroy(struct wlr_keyboard *kb) { } xkb_state_unref(kb->xkb_state); xkb_keymap_unref(kb->keymap); - close(kb->keymap_fd); + if (kb->keymap_fd >= 0) { + close(kb->keymap_fd); + } if (kb->impl && kb->impl->destroy) { kb->impl->destroy(kb); } else { @@ -212,7 +215,7 @@ void wlr_keyboard_set_keymap(struct wlr_keyboard *kb, keymap_str = xkb_keymap_get_as_string(kb->keymap, XKB_KEYMAP_FORMAT_TEXT_V1); kb->keymap_size = strlen(keymap_str) + 1; - if (kb->keymap_fd) { + if (kb->keymap_fd >= 0) { close(kb->keymap_fd); } kb->keymap_fd = os_create_anonymous_file(kb->keymap_size); @@ -228,6 +231,7 @@ void wlr_keyboard_set_keymap(struct wlr_keyboard *kb, } strcpy(ptr, keymap_str); free(keymap_str); + munmap(ptr, kb->keymap_size); for (size_t i = 0; i < kb->num_keycodes; ++i) { xkb_keycode_t keycode = kb->keycodes[i] + 8; @@ -244,7 +248,10 @@ err: kb->xkb_state = NULL; xkb_keymap_unref(keymap); kb->keymap = NULL; - close(kb->keymap_fd); + if (kb->keymap_fd >= 0) { + close(kb->keymap_fd); + kb->keymap_fd = -1; + } free(keymap_str); } -- cgit v1.2.3 From 399de4d11bb36b71fdfb5f1a06e74cf7c4e6831c Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:55:33 +0900 Subject: util/create_tmpfile: set restrictive umask for these files Even if the file is removed right away, a race with someone using inotify is definitely possible, so play safe and restrict umask for our tmpfiles Found through static analysis. --- util/os-compatibility.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/util/os-compatibility.c b/util/os-compatibility.c index bd3067d2..38333605 100644 --- a/util/os-compatibility.c +++ b/util/os-compatibility.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include "util/os-compatibility.h" @@ -61,6 +62,7 @@ int create_tmpfile_cloexec(char *tmpname) { int fd; + mode_t prev_umask = umask(0066); #ifdef HAVE_MKOSTEMP fd = mkostemp(tmpname, O_CLOEXEC); if (fd >= 0) @@ -72,6 +74,7 @@ int create_tmpfile_cloexec(char *tmpname) unlink(tmpname); } #endif + umask(prev_umask); return fd; } -- cgit v1.2.3 From b3313b7f395983482d6efc60db039ef7a97cd6e0 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 10:58:22 +0900 Subject: wlr_output: fix scope for 'now' 'when' points to now that was defined in the if, so compiler could reuse that memory area by the time 'when' is called Found through static analysis. --- types/wlr_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/types/wlr_output.c b/types/wlr_output.c index 40332efd..9f13bef4 100644 --- a/types/wlr_output.c +++ b/types/wlr_output.c @@ -486,8 +486,8 @@ bool wlr_output_swap_buffers(struct wlr_output *output, struct timespec *when, pixman_region32_intersect(&render_damage, &render_damage, damage); } + struct timespec now; if (when == NULL) { - struct timespec now; clock_gettime(CLOCK_MONOTONIC, &now); when = &now; } -- cgit v1.2.3 From 4cc441248121493350eb50277d2815ec31e9ea59 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 11:04:17 +0900 Subject: wlr_renderer_destroy: fix renderer NULL check renderer is checked for NULL, but was dereferenced before that. Found through static analysis --- render/wlr_renderer.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/render/wlr_renderer.c b/render/wlr_renderer.c index 98c91132..00f1e411 100644 --- a/render/wlr_renderer.c +++ b/render/wlr_renderer.c @@ -25,9 +25,12 @@ void wlr_renderer_init(struct wlr_renderer *renderer, } void wlr_renderer_destroy(struct wlr_renderer *r) { + if (!r) { + return; + } wlr_signal_emit_safe(&r->events.destroy, r); - if (r && r->impl && r->impl->destroy) { + if (r->impl && r->impl->destroy) { r->impl->destroy(r); } else { free(r); -- cgit v1.2.3 From 1940c6bbd9c0a8867e40a36f27b69c7069213cf0 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 11:08:49 +0900 Subject: wayland backend: fix width/height == 0 check We cannot handle just one of the two being NULL later down the road (e.g. divide by zero in matrix projection code), just ignore any such configure request. Found through static analysis --- backend/wayland/output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/wayland/output.c b/backend/wayland/output.c index 42b41508..6aa59537 100644 --- a/backend/wayland/output.c +++ b/backend/wayland/output.c @@ -220,7 +220,7 @@ static void xdg_toplevel_handle_configure(void *data, struct zxdg_toplevel_v6 *x struct wlr_wl_output *output = data; assert(output && output->xdg_toplevel == xdg_toplevel); - if (width == 0 && height == 0) { + if (width == 0 || height == 0) { return; } // loop over states for maximized etc? -- cgit v1.2.3 From e5348ad7d374713d4e1a386849b15fb0d68de31c Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 11:11:06 +0900 Subject: backend autocreate: fix leak when WLR_BACKENDS is set Found through static analysis --- backend/backend.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/backend.c b/backend/backend.c index 07c171bc..07e05fca 100644 --- a/backend/backend.c +++ b/backend/backend.c @@ -203,6 +203,7 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, wlr_log(L_ERROR, "failed to start backend '%s'", name); wlr_backend_destroy(backend); wlr_session_destroy(session); + free(names); return NULL; } @@ -210,12 +211,14 @@ struct wlr_backend *wlr_backend_autocreate(struct wl_display *display, wlr_log(L_ERROR, "failed to add backend '%s'", name); wlr_backend_destroy(backend); wlr_session_destroy(session); + free(names); return NULL; } name = strtok_r(NULL, ",", &saveptr); } + free(names); return backend; } -- cgit v1.2.3 From 1fef1f88b2c28777d559f7240aafe0dc078cf9e5 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 11:17:30 +0900 Subject: export dmabuf manager_handle_capture_output: fix leak on error Found through static analysis --- types/wlr_export_dmabuf_v1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/types/wlr_export_dmabuf_v1.c b/types/wlr_export_dmabuf_v1.c index 68adda02..e68b0fef 100644 --- a/types/wlr_export_dmabuf_v1.c +++ b/types/wlr_export_dmabuf_v1.c @@ -84,6 +84,7 @@ static void manager_handle_capture_output(struct wl_client *client, &zwlr_export_dmabuf_frame_v1_interface, version, id); if (frame->resource == NULL) { wl_client_post_no_memory(client); + free(frame); return; } wl_resource_set_implementation(frame->resource, &frame_impl, frame, -- cgit v1.2.3 From 0c2a64df18f8740ab795fb2970d1954a8aac34b1 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sat, 30 Jun 2018 11:18:42 +0900 Subject: headless add_input_device: fix leak on error Found through static analysis --- backend/headless/input_device.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/backend/headless/input_device.c b/backend/headless/input_device.c index a1e18428..63d28e8e 100644 --- a/backend/headless/input_device.c +++ b/backend/headless/input_device.c @@ -39,7 +39,7 @@ struct wlr_input_device *wlr_headless_add_input_device( wlr_device->keyboard = calloc(1, sizeof(struct wlr_keyboard)); if (wlr_device->keyboard == NULL) { wlr_log(L_ERROR, "Unable to allocate wlr_keyboard"); - return NULL; + goto error; } wlr_keyboard_init(wlr_device->keyboard, NULL); break; @@ -47,7 +47,7 @@ struct wlr_input_device *wlr_headless_add_input_device( wlr_device->pointer = calloc(1, sizeof(struct wlr_pointer)); if (wlr_device->pointer == NULL) { wlr_log(L_ERROR, "Unable to allocate wlr_pointer"); - return NULL; + goto error; } wlr_pointer_init(wlr_device->pointer, NULL); break; @@ -55,7 +55,7 @@ struct wlr_input_device *wlr_headless_add_input_device( wlr_device->touch = calloc(1, sizeof(struct wlr_touch)); if (wlr_device->touch == NULL) { wlr_log(L_ERROR, "Unable to allocate wlr_touch"); - return NULL; + goto error; } wlr_touch_init(wlr_device->touch, NULL); break; @@ -63,7 +63,7 @@ struct wlr_input_device *wlr_headless_add_input_device( wlr_device->tablet_tool = calloc(1, sizeof(struct wlr_tablet_tool)); if (wlr_device->tablet_tool == NULL) { wlr_log(L_ERROR, "Unable to allocate wlr_tablet_tool"); - return NULL; + goto error; } wlr_tablet_tool_init(wlr_device->tablet_tool, NULL); break; @@ -71,7 +71,7 @@ struct wlr_input_device *wlr_headless_add_input_device( wlr_device->tablet_pad = calloc(1, sizeof(struct wlr_tablet_pad)); if (wlr_device->tablet_pad == NULL) { wlr_log(L_ERROR, "Unable to allocate wlr_tablet_pad"); - return NULL; + goto error; } wlr_tablet_pad_init(wlr_device->tablet_pad, NULL); break; @@ -84,4 +84,7 @@ struct wlr_input_device *wlr_headless_add_input_device( } return wlr_device; +error: + free(device); + return NULL; } -- cgit v1.2.3