Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Commit

Permalink
Fix invalid uses of wl_array_for_each
Browse files Browse the repository at this point in the history
[1] and [2] have introduced new wl_array usage in wlroots, but
contains a mistake: wl_array_for_each iterates over pointers to
the wl_array entries, not over entries themselves.

Fix all wl_array_for_each call sites. Name the variables "ptr"
to avoid confusion.

Found via ASan:

    ==148752==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x602000214111 in thread T0
        #0 0x7f6ff2235f19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
        #1 0x7f6ff1c04004 in wlr_tablet_destroy ../subprojects/wlroots/types/wlr_tablet_tool.c:24
        #2 0x7f6ff1b8463c in wlr_input_device_destroy ../subprojects/wlroots/types/wlr_input_device.c:51
        #3 0x7f6ff1ab9941 in backend_destroy ../subprojects/wlroots/backend/wayland/backend.c:306
        #4 0x7f6ff1a68323 in wlr_backend_destroy ../subprojects/wlroots/backend/backend.c:57
        #5 0x7f6ff1ab36b4 in multi_backend_destroy ../subprojects/wlroots/backend/multi/backend.c:57
        #6 0x7f6ff1ab417c in handle_display_destroy ../subprojects/wlroots/backend/multi/backend.c:124
        #7 0x7f6ff106184e in wl_display_destroy (/usr/lib/libwayland-server.so.0+0x884e)
        #8 0x55cd1a77c9e5 in server_fini ../sway/server.c:218
        #9 0x55cd1a77893f in main ../sway/main.c:400
        #10 0x7f6ff04bdb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
        #11 0x55cd1a73a7ad in _start (/home/simon/src/sway/build/sway/sway+0x33a7ad)

    0x602000214111 is located 1 bytes inside of 16-byte region [0x602000214110,0x602000214120)
    freed by thread T0 here:
        #0 0x7f6ff2235f19 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
        #1 0x7f6ff1c04004 in wlr_tablet_destroy ../subprojects/wlroots/types/wlr_tablet_tool.c:24
        #2 0x7f6ff1b8463c in wlr_input_device_destroy ../subprojects/wlroots/types/wlr_input_device.c:51
        #3 0x7f6ff1ab9941 in backend_destroy ../subprojects/wlroots/backend/wayland/backend.c:306
        #4 0x7f6ff1a68323 in wlr_backend_destroy ../subprojects/wlroots/backend/backend.c:57
        #5 0x7f6ff1ab36b4 in multi_backend_destroy ../subprojects/wlroots/backend/multi/backend.c:57
        #6 0x7f6ff1ab417c in handle_display_destroy ../subprojects/wlroots/backend/multi/backend.c:124
        #7 0x7f6ff106184e in wl_display_destroy (/usr/lib/libwayland-server.so.0+0x884e)

    previously allocated by thread T0 here:
        #0 0x7f6ff2236279 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
        #1 0x7f6ff1066d03 in wl_array_add (/usr/lib/libwayland-server.so.0+0xdd03)

[1]: #3002
[2]: #3004
  • Loading branch information
emersion authored and bl4ckb0ne committed Jul 8, 2021
1 parent b934fba commit e035f2b
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 23 deletions.
8 changes: 4 additions & 4 deletions backend/libinput/backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@ static void backend_destroy(struct wlr_backend *wlr_backend) {
struct wlr_libinput_backend *backend =
get_libinput_backend_from_backend(wlr_backend);

struct wl_list *wlr_devices;
wl_array_for_each(wlr_devices, &backend->wlr_device_lists) {
struct wl_list **wlr_devices_ptr;
wl_array_for_each(wlr_devices_ptr, &backend->wlr_device_lists) {
struct wlr_input_device *wlr_dev, *next;
wl_list_for_each_safe(wlr_dev, next, wlr_devices, link) {
wl_list_for_each_safe(wlr_dev, next, *wlr_devices_ptr, link) {
wlr_input_device_destroy(wlr_dev);
}
free(wlr_devices);
free(*wlr_devices_ptr);
}

wlr_backend_finish(wlr_backend);
Expand Down
5 changes: 3 additions & 2 deletions backend/libinput/events.c
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,9 @@ static void handle_device_removed(struct wlr_libinput_backend *backend,
wlr_input_device_destroy(dev);
}
size_t i = 0;
struct wl_list *iter;
wl_array_for_each(iter, &backend->wlr_device_lists) {
struct wl_list **ptr;
wl_array_for_each(ptr, &backend->wlr_device_lists) {
struct wl_list *iter = *ptr;
if (iter == wlr_devices) {
array_remove_at(&backend->wlr_device_lists,
i * sizeof(struct wl_list *), sizeof(struct wl_list *));
Expand Down
11 changes: 6 additions & 5 deletions backend/libinput/tablet_tool.c
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,9 @@ static void destroy_tablet(struct wlr_tablet *wlr_tablet) {
struct wlr_libinput_tablet *tablet =
wl_container_of(wlr_tablet, tablet, wlr_tablet);

struct wlr_libinput_tablet_tool *tool;
wl_array_for_each(tool, &tablet->tools) {
struct wlr_libinput_tablet_tool **tool_ptr;
wl_array_for_each(tool_ptr, &tablet->tools) {
struct wlr_libinput_tablet_tool *tool = *tool_ptr;
if (--tool->pad_refs == 0) {
destroy_tool(tool);
}
Expand Down Expand Up @@ -151,9 +152,9 @@ static void ensure_tool_reference(struct wlr_libinput_tablet_tool *tool,
struct wlr_libinput_tablet *tablet =
wl_container_of(wlr_dev, tablet, wlr_tablet);

struct wlr_libinput_tablet_tool *iter;
wl_array_for_each(iter, &tablet->tools) {
if (iter == tool) { // We already have a ref
struct wlr_libinput_tablet_tool **tool_ptr;
wl_array_for_each(tool_ptr, &tablet->tools) {
if (*tool_ptr == tool) { // We already have a ref
// XXX: We *could* optimize the tool to the front of
// the list here, since we will probably get the next
// couple of events from the same tool.
Expand Down
6 changes: 3 additions & 3 deletions types/tablet_v2/wlr_tablet_v2_pad.c
Original file line number Diff line number Diff line change
Expand Up @@ -327,9 +327,9 @@ void add_tablet_pad_client(struct wlr_tablet_seat_client_v2 *seat,
zwp_tablet_pad_v2_send_buttons(client->resource, pad->wlr_pad->button_count);
}

const char *path;
wl_array_for_each(path, &pad->wlr_pad->paths) {
zwp_tablet_pad_v2_send_path(client->resource, path);
const char **path_ptr;
wl_array_for_each(path_ptr, &pad->wlr_pad->paths) {
zwp_tablet_pad_v2_send_path(client->resource, *path_ptr);
}

size_t i = 0;
Expand Down
6 changes: 3 additions & 3 deletions types/tablet_v2/wlr_tablet_v2_tablet.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ void add_tablet_client(struct wlr_tablet_seat_client_v2 *seat,
zwp_tablet_v2_send_id(client->resource,
tablet->wlr_device->vendor, tablet->wlr_device->product);

const char *path;
wl_array_for_each(path, &tablet->wlr_tablet->paths) {
zwp_tablet_v2_send_path(client->resource, path);
const char **path_ptr;
wl_array_for_each(path_ptr, &tablet->wlr_tablet->paths) {
zwp_tablet_v2_send_path(client->resource, *path_ptr);
}

zwp_tablet_v2_send_done(client->resource);
Expand Down
6 changes: 3 additions & 3 deletions types/wlr_tablet_pad.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ void wlr_tablet_pad_destroy(struct wlr_tablet_pad *pad) {
return;
}

char *path;
wl_array_for_each(path, &pad->paths) {
free(path);
char **path_ptr;
wl_array_for_each(path_ptr, &pad->paths) {
free(*path_ptr);
}
wl_array_release(&pad->paths);

Expand Down
6 changes: 3 additions & 3 deletions types/wlr_tablet_tool.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ void wlr_tablet_destroy(struct wlr_tablet *tablet) {
return;
}

char *path;
wl_array_for_each(path, &tablet->paths) {
free(path);
char **path_ptr;
wl_array_for_each(path_ptr, &tablet->paths) {
free(*path_ptr);
}
wl_array_release(&tablet->paths);

Expand Down

0 comments on commit e035f2b

Please sign in to comment.