[PATCH wayland v4 11/11] tests: Check for wrong fd delivery with zombie objects

Daniel Stone daniels at collabora.com
Thu Dec 28 19:53:57 UTC 2017


From: Derek Foreman <derekf at osg.samsung.com>

Until recently, if an event attempting to deliver an fd to a zombie
object was demarshalled after the object was made into a zombie, we
leaked the fd and left it in the buffer.

If another event attempting to deliver an fd to a live object was in that
same buffer, the zombie's fd would be delivered instead.

This test recreates that situation.

While this is a ridiculously contrived way to force this race - delivering
an event from a destruction handler - I do have reports of this race
being hit in real world code.

Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
Acked-by: Daniel Stone <daniels at collabora.com>
Signed-off-by: Daniel Stone <daniels at collabora.com>
---
 protocol/tests.xml   |  11 +++-
 tests/display-test.c | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 160 insertions(+), 4 deletions(-)

diff --git a/protocol/tests.xml b/protocol/tests.xml
index 77f6e243..ea56ae49 100644
--- a/protocol/tests.xml
+++ b/protocol/tests.xml
@@ -26,7 +26,7 @@
     SOFTWARE.
   </copyright>
 
-  <interface name="fd_passer" version="1">
+  <interface name="fd_passer" version="2">
     <description summary="Sends an event with an fd">
       A trivial interface for fd passing tests.
     </description>
@@ -39,5 +39,14 @@
       <description summary="passes a file descriptor"/>
       <arg name="fd" type="fd" summary="file descriptor"/>
     </event>
+
+    <!-- Version 2 additions -->
+    <request name="conjoin" since="2">
+      <description summary="register another fd passer with this one">
+        Tells this fd passer object about another one to send events
+        to for more complicated fd leak tests.
+      </description>
+      <arg name="passer" type="object" interface="fd_passer"/>
+    </request>
   </interface>
 </protocol>
diff --git a/tests/display-test.c b/tests/display-test.c
index 06231585..9b49a0ec 100644
--- a/tests/display-test.c
+++ b/tests/display-test.c
@@ -1128,27 +1128,83 @@ zombie_client(void *data)
 	client_disconnect_nocheck(c);
 }
 
+struct passer_data {
+	struct wl_resource *conjoined_passer;
+};
+
+static void
+feed_pipe(int fd, char tosend)
+{
+	int count;
+
+	do {
+		count = write(fd, &tosend, 1);
+	} while (count != 1 && errno == EAGAIN);
+	assert(count == 1);
+	close(fd);
+}
+
 static void
 fd_passer_clobber(struct wl_client *client, struct wl_resource *res)
 {
+	struct passer_data *pdata = wl_resource_get_user_data(res);
+	int pipes1[2], pipes2[2], ret;
+
+	if (pdata->conjoined_passer) {
+		ret = pipe(pipes1);
+		assert(ret == 0);
+		ret = pipe(pipes2);
+		assert(ret == 0);
+
+		wl_resource_queue_event(res, FD_PASSER_FD, pipes1[0]);
+		fd_passer_send_fd(pdata->conjoined_passer, pipes2[0]);
+		feed_pipe(pipes1[1], '1');
+		feed_pipe(pipes2[1], '2');
+		close(pipes1[0]);
+		close(pipes2[0]);
+	}
 	wl_resource_destroy(res);
 }
 
+static void
+fd_passer_twin(struct wl_client *client, struct wl_resource *res, struct wl_resource *passer)
+{
+	struct passer_data *pdata = wl_resource_get_user_data(res);
+
+	pdata->conjoined_passer = passer;
+}
+
 static const struct fd_passer_interface fdp_interface = {
 	fd_passer_clobber,
+	fd_passer_twin
 };
 
+static void
+pdata_destroy(struct wl_resource *res)
+{
+	struct passer_data *pdata = wl_resource_get_user_data(res);
+
+	free(pdata);
+}
+
 static void
 bind_fd_passer(struct wl_client *client, void *data,
 	       uint32_t vers, uint32_t id)
 {
 	struct wl_resource *res;
+	struct passer_data *pdata;
+
+	pdata = malloc(sizeof(*pdata));
+	assert(pdata);
+	pdata->conjoined_passer = NULL;
 
 	res = wl_resource_create(client, &fd_passer_interface, vers, id);
-	wl_resource_set_implementation(res, &fdp_interface, NULL, NULL);
+	wl_resource_set_implementation(res, &fdp_interface, pdata, pdata_destroy);
 	assert(res);
-	fd_passer_send_pre_fd(res);
-	fd_passer_send_fd(res, fileno(stdin));
+	if (vers == 1) {
+		fd_passer_send_pre_fd(res);
+		fd_passer_send_fd(res, fileno(stdin));
+	}
 }
 
 TEST(zombie_fd)
@@ -1168,3 +1224,94 @@ TEST(zombie_fd)
 
 	display_destroy(d);
 }
+
+
+static void
+double_pre_fd(void *data, struct fd_passer *fdp)
+{
+	assert(false);
+}
+
+static void
+double_fd(void *data, struct fd_passer *fdp, int32_t fd)
+{
+	char buf;
+	int count;
+
+	do {
+		count = read(fd, &buf, 1);
+	} while (count != 1 && errno == EAGAIN);
+	assert(count == 1);
+
+	close(fd);
+	fd_passer_destroy(fdp);
+	assert(buf == '2');
+}
+
+struct fd_passer_listener double_fd_passer_listener = {
+	double_pre_fd,
+	double_fd,
+};
+
+
+static void
+double_zombie_fd_handle_globals(void *data, struct wl_registry *registry,
+			 uint32_t id, const char *intf, uint32_t ver)
+{
+	struct fd_passer *fdp1, *fdp2;
+
+	if (!strcmp(intf, "fd_passer")) {
+		fdp1 = wl_registry_bind(registry, id, &fd_passer_interface, 2);
+		fd_passer_add_listener(fdp1, &double_fd_passer_listener, NULL);
+		fdp2 = wl_registry_bind(registry, id, &fd_passer_interface, 2);
+		fd_passer_add_listener(fdp2, &double_fd_passer_listener, NULL);
+		fd_passer_conjoin(fdp1, fdp2);
+		fd_passer_destroy(fdp1);
+	}
+}
+
+static const struct wl_registry_listener double_zombie_fd_registry_listener = {
+	double_zombie_fd_handle_globals,
+	NULL
+};
+
+static void
+double_zombie_client(void *data)
+{
+	struct client *c = client_connect();
+	struct wl_registry *registry;
+
+	registry = wl_display_get_registry(c->wl_display);
+	wl_registry_add_listener(registry, &double_zombie_fd_registry_listener, NULL);
+
+	/* Gets the registry */
+	wl_display_roundtrip(c->wl_display);
+
+	/* One more so server can respond to conjoin+destroy */
+	wl_display_roundtrip(c->wl_display);
+
+	/* And finally push out our last fd_passer.destroy */
+	wl_display_roundtrip(c->wl_display);
+
+	wl_registry_destroy(registry);
+
+	client_disconnect_nocheck(c);
+}
+
+TEST(zombie_fd_errant_consumption)
+{
+	struct display *d;
+	struct wl_global *g;
+
+	d = display_create();
+
+	g = wl_global_create(d->wl_display, &fd_passer_interface,
+			     2, d, bind_fd_passer);
+
+	client_create_noarg(d, double_zombie_client);
+	display_run(d);
+
+	wl_global_destroy(g);
+
+	display_destroy(d);
+}
-- 
2.14.3



More information about the wayland-devel mailing list