This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch wl/browser-all
in repository enlightenment.
View the commit online.
commit 5751788076dac5d901c92c6f5aaaa6f7fbcec0ad
Author: Cedric BAIL <[email protected]>
AuthorDate: Tue Aug 18 12:43:49 2026 -0600
e_comp_wl - do not crash when set_parent_of follows a failed import
zxdg_imported_v1.set_parent_of dereferenced a pointer that is NULL
whenever the import it belongs to did not resolve. An unknown handle is
answered with 'destroyed' and the Imported behind the resource is freed
there, while the resource itself lives until the client destroys it - and
a client is entitled to send set_parent_of in that window, because
'destroyed' is an event it cannot have read yet if the two requests went
out back to back.
So: import a handle nobody exported, then set_parent_of on it. Two
requests, no privileges, any client on the socket, and the compositor
segfaults with every window on it. Verified against the unguarded build -
SIGSEGV, core dumped.
The request is ignored rather than answered with a protocol error. The
object is already dead and the client has been told so; posting an error
would kill a client that did nothing wrong.
The test asserts only that the connection survives, which looks like
asserting nothing and is what a crash test looks like when it passes. It
was checked against a build with the guard removed to be sure it can
still fail.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01FtoiXoSKUmZb6Aix6U3GZS
---
src/bin/e_comp_wl_extensions.c | 14 ++++++
src/tests/wayland/meson.build | 2 +
src/tests/wayland/test_xdg_foreign.c | 84 ++++++++++++++++++++++++++++++++++++
3 files changed, 100 insertions(+)
diff --git a/src/bin/e_comp_wl_extensions.c b/src/bin/e_comp_wl_extensions.c
index a4679d02e..94b975e68 100644
--- a/src/bin/e_comp_wl_extensions.c
+++ b/src/bin/e_comp_wl_extensions.c
@@ -355,6 +355,20 @@ _e_comp_wl_zxdg_imported_v1_set_parent_of(struct wl_client *client EINA_UNUSED,
Imported *im = wl_resource_get_user_data(resource);
E_Client *ec = NULL;
+ /* NULL when the import failed. An unknown handle is answered with
+ * 'destroyed' and the Imported behind the resource is freed there, while
+ * the resource itself lives on until the client gets round to destroying
+ * it - and a client is entitled to call set_parent_of in that window,
+ * because 'destroyed' is an event it may not have read yet and the request
+ * was already in flight before it could have been.
+ *
+ * Every line below dereferences im, so this was a compositor-wide crash
+ * any client could cause deliberately with two requests and no privileges:
+ * import a handle nobody exported, then set_parent_of on it. Nothing to do
+ * but ignore the request - the object is already dead and the client has
+ * been told so. */
+ if (!im) return;
+
if (surface_resource) ec = wl_resource_get_user_data(surface_resource);
if (ec && ((ec->netwm.type != E_WINDOW_TYPE_NORMAL) || (!ec->comp_data->is_xdg_surface)))
diff --git a/src/tests/wayland/meson.build b/src/tests/wayland/meson.build
index 9c03787bf..86d2fc09c 100644
--- a/src/tests/wayland/meson.build
+++ b/src/tests/wayland/meson.build
@@ -41,6 +41,7 @@ foreach p: [
'@0@/unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml'.format(dir_wayland_protocols),
'@0@/unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml'.format(dir_wayland_protocols),
'@0@/stable/presentation-time/presentation-time.xml'.format(dir_wayland_protocols),
+ '@0@/unstable/xdg-foreign/xdg-foreign-unstable-v1.xml'.format(dir_wayland_protocols),
]
test_proto_src += gen_scanner_client.process(p)
test_proto_src += gen_scanner_impl.process(p)
@@ -83,6 +84,7 @@ wl_protocol_tests = [
['client-move', 'test_client_move.c'],
['maximize-restore', 'test_maximize_restore.c'],
['presentation-time', 'test_presentation_time.c'],
+ ['xdg-foreign', 'test_xdg_foreign.c'],
]
# Shared plumbing: registry binding, toplevel construction, enumeration and a
diff --git a/src/tests/wayland/test_xdg_foreign.c b/src/tests/wayland/test_xdg_foreign.c
new file mode 100644
index 000000000..db74b353b
--- /dev/null
+++ b/src/tests/wayland/test_xdg_foreign.c
@@ -0,0 +1,84 @@
+/* xdg-foreign: what happens after an import that could not succeed.
+ *
+ * zxdg_importer_v1.import takes an opaque handle string. When no window
+ * matches it the compositor answers 'destroyed' and that is the end of the
+ * object - but 'destroyed' is an *event*, and a client that sent import and
+ * set_parent_of back to back could not possibly have read it in between. The
+ * second request arrives at a compositor that has already thrown away its
+ * bookkeeping for the object.
+ *
+ * E freed the Imported on the failure and left the wl_resource alive with a
+ * NULL behind it, so the set_parent_of that followed dereferenced NULL. Two
+ * requests, no privileges, any client on the socket, and the whole compositor
+ * goes down with every window on it. Verified before the fix: SIGSEGV, core
+ * dumped.
+ *
+ * The assertion is only that we are still connected afterwards, which looks
+ * like asserting nothing - and that is what a crash test looks like when it
+ * passes. Checked against a build with the guard removed to be sure it can
+ * still fail.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "e_wl_testkit.h"
+#include "xdg-foreign-unstable-v1-client-protocol.h"
+
+#define PROG "test-xdg-foreign"
+
+static int destroyed_v1;
+
+static void
+_imported_v1_destroyed(void *data, struct zxdg_imported_v1 *i)
+{
+ (void)data; (void)i;
+ destroyed_v1++;
+}
+
+static const struct zxdg_imported_v1_listener _imported_v1_listener =
+{
+ _imported_v1_destroyed
+};
+
+int
+main(void)
+{
+ Tk *tk;
+ Tk_Toplevel *kid;
+ struct zxdg_importer_v1 *importer_v1;
+ struct zxdg_imported_v1 *bad;
+
+ tk = tk_connect(PROG);
+
+ if (tk_global_version(tk, "zxdg_importer_v1") < 1)
+ tk_fail(tk, "no zxdg_importer_v1");
+
+ importer_v1 = tk_bind(tk, &zxdg_importer_v1_interface, 1);
+ if (!importer_v1) tk_fail(tk, "advertised but would not bind");
+
+ kid = tk_toplevel_new(tk, "foreign-child", "child", 200, 150);
+ tk_settle(tk);
+ tk_expect(tk, "foreign-child");
+
+ /* Back to back, deliberately: the whole point is that the client has not
+ * read 'destroyed' yet when it sends the second request. */
+ bad = zxdg_importer_v1_import(importer_v1, "no-such-handle");
+ zxdg_imported_v1_add_listener(bad, &_imported_v1_listener, NULL);
+ zxdg_imported_v1_set_parent_of(bad, tk_toplevel_surface(kid));
+
+ /* tk_sync exits the process with "disconnected during sync" if the
+ * compositor has gone, so reaching the line after it is the assertion. */
+ tk_sync(tk);
+ tk_expect(tk, "foreign-child");
+
+ if (!destroyed_v1)
+ tk_fail(tk, "importing a handle nobody exported was not answered with "
+ "'destroyed' - the client is left holding an object that "
+ "will never resolve");
+
+ printf(PROG ": set_parent_of after a failed import - compositor survived\n");
+ printf(PROG ": ok\n");
+ tk_disconnect(tk);
+ return 0;
+}
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
