summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDerek Foreman <derekf@osg.samsung.com>2018-04-16 15:00:58 -0500
committerDerek Foreman <derekf@osg.samsung.com>2018-04-20 13:12:57 -0500
commit58ee271bff499b6b0d865fa0126990dc478bff24 (patch)
treefdde8b0be7fba18705c2b872017055522bfc8e73
parent685f9a09097a7c598dd737eb83ea419f6a21b260 (diff)
downloadwayland-58ee271bff499b6b0d865fa0126990dc478bff24.tar.gz
tests: Test for use after free in resource destruction signals
For years it's been common practice to free the object containing the wl_listener inside resource destruction notifiers, but not remove the listener from the list. That is: It's been safe to assume (when only one listener is present) that the wl_listener will never be touched again, since this is a destruction callback. Recently some patches were reviewed that made some positive changes to our internal signal handling code, but would've violated this assumption, and changed free()d memory in several existing compositors (weston, mutter, enlightenment). Since the breakage was extremely subtle, codify this assumption in a test case (thus promoting it to an ABI promise). Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk> Reviewed-by: Markus Ongyerth <wl@ongy.net> Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
-rw-r--r--tests/resources-test.c15
1 files changed, 15 insertions, 0 deletions
diff --git a/tests/resources-test.c b/tests/resources-test.c
index 59d8beb..76c9eb8 100644
--- a/tests/resources-test.c
+++ b/tests/resources-test.c
@@ -84,6 +84,17 @@ destroy_notify(struct wl_listener *l, void *data)
{
assert(l && data);
notify_called = 1;
+
+ /* In real code it's common to free the structure holding the
+ * listener at this point, but not to remove it from the list.
+ *
+ * That's fine since this is a destruction notification and
+ * it's the last time this signal can fire. We set these
+ * to NULL so we can check them later to ensure no write after
+ * "free" occurred.
+ */
+ l->link.prev = NULL;
+ l->link.next = NULL;
}
TEST(destroy_res_tst)
@@ -119,6 +130,8 @@ TEST(destroy_res_tst)
assert(destroyed);
assert(notify_called); /* check if signal was emitted */
assert(wl_client_get_object(client, id) == NULL);
+ assert(destroy_listener.link.prev == NULL);
+ assert(destroy_listener.link.next == NULL);
res = wl_resource_create(client, &wl_seat_interface, 2, 0);
assert(res);
@@ -131,6 +144,8 @@ TEST(destroy_res_tst)
wl_client_destroy(client);
assert(destroyed);
assert(notify_called);
+ assert(destroy_listener.link.prev == NULL);
+ assert(destroy_listener.link.next == NULL);
wl_display_destroy(display);
close(s[1]);