From 6c5df47bf0e638c4305dafd40be37a6ef4ebd38f Mon Sep 17 00:00:00 2001 From: Andrew Hamilton Date: Thu, 3 Sep 2026 23:24:46 +1000 Subject: [PATCH] x11: clear or_focus_owner when its tab is destroyed An override-redirect menu that takes keyboard focus stashes the active tab in server.or_focus_owner; dismissing the menu later reactivates it. If the owner tab was closed first, the release activated freed memory (heap-use-after-free in set_active_tab via tab_current_window, caught by ASan: free in remove_tab <- drop_tab <- window_destroy_common <- handle_xs_destroy; use in or_window_focus_release <- or_handle_unmap). Clear the stash in remove_tab (the single live tab-free choke point), so the release drops keyboard focus instead. Without ASan the bug is silent (stale memory usually still mapped), which is why it survived: repro needs a focus-stealing OR menu (real menus paint content; an unpainted probe never completes the wl map handshake). Add tests/xormenu.c (frame + OR menu in separate processes, after the xpopup pattern) and a headless smoke test driving the exact sequence: menu takes focus, owner tab closed, menu dismissed, compositor alive and crash-indicator-free. --- kick.c | 6 +++ meson.build | 3 ++ tests/test_smoke.py | 89 +++++++++++++++++++++++++++++++++ tests/xormenu.c | 117 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 215 insertions(+) create mode 100644 tests/xormenu.c diff --git a/kick.c b/kick.c index 212cbec..722af9a 100644 --- a/kick.c +++ b/kick.c @@ -2795,6 +2795,12 @@ static void remove_tab(struct kick_tab *tab) { if (!tab) return; if (server.active_tab == tab) server.active_tab = NULL; + /* An override-redirect menu may have stashed this tab as its focus + owner (or_handle_map); the menu outlives the tab when the tab is + closed first. Clear it so the later focus release drops keyboard + focus instead of activating freed memory. */ + if (server.or_focus_owner == tab) + server.or_focus_owner = NULL; struct kick_tab *t; wl_list_for_each(t, &server.tabs, link) { if (t == tab) continue; diff --git a/meson.build b/meson.build index 1ddab62..ec3b845 100644 --- a/meson.build +++ b/meson.build @@ -337,4 +337,7 @@ if x11_dep.found() # xpopup: instantly-mapped managed transient (synthetic menu) for the # late-association regression test. executable('xpopup', 'tests/xpopup.c', dependencies: x11_dep) + # xormenu: override-redirect focus-stealing menu probe (frame + menu in + # separate processes) for the or_focus_owner regression test. + executable('xormenu', 'tests/xormenu.c', dependencies: x11_dep) endif diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 3f243e4..7f27591 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -38,6 +38,7 @@ BIN = os.path.join(ROOT, "build", "kick") WL = os.path.join(ROOT, "build", "wltest") XL = os.path.join(ROOT, "build", "xleader") XP = os.path.join(ROOT, "build", "xpopup") +XM = os.path.join(ROOT, "build", "xormenu") LOG = "/tmp/tabtest.log" RED = "#ff0000" BLUE = "#0000ff" @@ -3692,3 +3693,91 @@ def test_standalone_headless_xdg_activation_focus(tmp_path): finally: s.close() + + +def _wait_log(s, rx, timeout=15.0): + """Wait until the compositor log matches rx; return the match or None.""" + pat = re.compile(rx) + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + m = pat.search(s.read_log()) + if m: + return m + time.sleep(0.1) + return None + + +def test_standalone_headless_or_menu_owner_closed(tmp_path): + """An override-redirect menu's focus owner is freed before the menu is + dismissed: closing the menu's owner tab and then unmapping the menu must + drop keyboard focus, not activate freed memory (heap-use-after-free, + caught by ASan; on stock builds this guards the scenario stays alive + and crash-indicator-free).""" + if not os.path.exists(XM): + pytest.skip("needs build/xormenu (libX11 dev headers)") + disp_file = tmp_path / "xdisplay" + frame = menu = None + s = _spawn_standalone( + {}, + "sh -c 'echo $DISPLAY > %s; exec sleep 999'" % disp_file, + "%s --app-id keep '#0000ff' 320x240" % WL) + try: + s.wait_any_active() + # The reporter tab exports the embedded Xwayland DISPLAY to a file + # (children learn it synchronously at create time, before ready). + deadline = time.monotonic() + 15.0 + while time.monotonic() < deadline: + if disp_file.exists() and disp_file.read_text().strip(): + break + if "XWayland unavailable" in s.read_log(): + pytest.skip("no Xwayland binary") + time.sleep(0.1) + else: + pytest.fail("embedded Xwayland DISPLAY never exported") + xdisp = disp_file.read_text().strip() + assert xdisp.startswith(":"), f"bad DISPLAY {xdisp!r}" + xenv = dict(os.environ, DISPLAY=xdisp) + frame = subprocess.Popen( + [XM, "frame"], env=xenv, + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, start_new_session=True, + preexec_fn=_preexec_death(os.getpid())) + assert _wait_log(s, r"map app=1", timeout=15.0), \ + f"X11 frame never mapped:\n{s.read_log()[-500:]}" + # Make the frame's tab active (it normally maps focused already). + for _ in range(6): + m = re.findall(r"active app=(\d+)", s.read_log()) + if m and m[-1] == "1": + break + _oneshot_virtual_key(s.socket, "56:1,108:1,108:0,56:0") + time.sleep(0.7) + else: + pytest.fail("frame tab never became active") + # Map the menu now: the compositor stashes the frame tab as owner. + menu = subprocess.Popen( + [XM, "menu", "8"], env=xenv, + stdin=subprocess.DEVNULL, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, start_new_session=True, + preexec_fn=_preexec_death(os.getpid())) + assert _wait_log(s, r"or associate", timeout=15.0), \ + f"OR menu never associated:\n{s.read_log()[-500:]}" + # Close the owner tab, then let the menu unmap (release path). + _oneshot_virtual_key(s.socket, "56:1,111:1,111:0,56:0") + assert _wait_log(s, r"closing tab", timeout=15.0), \ + "owner tab never closed" + menu.wait(timeout=25.0) + time.sleep(1.0) + assert s.proc.poll() is None, \ + f"compositor died on menu dismiss; log tail:\n{s.read_log()[-400:]}" + log = s.read_log() + for pat in ("Segmentation fault", "Aborted", "BACKTRACE", + "Assertion `", "AddressSanitizer"): + assert pat not in log, f"crash indicator {pat!r} in log" + finally: + for p in (frame, menu): + try: + if p is not None: + p.kill() + except Exception: + pass + s.close() diff --git a/tests/xormenu.c b/tests/xormenu.c new file mode 100644 index 0000000..34b3485 --- /dev/null +++ b/tests/xormenu.c @@ -0,0 +1,117 @@ +/* xormenu: override-redirect focus-stealing menu probe. + * + * Exercises the compositor's X11 override-redirect focus handoff: a managed + * frame window (its own tab) plus a separate override-redirect popup with + * input hints, so mapping the popup makes the compositor stash the frame's + * tab as its focus owner. Closing the frame's tab while the popup is open + * and then dismissing the popup must drop keyboard focus, not activate + * freed memory (regression test for the or_focus_owner use-after-free). + * + * usage: + * xormenu frame -- map a managed 400x300 frame, pump events + * xormenu menu -- map an OR input-hint popup, paint it + * continuously (XWM only maps the wl surface + * once the X window has buffer damage), keep + * it up seconds, unmap, exit + * xormenu probe -- exit 0 if an X display opens, 1 otherwise + * (used to discover the compositor's lazy + * Xwayland display) + * + * The frame and the menu are separate processes so the driver can kill the + * frame (freeing the owner tab) while the menu stays mapped. + */ +#define _POSIX_C_SOURCE 200809L +#include +#include +#include +#include +#include +#include +#include +#include + +#define MENU_W 300 +#define MENU_H 200 + +static void set_pid(Display *d, Window w) { + pid_t pid = getpid(); + Atom a = XInternAtom(d, "_NET_WM_PID", False); + XChangeProperty(d, w, a, XA_CARDINAL, 32, PropModeReplace, + (unsigned char *)&pid, 1); +} + +static int frame_main(void) { + Display *d = XOpenDisplay(NULL); + if (!d) { + fprintf(stderr, "xormenu: frame: cannot open display\n"); + return 1; + } + int scr = DefaultScreen(d); + Window f = XCreateSimpleWindow(d, RootWindow(d, scr), 0, 0, 400, 300, 0, + BlackPixel(d, scr), 0x404080UL); + XStoreName(d, f, "xormenu-frame"); + set_pid(d, f); + XMapWindow(d, f); + XFlush(d); + for (;;) { + XEvent ev; + XNextEvent(d, &ev); + } + return 0; +} + +static int menu_main(int hold) { + Display *d = XOpenDisplay(NULL); + if (!d) { + fprintf(stderr, "xormenu: menu: cannot open display\n"); + return 1; + } + int scr = DefaultScreen(d); + XSetWindowAttributes attr = { .override_redirect = True }; + Window m = XCreateWindow(d, RootWindow(d, scr), 100, 100, + MENU_W, MENU_H, 0, CopyFromParent, InputOutput, + CopyFromParent, CWOverrideRedirect, &attr); + XStoreName(d, m, "xormenu-menu"); + set_pid(d, m); + XWMHints *h = XAllocWMHints(); + h->flags = InputHint; + h->input = True; + XSetWMHints(d, m, h); + XFree(h); + XFlush(d); + /* Let the X server manager see the window before mapping: a back-to-back + create+map can deliver MapNotify before management, losing the map. */ + sleep(2); + XMapWindow(d, m); + XFlush(d); + /* Keep it painted: no buffer damage means no wl-surface map. */ + GC gc = XCreateGC(d, m, 0, NULL); + XSetForeground(d, gc, 0x00C000UL); + time_t t0 = time(NULL); + struct timespec tick = { 0, 300 * 1000000L }; + while (time(NULL) - t0 < hold) { + XFillRectangle(d, m, gc, 0, 0, MENU_W, MENU_H); + XFlush(d); + nanosleep(&tick, NULL); + } + XUnmapWindow(d, m); + XFlush(d); + sleep(1); + return 0; +} + +int main(int argc, char **argv) { + if (argc > 1 && strcmp(argv[1], "frame") == 0) + return frame_main(); + if (argc > 1 && strcmp(argv[1], "menu") == 0) + return menu_main(argc > 2 ? atoi(argv[2]) : 8); + if (argc > 1 && strcmp(argv[1], "probe") == 0 && argc > 2) { + Display *d = XOpenDisplay(argv[2]); + if (!d) + return 1; + XCloseDisplay(d); + return 0; + } + fprintf(stderr, "usage: xormenu frame | menu | probe \n"); + return 2; +}