dev-libs/wayland: Fix some undefined behaviour
Upstream merge request:
https://gitlab.freedesktop.org/wayland/wayland/-/merge_requests/152
BUG=chromium:1227753
TEST=Ran wayland test suite under ubsan
Change-Id: I0d5587c7a50f63e0940304ef15071b5fb4fe1018
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/overlays/portage-stable/+/3016224
Tested-by: Fergus Dall <sidereal@google.com>
Auto-Submit: Fergus Dall <sidereal@google.com>
Reviewed-by: David Munro <davidmunro@google.com>
Commit-Queue: Fergus Dall <sidereal@google.com>
diff --git a/dev-libs/wayland/files/0003-util-Avoid-undefined-behaviour-in-for_each_helper.patch b/dev-libs/wayland/files/0003-util-Avoid-undefined-behaviour-in-for_each_helper.patch
new file mode 100644
index 0000000..ff2d677
--- /dev/null
+++ b/dev-libs/wayland/files/0003-util-Avoid-undefined-behaviour-in-for_each_helper.patch
@@ -0,0 +1,79 @@
+From 241f173f365be7e810e8713afe87f07bceb1c813 Mon Sep 17 00:00:00 2001
+From: Fergus Dall <sidereal@google.com>
+Date: Fri, 9 Jul 2021 17:52:01 +1000
+Subject: [PATCH 1/2] util: Avoid undefined behaviour in for_each_helper
+
+for_each_helper tries to calculate a one-past-the-end pointer for its
+wl_array input. This is fine when the array has one or more entries, but we
+initialize arrays by setting wl_array.data to NULL. Pointer arithmetic is
+only defined when both the pointer operand and the result point to the same
+allocation, or one-past-the-end of that allocation. As NULL points to no
+allocation, no pointer arithmetic can be performed on it, not even adding 0,
+even if the result is never dereferenced.
+
+This is caught by clang's ubsan from version 10.
+
+Many tests already hit this case, but I added an explicit test for iterating
+over an empty wl_map.
+
+Signed-off-by: Fergus Dall <sidereal@google.com>
+---
+ src/wayland-util.c | 9 +++++----
+ tests/map-test.c | 16 ++++++++++++++++
+ 2 files changed, 21 insertions(+), 4 deletions(-)
+
+diff --git a/src/wayland-util.c b/src/wayland-util.c
+index d5973bf..b502643 100644
+--- a/src/wayland-util.c
++++ b/src/wayland-util.c
+@@ -361,18 +361,19 @@ wl_map_lookup_flags(struct wl_map *map, uint32_t i)
+ static enum wl_iterator_result
+ for_each_helper(struct wl_array *entries, wl_iterator_func_t func, void *data)
+ {
+- union map_entry *start, *end, *p;
+ enum wl_iterator_result ret = WL_ITERATOR_CONTINUE;
+
+- start = entries->data;
+- end = (union map_entry *) ((char *) entries->data + entries->size);
++ union map_entry *start = (union map_entry*)entries->data;
++ size_t count = entries->size / sizeof(union map_entry);
+
+- for (p = start; p < end; p++)
++ for (size_t idx = 0; idx < count; idx++) {
++ union map_entry *p = start + idx;
+ if (p->data && !map_entry_is_free(*p)) {
+ ret = func(map_entry_get_data(*p), data, map_entry_get_flags(*p));
+ if (ret != WL_ITERATOR_CONTINUE)
+ break;
+ }
++ }
+
+ return ret;
+ }
+diff --git a/tests/map-test.c b/tests/map-test.c
+index 8ecc1aa..03568ea 100644
+--- a/tests/map-test.c
++++ b/tests/map-test.c
+@@ -119,3 +119,19 @@ TEST(map_flags)
+
+ wl_map_release(&map);
+ }
++
++static enum wl_iterator_result never_run(void *element, void *data, uint32_t flags)
++{
++ assert(0);
++}
++
++TEST(map_iter_empty)
++{
++ struct wl_map map;
++
++ wl_map_init(&map, WL_MAP_SERVER_SIDE);
++
++ wl_map_for_each(&map, never_run, NULL);
++
++ wl_map_release(&map);
++}
+--
+2.32.0.93.g670b81a890-goog
+
diff --git a/dev-libs/wayland/files/0004-server-Fix-undefined-behavior-in-wl_socket_init_for_.patch b/dev-libs/wayland/files/0004-server-Fix-undefined-behavior-in-wl_socket_init_for_.patch
new file mode 100644
index 0000000..95e13a9
--- /dev/null
+++ b/dev-libs/wayland/files/0004-server-Fix-undefined-behavior-in-wl_socket_init_for_.patch
@@ -0,0 +1,54 @@
+From 73f3b14b87742512cf535f07dddc22954d5d6677 Mon Sep 17 00:00:00 2001
+From: Fergus Dall <sidereal@google.com>
+Date: Fri, 9 Jul 2021 18:04:27 +1000
+Subject: [PATCH 2/2] server: Fix undefined behavior in
+ wl_socket_init_for_display_name
+
+This function constructs a socket path in sun_path using snprintf, which
+returns the amount of space that would have been used if the buffer was
+large enough. It then checks if this is larger then the actual buffer size
+and, if so, returns ENAMETOOLONG. This is correct.
+
+However, after calling snprintf and before checking that the length isn't too
+long, it tries to compute a pointer to the part of the path that matches the
+input name. It does this by adding the computed path length to the pointer to
+the start of the path buffer, which will take it to one-past the null
+terminator, and then walking backwards. If the path fits in the buffer, this
+will take it at most one-past-the-end of the allocation, which is allowed, but
+if the path is longer then the buffer then the pointer addition is undefined behavior.
+
+Fix this by moving the display name computation past the check that the path
+length is not too long.
+
+This is detected by the test socket_path_overflow_server_create under ubsan.
+
+Signed-off-by: Fergus Dall <sidereal@google.com>
+---
+ src/wayland-server.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/src/wayland-server.c b/src/wayland-server.c
+index d83bdec..d1db3bf 100644
+--- a/src/wayland-server.c
++++ b/src/wayland-server.c
+@@ -1501,8 +1501,6 @@ wl_socket_init_for_display_name(struct wl_socket *s, const char *name)
+ name_size = snprintf(s->addr.sun_path, sizeof s->addr.sun_path,
+ "%s%s%s", runtime_dir, separator, name) + 1;
+
+- s->display_name = (s->addr.sun_path + name_size - 1) - strlen(name);
+-
+ assert(name_size > 0);
+ if (name_size > (int)sizeof s->addr.sun_path) {
+ wl_log("error: socket path \"%s%s%s\" plus null terminator"
+@@ -1514,6 +1512,8 @@ wl_socket_init_for_display_name(struct wl_socket *s, const char *name)
+ return -1;
+ }
+
++ s->display_name = (s->addr.sun_path + name_size - 1) - strlen(name);
++
+ return 0;
+ }
+
+--
+2.32.0.93.g670b81a890-goog
+
diff --git a/dev-libs/wayland/wayland-1.19.0-r1.ebuild b/dev-libs/wayland/wayland-1.19.0-r2.ebuild
similarity index 90%
rename from dev-libs/wayland/wayland-1.19.0-r1.ebuild
rename to dev-libs/wayland/wayland-1.19.0-r2.ebuild
index ef33ea0..2bc194b 100644
--- a/dev-libs/wayland/wayland-1.19.0-r1.ebuild
+++ b/dev-libs/wayland/wayland-1.19.0-r2.ebuild
@@ -38,6 +38,8 @@
PATCHES=(
"${FILESDIR}"/0001-connection-test-Encode-size-in-message-headers-corre.patch
"${FILESDIR}"/0002-connection-Handle-non-nullable-strings-in-wl_connect.patch
+ "${FILESDIR}"/0003-util-Avoid-undefined-behaviour-in-for_each_helper.patch
+ "${FILESDIR}"/0004-server-Fix-undefined-behavior-in-wl_socket_init_for_.patch
)
multilib_src_configure() {
