From 2bd398acb8f6a843a0f1ac90089fca42d1134fb7 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 11 Sep 2016 02:47:00 -0700 Subject: [PATCH 01/17] glx: Initialize glx even if there are currently no screens attached Failure to do so causes an overvlow in glxClientCallback Application Specific Information: X.Org X Server 1.18.99.1 Build Date: 20160911 ================================================================= ==52118==ERROR: AddressSanitizer: SEGV on unknown address 0x000102b27b80 (pc 0x000103433245 bp 0x70000de67c20 sp 0x70000de67c00 T6) #0 0x103433244 in __asan::asan_free(void*, __sanitizer::BufferedStackTrace*, __asan::AllocType) (libclang_rt.asan_osx_dynamic.dylib+0x3244) #1 0x10347aeee in wrap_free (libclang_rt.asan_osx_dynamic.dylib+0x4aeee) #2 0x102e6a5ed in glxClientCallback glxext.c:301 #3 0x102b672a3 in _CallCallbacks dixutils.c:737 #4 0x102b2f0c6 in CallCallbacks callback.h:83 #5 0x102b5c15a in NextAvailableClient dispatch.c:3562 #6 0x102d7060c in AllocNewConnection connection.c:777 #7 0x102d71355 in EstablishNewConnections connection.c:863 #8 0x102b662f0 in ProcessWorkQueue dixutils.c:523 #9 0x102d52a7f in WaitForSomething WaitFor.c:175 #10 0x102b204f6 in Dispatch dispatch.c:411 #11 0x102b61e01 in dix_main main.c:301 #12 0x10254c42a in server_thread quartzStartup.c:66 #13 0x7fffc5f16aaa in _pthread_body (libsystem_pthread.dylib+0x3aaa) #14 0x7fffc5f169f6 in _pthread_start (libsystem_pthread.dylib+0x39f6) #15 0x7fffc5f161fc in thread_start (libsystem_pthread.dylib+0x31fc) Signed-off-by: Jeremy Huddleston Sequoia (cherry picked from commit 1d2293101fca46c9a68c553f1be8e815c40de69a) --- glx/glxext.c | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/glx/glxext.c b/glx/glxext.c index b7d6d1196..2fd6fb8c8 100644 --- a/glx/glxext.c +++ b/glx/glxext.c @@ -281,24 +281,6 @@ GlxPushProvider(__GLXprovider * provider) __glXProviderStack = provider; } -static Bool -checkScreenVisuals(void) -{ - int i, j; - - for (i = 0; i < screenInfo.numScreens; i++) { - ScreenPtr screen = screenInfo.screens[i]; - for (j = 0; j < screen->numVisuals; j++) { - if ((screen->visuals[j].class == TrueColor || - screen->visuals[j].class == DirectColor) && - screen->visuals[j].nplanes > 12) - return TRUE; - } - } - - return FALSE; -} - static void GetGLXDrawableBytes(void *value, XID id, ResourceSizePtr size) { @@ -472,10 +454,6 @@ static Bool xorgGlxServerPreInit(const ExtensionEntry *extEntry) { if (glxGeneration != serverGeneration) { - /* Mesa requires at least one True/DirectColor visual */ - if (!checkScreenVisuals()) - return FALSE; - __glXContextRes = CreateNewResourceType((DeleteType) ContextGone, "GLXContext"); __glXDrawableRes = CreateNewResourceType((DeleteType) DrawableGone, From 24264dd39ab97a22bd5046d60bd29e6e60c8c057 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 11 Sep 2016 02:19:19 -0700 Subject: [PATCH 02/17] randr: Initialize RandR even if there are currently no screens attached Failure to do so causes an overvlow in RRClientCallback(). ================================================================= ==41262==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000103ccfbc8 at pc 0x0001034f32b9 bp 0x7000035a94c0 sp 0x7000035a94b8 WRITE of size 4 at 0x000103ccfbc8 thread T6 #0 0x1034f32b8 in RRClientCallback randr.c:72 #1 0x1038c75e3 in _CallCallbacks dixutils.c:737 #2 0x10388f406 in CallCallbacks callback.h:83 #3 0x1038bc49a in NextAvailableClient dispatch.c:3562 #4 0x103ad094c in AllocNewConnection connection.c:777 #5 0x103ad1695 in EstablishNewConnections connection.c:863 #6 0x1038c6630 in ProcessWorkQueue dixutils.c:523 #7 0x103ab2dbf in WaitForSomething WaitFor.c:175 #8 0x103880836 in Dispatch dispatch.c:411 #9 0x1038c2141 in dix_main main.c:301 #10 0x1032ac75a in server_thread quartzStartup.c:66 #11 0x7fffc5f16aaa in _pthread_body (libsystem_pthread.dylib+0x3aaa) #12 0x7fffc5f169f6 in _pthread_start (libsystem_pthread.dylib+0x39f6) #13 0x7fffc5f161fc in thread_start (libsystem_pthread.dylib+0x31fc) Signed-off-by: Jeremy Huddleston Sequoia (cherry picked from commit f9235000d67a61b0de951598146b4b5e0032384e) --- randr/randr.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/randr/randr.c b/randr/randr.c index 2994bcd6c..cc85d27bd 100644 --- a/randr/randr.c +++ b/randr/randr.c @@ -414,9 +414,6 @@ RRExtensionInit(void) { ExtensionEntry *extEntry; - if (RRNScreens == 0) - return; - if (!dixRegisterPrivateKey(&RRClientPrivateKeyRec, PRIVATE_CLIENT, sizeof(RRClientRec) + screenInfo.numScreens * sizeof(RRTimesRec))) From ecea15118fe2d86fcc8ce958a2075925b6ed13ee Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Tue, 14 Jun 2022 11:03:31 -0700 Subject: [PATCH 03/17] Revert "os/WaitFor: Check timers on every iteration" Workaround a performance issue that this introduces in XQuartz Fixes: https://github.com/XQuartz/XQuartz/issues/166 This reverts commit ac7a4bf44c68c5f323375974b208d4530fb5b60f. Signed-off-by: Jeremy Huddleston Sequoia --- os/WaitFor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/os/WaitFor.c b/os/WaitFor.c index b0747faee..f335b6667 100644 --- a/os/WaitFor.c +++ b/os/WaitFor.c @@ -192,11 +192,12 @@ WaitForSomething(Bool are_ready) /* deal with any blocked jobs */ ProcessWorkQueue(); - timeout = check_timers(); are_ready = clients_are_ready(); if (are_ready) timeout = 0; + else + timeout = check_timers(); BlockHandler(&timeout); if (NewOutputPending) From 247c17fdaa8c01c1c46b12c56175ff40041f85fa Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sat, 21 Mar 2026 17:05:20 -0700 Subject: [PATCH 04/17] rootless: Fix Glyphs damage bounding box origin and coordinate space The glyph bounding box in RootlessGlyphs is computed in drawable-relative (window-local) coordinates, but RootlessDamageBox expects global (screen) coordinates. Without translating by drawable.x/y, the damage region often fell outside the window's borderClip and was silently discarded by RootlessDamageRegion. RootlessGlyphs accumulated glyph positions starting from (xSrc, ySrc) instead of (0, 0). The xSrc/ySrc parameters are the source picture reference point, not destination offsets. Every standard Glyphs implementation (fbGlyphs, miGlyphs) starts position accumulation from (0, 0) and builds up destination coordinates by adding each GlyphList's xOff/yOff deltas. Starting from xSrc shifted the entire damage bounding box by (xSrc, ySrc), causing it to miss the actual rendered region. For typical text rendering where xSrc equals the destination x position, this doubled the offset, placing the damage box well outside the window's borderClip and causing RootlessDamageRegion to silently discard it. Fixes [1/2]: https://github.com/XQuartz/XQuartz/issues/323 Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/rootlessScreen.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/miext/rootless/rootlessScreen.c b/miext/rootless/rootlessScreen.c index 0cd1995c0..af911c2b3 100644 --- a/miext/rootless/rootlessScreen.c +++ b/miext/rootless/rootlessScreen.c @@ -348,8 +348,8 @@ RootlessGlyphs(CARD8 op, PicturePtr pSrc, PicturePtr pDst, //SCREEN_WRAP(ps, Glyphs); if (dstWin && IsFramedWindow(dstWin)) { - x = xSrc; - y = ySrc; + x = 0; + y = 0; while (nlist--) { x += list->xOff; @@ -392,6 +392,11 @@ RootlessGlyphs(CARD8 op, PicturePtr pSrc, PicturePtr pDst, y += glyph->info.yOff; } + /* RootlessDamageBox expects global (screen) coordinates */ + box.x1 += dstWin->drawable.x; + box.y1 += dstWin->drawable.y; + box.x2 += dstWin->drawable.x; + box.y2 += dstWin->drawable.y; RootlessDamageBox(dstWin, &box); } list++; From 5c3881c070611d41acb0e8bca831a80f633b20c3 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:38:10 -0700 Subject: [PATCH 05/17] rootless: Factor RootlessStopDrawing() into a frame-keyed helper RootlessStopDrawing() finds the frame to release by walking up from a window with TopLevelParent(), which is wrong for callers whose window no longer resolves to the frame it still owns. Once a framed window has been reparented below another framed window, that lookup finds either a different, live frame -- releasing the wrong one and leaving the intended one held -- or no frame at all, in which case it silently does nothing. Split out an entry point that names the frame directly so the next commits can release a frame from those paths. Naming the frame alone is sufficient because winRec->win is assigned in exactly one place, immediately adjacent to the SETWINREC() it pairs with, so a published frame record always points back at its own window. That is also what makes this behavior-preserving for the existing callers, since winRec->win is then the TopLevelParent() the old code computed. Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/rootlessCommon.c | 55 +++++++++++++++++++++++---------- miext/rootless/rootlessCommon.h | 4 +++ 2 files changed, 43 insertions(+), 16 deletions(-) diff --git a/miext/rootless/rootlessCommon.c b/miext/rootless/rootlessCommon.c index e80655e68..805ad203f 100644 --- a/miext/rootless/rootlessCommon.c +++ b/miext/rootless/rootlessCommon.c @@ -232,11 +232,6 @@ RootlessStartDrawing(WindowPtr pWindow) } } -/* - * RootlessStopDrawing - * Stop drawing to a window's backing buffer. If flush is true, - * damaged regions are flushed to the screen. - */ static int RestorePreDrawingPixmapVisitor(WindowPtr pWindow, void *data) { @@ -268,24 +263,28 @@ RestorePreDrawingPixmapVisitor(WindowPtr pWindow, void *data) return WT_WALKCHILDREN; } +/* + * RootlessStopDrawingFrame + * Stop drawing to the given frame's backing buffer. If flush is true, damaged regions are flushed to the screen. + * winRec must not be NULL. + * + * Unlike RootlessStopDrawing(), the frame is named directly instead of being found by walking up from a window. + * Callers need that once a framed window has been reparented below another framed window, because TopLevelParent() + * then resolves either to a different frame or to no frame at all rather than to the one this record describes. + * + * This deliberately does not process is_reorder_pending; see RootlessStopDrawing(). + */ void -RootlessStopDrawing(WindowPtr pWindow, Bool flush) +RootlessStopDrawingFrame(RootlessWindowPtr winRec, Bool flush) { - ScreenPtr pScreen = pWindow->drawable.pScreen; - WindowPtr top = TopLevelParent(pWindow); - RootlessWindowRec *winRec; - - if (top == NULL) - return; - winRec = WINREC(top); - if (winRec == NULL) - return; + WindowPtr pWin = winRec->win; + ScreenPtr pScreen = pWin->drawable.pScreen; if (winRec->is_drawing) { SCREENREC(pScreen)->imp->StopDrawing(winRec->wid, flush); FreeScratchPixmapHeader(winRec->pixmap); - TraverseTree(top, RestorePreDrawingPixmapVisitor, (void *) winRec); + TraverseTree(pWin, RestorePreDrawingPixmapVisitor, (void *) winRec); winRec->pixmap = NULL; winRec->is_drawing = FALSE; @@ -293,7 +292,31 @@ RootlessStopDrawing(WindowPtr pWindow, Bool flush) else if (flush) { SCREENREC(pScreen)->imp->UpdateRegion(winRec->wid, NULL); } +} + +/* + * RootlessStopDrawing + * Stop drawing to a window's backing buffer. If flush is true, + * damaged regions are flushed to the screen. + */ +void +RootlessStopDrawing(WindowPtr pWindow, Bool flush) +{ + WindowPtr top = TopLevelParent(pWindow); + RootlessWindowRec *winRec; + + if (top == NULL) + return; + winRec = WINREC(top); + if (winRec == NULL) + return; + + RootlessStopDrawingFrame(winRec, flush); + /* Reorder the window the caller named rather than the frame's owner. Clearing the flag consumes it either way, + * but RootlessReorderWindow() itself does nothing unless pWindow is the window that owns the frame. This is + * long-standing behavior; every caller that passes flush reorders a frame-owning window. + */ if (flush && winRec->is_reorder_pending) { winRec->is_reorder_pending = FALSE; RootlessReorderWindow(pWindow); diff --git a/miext/rootless/rootlessCommon.h b/miext/rootless/rootlessCommon.h index 4b9010840..2a20e7bd0 100644 --- a/miext/rootless/rootlessCommon.h +++ b/miext/rootless/rootlessCommon.h @@ -247,6 +247,10 @@ void RootlessDamageBox(WindowPtr pWindow, BoxPtr pBox); void RootlessRedisplay(WindowPtr pWindow); void RootlessRedisplayScreen(ScreenPtr pScreen); +// As RootlessRedisplay/RootlessStopDrawing, but for a frame named directly +// rather than one found by walking up from a window. winRec must not be NULL. +void RootlessStopDrawingFrame(RootlessWindowPtr winRec, Bool flush); + void RootlessQueueRedisplay(ScreenPtr pScreen); /* Return the colormap currently installed on the given screen. */ From 854a8df09bdc0684ae97f3779511bcc838bac97f Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:38:52 -0700 Subject: [PATCH 06/17] rootless: Stop drawing before destroying a frame Xplugin refuses to destroy a window whose backing store is still locked for drawing, and XQuartz escalates that refusal to FatalError(), so the whole session dies when a client closes a window. The rootless layer holds the drawing lock across requests and releases it lazily from the block handler, so every frame operation that has to talk to the implementation releases it first -- with three exceptions this series fixes, of which frame destruction is the one that kills the server, and whose documented contract has always claimed otherwise. Two paths reach the destroy with the lock still held. Reconfiguring a window after it was unrealized re-locks the frame, and the damage that would normally arm the block handler is clipped away against the emptied borderClip, so nothing ever releases it again; a window manager that restores a client's border width while withdrawing it does exactly this. Leaving rooted mode also destroys the root's frame, which is always realized and routinely drawn into. Updates are deliberately not flushed, matching how unrealizing a window releases the lock, so the contract is corrected rather than satisfied. Fixes: https://github.com/XQuartz/XQuartz/issues/451 Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/README.txt | 3 ++- miext/rootless/rootless.h | 3 ++- miext/rootless/rootlessWindow.c | 7 +++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/miext/rootless/README.txt b/miext/rootless/README.txt index f5ebc9080..e3c804737 100644 --- a/miext/rootless/README.txt +++ b/miext/rootless/README.txt @@ -173,7 +173,8 @@ typedef Bool (*RootlessCreateFrameProc) /* * Destroy a frame. - * Drawing is stopped and all updates are flushed before this is called. + * Drawing is stopped before this is called. + * The frame record stays valid for the duration of the call and is freed immediately afterwards. * * wid Frame id */ diff --git a/miext/rootless/rootless.h b/miext/rootless/rootless.h index a843e077b..77030c67c 100644 --- a/miext/rootless/rootless.h +++ b/miext/rootless/rootless.h @@ -114,7 +114,8 @@ typedef Bool (*RootlessCreateFrameProc) /* * Destroy a frame. - * Drawing is stopped and all updates are flushed before this is called. + * Drawing is stopped before this is called. + * The frame record stays valid for the duration of the call and is freed immediately afterwards. * * wid Frame id */ diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index 3cda2bbf9..bc787c998 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -182,6 +182,13 @@ RootlessCreateWindow(WindowPtr pWin) static void RootlessDestroyFrame(WindowPtr pWin, RootlessWindowPtr winRec) { + /* The implementation cannot destroy a frame that is still locked for drawing, and the rootless layer + * intentionally holds that lock across requests (see RootlessSourceValidate() and StartFrameResize()) and drops it + * lazily from the block handler. A frame reconfigured after its window was unrealized queues no damage to wake + * that handler, so it can still be locked here. + */ + RootlessStopDrawingFrame(winRec, FALSE); + SCREENREC(pWin->drawable.pScreen)->imp->DestroyFrame(winRec->wid); free(winRec); SETWINREC(pWin, NULL); From 5a0f22b6f662ad116ebcdd25cbbe1bffff628fd6 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:39:18 -0700 Subject: [PATCH 07/17] rootless: Stop drawing before handing a frame to a new top-level parent When a framed window is reparented below another window that has no frame of its own, the frame is handed over rather than destroyed, but nothing released the drawing lock first. The implementation rejects the unmap that follows on a locked frame and the return value is discarded, so the frame silently stays mapped. The resize after it was already covered, but only because reshaping the new parent happens to release the lock once the frame record has moved across; releasing it explicitly up front no longer depends on that ordering. Flushing rather than discarding matters here because the pending damage is expressed relative to the frame's current geometry, which the hand-off is about to rewrite. For a colour-indexed frame the implementation also re-lays-out the client-side backing store on the resize without adjusting the recorded damage, so damage left behind would afterwards describe stale coordinates over relocated content. Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/README.txt | 1 + miext/rootless/rootless.h | 1 + miext/rootless/rootlessWindow.c | 12 +++++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/miext/rootless/README.txt b/miext/rootless/README.txt index e3c804737..dce5141bf 100644 --- a/miext/rootless/README.txt +++ b/miext/rootless/README.txt @@ -233,6 +233,7 @@ typedef void (*RootlessReshapeFrameProc) /* * Unmap a frame. + * Drawing is stopped before this is called. * * wid Frame id */ diff --git a/miext/rootless/rootless.h b/miext/rootless/rootless.h index 77030c67c..668199124 100644 --- a/miext/rootless/rootless.h +++ b/miext/rootless/rootless.h @@ -173,6 +173,7 @@ typedef void (*RootlessReshapeFrameProc) /* * Unmap a frame. + * Drawing is stopped before this is called. * * wid Frame id */ diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index bc787c998..173ca94f0 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -1083,11 +1083,21 @@ RootlessReparentWindow(WindowPtr pWin, WindowPtr pPriorParent) RootlessDestroyFrame(pWin, winRec); } else { + /* Flush while the frame's damage still matches winRec's geometry, because RootlessInitializeFrame() below is + * about to change it, and release the lock before UnmapFrame() and ResizeFrame(), which the implementation + * rejects on a locked frame. + */ + RootlessStopDrawingFrame(winRec, TRUE); + if (!pTopWin->realized && pWin->realized) { SCREENREC(pScreen)->imp->UnmapFrame(winRec->wid); } - /* Switch the frame record from one to the other. */ + /* Switch the frame record from one to the other. Nothing in here may resolve a frame through + * TopLevelParent(): until the second SETWINREC() the record is unreachable from any window, and from there + * until RootlessInitializeFrame() it is reachable from pTopWin while winRec->win still names pWin, so a lookup + * would find it and then walk the wrong subtree. + */ SETWINREC(pWin, NULL); SETWINREC(pTopWin, winRec); From b4958e516bb926b9a1a6954dc422962622944455 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Mon, 17 Aug 2026 00:02:25 -0700 Subject: [PATCH 08/17] xquartz: Do not initialize the window hash inside assert() Both the frame hash and the queue that guards it were assigned inside assert(), so a build that defines NDEBUG drops the assignments and leaves them null for the life of the server, trapping on the first frame that is created. Signed-off-by: Jeremy Huddleston Sequoia --- hw/xquartz/xpr/xprFrame.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/xquartz/xpr/xprFrame.c b/hw/xquartz/xpr/xprFrame.c index 57d16d42f..bb4586d9b 100644 --- a/hw/xquartz/xpr/xprFrame.c +++ b/hw/xquartz/xpr/xprFrame.c @@ -476,10 +476,14 @@ xprInit(ScreenPtr pScreen) rootless_CopyBytes_threshold = xp_copy_bytes_threshold; rootless_CopyWindow_threshold = xp_scroll_area_threshold; - assert((window_hash = x_hash_table_new(NULL, NULL, NULL, NULL))); - assert((window_hash_serial_q = - dispatch_queue_create(BUNDLE_ID_PREFIX ".X11.xpr_window_hash", - NULL))); + window_hash = x_hash_table_new(NULL, NULL, NULL, NULL); + if (window_hash == NULL) + FatalError("Could not allocate window hash."); + + window_hash_serial_q = + dispatch_queue_create(BUNDLE_ID_PREFIX ".X11.xpr_window_hash", NULL); + if (window_hash_serial_q == NULL) + FatalError("Could not create window hash queue."); return TRUE; } From ff58a4afb5a34f014af16b7fecb217f9005c53b3 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:39:36 -0700 Subject: [PATCH 09/17] rootless: Guard against a NULL window when a native window moves The handler for a native window having been moved dereferences the window it is given without checking it, unlike its sibling that handles native state changes. The implementation looks the window up from an Xplugin window id and has always been able to come up empty, for instance once the frame has been destroyed but a queued move event for it has not yet been drained. The next commit makes that lookup return nothing reliably rather than occasionally, so guard the handler to match its sibling before it becomes easy to hit. Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/rootlessWindow.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index 173ca94f0..315cef7b5 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -105,7 +105,12 @@ RootlessNativeWindowMoved(WindowPtr pWin) ClientPtr pClient; RootlessWindowRec *winRec; + if (pWin == NULL) + return; + winRec = WINREC(pWin); + if (winRec == NULL) + return; if (xp_get_window_bounds(MAKE_WINDOW_ID(winRec->wid), &bounds) != Success) return; From ec54923f797f622c36e59c3c24b678b2911dc76b Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:40:24 -0700 Subject: [PATCH 10/17] xquartz: Fix use-after-free of the frame record in the window hash The hash mapping Xplugin window ids back to frame records is updated from asynchronously dispatched blocks, but the rootless layer frees the record as soon as the frame is destroyed, so both updates can run against freed memory. The removal leaves the hash handing out a freed record until the block is scheduled. The insertion is worse: it reads the record to obtain the key, so a frame created and destroyed within one turn of the queue files the record under a garbage id that the matching removal then cannot find, stranding it in the hash indefinitely. Doing both synchronously keeps the hash from ever describing a frame that no longer exists. Nothing else uses the queue in a way that could deadlock, and frames are created and destroyed rarely enough for the cost not to matter. The lookup also let the record escape its critical section before reading the window out of it, which is what actually exposed callers on the AppKit thread; reading it inside closes that. The window it returns may still be stale, but only the server thread ever dereferences one, and it is the thread that would have freed it. Signed-off-by: Jeremy Huddleston Sequoia --- hw/xquartz/xpr/xprFrame.c | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/hw/xquartz/xpr/xprFrame.c b/hw/xquartz/xpr/xprFrame.c index bb4586d9b..57f3f5c93 100644 --- a/hw/xquartz/xpr/xprFrame.c +++ b/hw/xquartz/xpr/xprFrame.c @@ -197,9 +197,9 @@ xprCreateFrame(RootlessWindowPtr pFrame, ScreenPtr pScreen, return FALSE; } - dispatch_async(window_hash_serial_q, ^ { - x_hash_table_insert(window_hash, pFrame->wid, pFrame); - }); + dispatch_sync(window_hash_serial_q, ^ { + x_hash_table_insert(window_hash, pFrame->wid, pFrame); + }); xprSetNativeProperty(pFrame); @@ -214,9 +214,9 @@ xprDestroyFrame(RootlessFrameID wid) { xp_error err; - dispatch_async(window_hash_serial_q, ^ { - x_hash_table_remove(window_hash, wid); - }); + dispatch_sync(window_hash_serial_q, ^ { + x_hash_table_remove(window_hash, wid); + }); err = xp_destroy_window(x_cvt_vptr_to_uint(wid)); if (err != Success) @@ -284,6 +284,9 @@ xprRestackFrame(RootlessFrameID wid, RootlessFrameID nextWid) wc.sibling = x_cvt_vptr_to_uint(nextWid); } + /* Unlike xprGetXWindow(), reading the record after the critical section is safe here because this only ever runs on + * the server thread, which is also the only thread that frees it. + */ dispatch_sync(window_hash_serial_q, ^ { winRec = x_hash_table_lookup(window_hash, wid, NULL); }); @@ -495,14 +498,21 @@ xprInit(ScreenPtr pScreen) WindowPtr xprGetXWindow(xp_window_id wid) { - RootlessWindowRec *winRec __block; + WindowPtr pWin __block = NULL; + + /* Load the window inside the critical section. Letting the frame record escape it would defeat the point, since + * the server thread frees the record as soon as the frame is destroyed. + */ dispatch_sync(window_hash_serial_q, ^ { - winRec = + RootlessWindowRec *winRec = x_hash_table_lookup(window_hash, x_cvt_uint_to_vptr(wid), NULL); + + if (winRec != NULL) + pWin = winRec->win; }); - return winRec != NULL ? winRec->win : NULL; + return pWin; } /* From 52b2a392fe5ce48399d29c824ca38c80b4fd57eb Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:40:43 -0700 Subject: [PATCH 11/17] xquartz: Do not treat a failed frame destroy as fatal Failing to hand a window back to the window server is recoverable, so terminating the X server over it is wildly disproportionate: every connected client loses its unsaved work because we could not release one window. This is the same judgement already made for the matching unlock failure, which has been logged rather than fatal for over a decade. The cost is worse than a plain memory leak and worth stating: Xplugin bails out before deregistering the window, so it stays registered, locked, and possibly still on screen for the life of the server, while we forget its id entirely. That is still preferable to killing the session. Signed-off-by: Jeremy Huddleston Sequoia --- hw/xquartz/xpr/xprFrame.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/xquartz/xpr/xprFrame.c b/hw/xquartz/xpr/xprFrame.c index 57f3f5c93..c9fb69d71 100644 --- a/hw/xquartz/xpr/xprFrame.c +++ b/hw/xquartz/xpr/xprFrame.c @@ -219,10 +219,10 @@ xprDestroyFrame(RootlessFrameID wid) }); err = xp_destroy_window(x_cvt_vptr_to_uint(wid)); + /* Not a FatalError: leaking the window costs us far less than terminating every client connected to the server. + */ if (err != Success) - FatalError("Could not destroy window %d (%d).", - (int)x_cvt_vptr_to_uint( - wid), (int)err); + ErrorF("Could not destroy window %d (%d).\n", (int)x_cvt_vptr_to_uint(wid), (int)err); } /* @@ -358,6 +358,10 @@ xprStartDrawing(RootlessFrameID wid, char **pixelData, int *bytesPerRow) err = xp_lock_window(x_cvt_vptr_to_uint( wid), NULL, NULL, data, rowbytes, NULL); + /* Deliberately still fatal, unlike the failures either side of this: the implementation leaves data[] untouched on + * failure and RootlessStartDrawing() cannot refuse, so carrying on would hand fb an uninitialized pointer to draw + * through. Demoting this needs a way to report the failure upwards first. + */ if (err != Success) FatalError("Could not lock window %d for drawing (%d).", (int)x_cvt_vptr_to_uint( @@ -389,7 +393,7 @@ xprStopDrawing(RootlessFrameID wid, Bool flush) * FatalError after http://xquartz.macosforge.org/trac/ticket/482 is fixed. */ if (err != Success) - ErrorF("Could not unlock window %d after drawing (%d).", + ErrorF("Could not unlock window %d after drawing (%d).\n", (int)x_cvt_vptr_to_uint( wid), (int)err); } From 4ed3b221413568da1f37ab459582c66ae2a65a34 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:42:17 -0700 Subject: [PATCH 12/17] rootless: Stop drawing before resizing the root's frame Resizing the root window takes a separate path from every other resize, and that path reconfigured the frame without releasing the drawing lock first. In rooted mode the root has a frame and is routinely drawn into, so a display reconfiguration could be rejected outright by the implementation and leave the root's frame at the old size, with the error discarded. Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/rootlessWindow.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index 315cef7b5..ba6f496bf 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -997,11 +997,24 @@ RootlessResizeWindow(WindowPtr pWin, int x, int y, /* Special case for resizing the root window */ BoxRec box; + /* Release the frame before reconfiguring it, as everywhere else: the implementation rejects the resize below on + * a frame that is locked for drawing. Flush first, because the pending damage is relative to the geometry the + * resize is about to replace. + */ + if (winRec) + RootlessStopDrawingFrame(winRec, TRUE); + pWin->drawable.x = x; pWin->drawable.y = y; pWin->drawable.width = w; pWin->drawable.height = h; + /* Keep the frame record in step with the window. Nothing else updates it on this path, and callers such as + * RootlessRepositionWindow() and RootlessDamageRegion() position the frame from it. + */ + if (winRec) + RootlessInitializeFrame(pWin, winRec); + box.x1 = x; box.y1 = y; box.x2 = x + w; From 5d0995ac76368f824e472c6efad6b86129db0390 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:41:15 -0700 Subject: [PATCH 13/17] xquartz: Log failures to reconfigure a frame Xplugin rejects any attempt to reconfigure a frame that is locked for drawing, and every caller discarded the result, so a move, resize, restack, reshape or unmap could be dropped without a trace and leave a window on screen that the X server believed it had already dealt with. That silence is much of why the fatal destroy failure took so long to pin down. Folding the variant that honours the native-reconfiguration guard into the same wrapper keeps that from being the one path with no diagnostic, and records which hooks the guard actually covers. Signed-off-by: Jeremy Huddleston Sequoia --- hw/xquartz/xpr/xprFrame.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/hw/xquartz/xpr/xprFrame.c b/hw/xquartz/xpr/xprFrame.c index c9fb69d71..cff001137 100644 --- a/hw/xquartz/xpr/xprFrame.c +++ b/hw/xquartz/xpr/xprFrame.c @@ -106,11 +106,18 @@ xprCopyWindow(RootlessFrameID wid, int dstNrects, const BoxRec *dstRects, int dx, int dy); -static inline xp_error +static inline void xprConfigureWindow(xp_window_id id, unsigned int mask, const xp_window_changes *values) { - return xp_configure_window(id, mask, values); + xp_error err = xp_configure_window(id, mask, values); + + /* Log rather than drop a reconfigure silently: the implementation rejects every mask on a frame that is locked for + * drawing, which would otherwise leave a window on screen that the X server believes it has already moved, resized + * or unmapped. + */ + if (err != Success) + ErrorF("Could not configure window %d, mask 0x%x (%d).\n", (int)id, mask, (int)err); } static void @@ -584,14 +591,16 @@ xprHideWindows(Bool hide) Bool no_configure_window; -static inline int +/* As xprConfigureWindow(), but honoring no_configure_window so that responding to a native window change does not echo + * straight back to the implementation. Only the colormap and hide paths below go through this; the move, resize, + * restack, reshape and unmap hooks call xprConfigureWindow() directly and so are not suppressed. + */ +static inline void configure_window(xp_window_id id, unsigned int mask, const xp_window_changes *values) { if (!no_configure_window) - return xp_configure_window(id, mask, values); - else - return XP_Success; + xprConfigureWindow(id, mask, values); } static From 5ba38fba62e91bc1ad1b17e4e3561b4993477627 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:42:54 -0700 Subject: [PATCH 14/17] rootless: Correct the documented flush behavior for moving a frame Moving a frame releases the drawing lock without flushing, and has done for as long as the two callers have existed, so the documented contract has never matched the code. Left alone it would now be the only stale copy of a sentence corrected for the hook just above it. Resizing is deliberately left as documented: with the root's frame and the reparent hand-off both flushing, every caller now really does flush first. Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/README.txt | 2 +- miext/rootless/rootless.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/miext/rootless/README.txt b/miext/rootless/README.txt index dce5141bf..b2f935d9d 100644 --- a/miext/rootless/README.txt +++ b/miext/rootless/README.txt @@ -183,7 +183,7 @@ typedef void (*RootlessDestroyFrameProc) /* * Move a frame on screen. - * Drawing is stopped and all updates are flushed before this is called. + * Drawing is stopped before this is called. * * wid Frame id * pScreen Screen to move the new frame to diff --git a/miext/rootless/rootless.h b/miext/rootless/rootless.h index 668199124..3538c8d9f 100644 --- a/miext/rootless/rootless.h +++ b/miext/rootless/rootless.h @@ -124,7 +124,7 @@ typedef void (*RootlessDestroyFrameProc) /* * Move a frame on screen. - * Drawing is stopped and all updates are flushed before this is called. + * Drawing is stopped before this is called. * * wid Frame id * pScreen Screen to move the new frame to From cab18f655fc4f1d94a58c3f1329139504d2d4ce1 Mon Sep 17 00:00:00 2001 From: Jeremy Huddleston Sequoia Date: Sun, 16 Aug 2026 23:16:01 -0700 Subject: [PATCH 15/17] rootless: Bound the clip of frameless windows to the screen pixmap The rootless screen pixmap is a single scanline with a zero devKind, so every row aliases row 0. That bounds vertical addressing, but fb derives addresses from the composite clip alone and never looks at the pixmap's size, so nothing bounds the horizontal term. Rootless deliberately leaves top-level windows unclipped by the root, which is only safe while a window has a frame to draw into. A window without one still points at the screen pixmap, and window coordinates are signed, so any client drawing to it reads and writes outside the allocation in either direction. Bound these clips where they are computed, so that every consumer inherits it -- clipList and borderClip also drive Render, expose handling and CopyWindow, none of which involve a GC. Clamp again during ValidateGC to cover a screen resize, which republishes the pixmap without recomputing any clip but the root's. Fixes: https://github.com/XQuartz/XQuartz/issues/466 Signed-off-by: Jeremy Huddleston Sequoia --- miext/rootless/rootlessCommon.c | 73 ++++++++++++++++++++++++++ miext/rootless/rootlessCommon.h | 9 ++++ miext/rootless/rootlessGC.c | 89 ++++++++++++++++++++++++++++++++ miext/rootless/rootlessValTree.c | 7 +++ miext/rootless/rootlessWindow.c | 12 ++++- 5 files changed, 189 insertions(+), 1 deletion(-) diff --git a/miext/rootless/rootlessCommon.c b/miext/rootless/rootlessCommon.c index 805ad203f..7c4186416 100644 --- a/miext/rootless/rootlessCommon.c +++ b/miext/rootless/rootlessCommon.c @@ -96,6 +96,79 @@ IsFramedWindow(WindowPtr pWin) return (top && WINREC(top)); } +/* + * RootlessWindowIsScreenBacked + * Returns TRUE if pWin draws into the screen pixmap. Framed and redirected + * windows have buffers of their own; anything else still points at the screen + * pixmap fbCreateWindow installed. Note that comparing pixmaps would not + * answer this: framed windows hold the screen pixmap while not mid-draw. + */ +Bool +RootlessWindowIsScreenBacked(WindowPtr pWin) +{ + WindowPtr top; + +#ifdef COMPOSITE + if (pWin->redirectDraw != RedirectDrawNone) + return FALSE; +#endif + + if (!dixPrivateKeyRegistered(&rootlessWindowPrivateKeyRec)) + return FALSE; + + /* Not IsFramedWindow(): an unrealized window still has its frame. */ + top = TopLevelParent(pWin); + + return (top == NULL || WINREC(top) == NULL); +} + +/* + * RootlessGetScreenPixmapBox + * Store the screen pixmap's extent in pBox, in the coordinates fb resolves + * against. Returns FALSE if there is no screen pixmap, or it is empty. + */ +Bool +RootlessGetScreenPixmapBox(ScreenPtr pScreen, BoxPtr pBox) +{ + PixmapPtr pPix = (*pScreen->GetScreenPixmap) (pScreen); + + if (pPix == NULL) + return FALSE; + + pBox->x1 = pPix->screen_x; + pBox->y1 = pPix->screen_y; + pBox->x2 = pPix->screen_x + pPix->drawable.width; + pBox->y2 = pPix->screen_y + pPix->drawable.height; + + return (pBox->x2 > pBox->x1 && pBox->y2 > pBox->y1); +} + +/* + * RootlessBoundRegionToScreenPixmap + * Restrict pRegion to what a window drawing into the screen pixmap may reach. + * That pixmap is one scanline, and fb bounds its addresses by the clip alone, + * so an unbounded clip indexes outside the allocation in either direction. + */ +void +RootlessBoundRegionToScreenPixmap(WindowPtr pWin, RegionPtr pRegion) +{ + RegionRec bound; + BoxRec box; + + if (!RootlessWindowIsScreenBacked(pWin)) + return; + + /* Nothing to bound against, so draw nothing. */ + if (!RootlessGetScreenPixmapBox(pWin->drawable.pScreen, &box)) { + RegionEmpty(pRegion); + return; + } + + RegionInit(&bound, &box, 1); + RegionIntersect(pRegion, pRegion, &bound); + RegionUninit(&bound); +} + Bool RootlessResolveColormap(ScreenPtr pScreen, int first_color, int n_colors, uint32_t * colors) diff --git a/miext/rootless/rootlessCommon.h b/miext/rootless/rootlessCommon.h index 2a20e7bd0..ce016fc50 100644 --- a/miext/rootless/rootlessCommon.h +++ b/miext/rootless/rootlessCommon.h @@ -238,6 +238,15 @@ extern RegionRec rootlessHugeRoot; // (e.g. it is visible and has a top-level or root parent) Bool IsFramedWindow(WindowPtr pWin); +// TRUE if this window draws into the screen pixmap rather than into a frame. +Bool RootlessWindowIsScreenBacked(WindowPtr pWin); + +// Store the screen pixmap's extent. FALSE if there is none, or it is empty. +Bool RootlessGetScreenPixmapBox(ScreenPtr pScreen, BoxPtr pBox); + +// Restrict a region to what a screen-pixmap-backed window may reach. +void RootlessBoundRegionToScreenPixmap(WindowPtr pWin, RegionPtr pRegion); + // Routines that cause regions to get redrawn. // DamageRegion and DamageRect are in global coordinates. // DamageBox is in window-local coordinates. diff --git a/miext/rootless/rootlessGC.c b/miext/rootless/rootlessGC.c index 5af18a4a0..40ce95741 100644 --- a/miext/rootless/rootlessGC.c +++ b/miext/rootless/rootlessGC.c @@ -315,6 +315,94 @@ RootlessCreateGC(GCPtr pGC) (pGC)->ops = &rootlessGCOps; \ } +/* + * Empty clip for the paths that must draw nothing and cannot allocate. It is + * installed with freeCompClip FALSE, so mi replaces it rather than writing + * through it or freeing it. + */ +static RegionRec rootlessEmptyClip = { {0, 0, 0, 0}, &RegionEmptyData }; + +/* + * RootlessInstallCompositeClip + * Give pGC a composite clip that it owns. The old one must never be modified + * in place: with no client clip it is the window's own clipList. + */ +static void +RootlessInstallCompositeClip(GCPtr pGC, RegionPtr pNewClip) +{ + if (pGC->freeCompClip) + RegionDestroy(pGC->pCompositeClip); + + pGC->pCompositeClip = pNewClip; + pGC->freeCompClip = TRUE; +} + +/* + * RootlessCloseCompositeClip + * Reduce pGC's composite clip to nothing, without allocating. + */ +static void +RootlessCloseCompositeClip(GCPtr pGC) +{ + if (pGC->freeCompClip) + RegionDestroy(pGC->pCompositeClip); + + pGC->pCompositeClip = &rootlessEmptyClip; + pGC->freeCompClip = FALSE; +} + +/* + * RootlessClampCompositeClip + * Bound the composite clip of a window drawing into the screen pixmap. + * + * RootlessMiValidateTree bounds these clips as it recomputes them, but a + * screen resize republishes the pixmap without recomputing any clip but the + * root's. After a shrink, a window nothing else revalidated is left bounded + * to the larger screen, so clamp here as well. + */ +static void +RootlessClampCompositeClip(GCPtr pGC, WindowPtr pWin) +{ + RegionPtr pClip = pGC->pCompositeClip; + RegionPtr pNewClip; + BoxPtr pExtents; + BoxRec box; + + if (pClip == NULL || !RootlessWindowIsScreenBacked(pWin)) + return; + + /* A nil region's extents say nothing about what it covers. */ + if (RegionNil(pClip)) + return; + + if (!RootlessGetScreenPixmapBox(pWin->drawable.pScreen, &box)) { + /* Nothing to bound against, so draw nothing. */ + RootlessCloseCompositeClip(pGC); + return; + } + + pExtents = RegionExtents(pClip); + if (pExtents->x1 >= box.x1 && pExtents->y1 >= box.y1 && + pExtents->x2 <= box.x2 && pExtents->y2 <= box.y2) + return; + + RL_DEBUG_MSG("clip of window %p (%d,%d %d,%d) escapes the screen pixmap " + "(%d,%d %d,%d); clamping\n", pWin, + pExtents->x1, pExtents->y1, pExtents->x2, pExtents->y2, + box.x1, box.y1, box.x2, box.y2); + + /* RegionCreate returns a shared static on failure, not NULL. */ + pNewClip = RegionCreate(&box, 1); + if (RegionNar(pNewClip)) { + RegionDestroy(pNewClip); + RootlessCloseCompositeClip(pGC); + return; + } + + RegionIntersect(pNewClip, pNewClip, pClip); + RootlessInstallCompositeClip(pGC, pNewClip); +} + static void RootlessValidateGC(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable) { @@ -335,6 +423,7 @@ RootlessValidateGC(GCPtr pGC, unsigned long changes, DrawablePtr pDrawable) #else VALIDATE_GC(pGC, changes, pDrawable); #endif + RootlessClampCompositeClip(pGC, (WindowPtr) pDrawable); } else { pGC->funcs->ValidateGC(pGC, changes, pDrawable); diff --git a/miext/rootless/rootlessValTree.c b/miext/rootless/rootlessValTree.c index f2146a63b..fac3e137f 100644 --- a/miext/rootless/rootlessValTree.c +++ b/miext/rootless/rootlessValTree.c @@ -104,6 +104,8 @@ Equipment Corporation. #include "globals.h" +#include "rootlessCommon.h" + int RootlessMiValidateTree(WindowPtr pRoot, WindowPtr pChild, VTKind kind); #define HasParentRelativeBorder(w) (!(w)->borderIsPixel && \ @@ -146,6 +148,10 @@ RootlessComputeClips(WindowPtr pParent, ScreenPtr pScreen, * will be completely inside the universe (the universe will cover it * completely). If the window is completely obscured, none of the * universe will cover the rectangle. + * + * A window bounded to the screen pixmap is the exception: it reports itself + * obscured where it extends past the screen. That also stops the VTMove + * case below from translating an unbounded clip into the stored regions. */ borderSize.x1 = pParent->drawable.x - wBorderWidth(pParent); borderSize.y1 = pParent->drawable.y - wBorderWidth(pParent); @@ -501,6 +507,7 @@ RootlessMiValidateTree(WindowPtr pRoot, /* Parent to validate */ if (pWin->viewable) { if (pWin->valdata) { RegionCopy(&childClip, &pWin->borderSize); + RootlessBoundRegionToScreenPixmap(pWin, &childClip); RootlessComputeClips(pWin, pScreen, &childClip, kind, &exposed); } else if (pWin->visibility == VisibilityNotViewable) { diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index ba6f496bf..f0b9add6b 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -461,8 +461,18 @@ RootlessRealizeWindow(WindowPtr pWin) RootlessWindowRec *winRec; winRec = RootlessEnsureFrame(pWin); - if (winRec == NULL) + if (winRec == NULL) { + /* + * RealizeTree marks the window viewable and ignores our return + * value, so it stays mapped with no buffer of its own. That is + * safe: it draws into the screen pixmap, whose clips are bounded. + * Clearing viewable here would only leave viewable children under + * an unviewable parent, since RealizeTree marks those too. + */ + ErrorF("rootless: no frame for window %p; it will not be drawn\n", + pWin); return FALSE; + } winRec->is_reorder_pending = TRUE; From 1f7805183f67e81e152f29299467efb04854f03e Mon Sep 17 00:00:00 2001 From: Antoni Sawicki Date: Wed, 26 Aug 2026 01:52:08 -0700 Subject: [PATCH 16/17] rootless: Don't use the implementation's accelerated copy for 8bit frames libXplugin backs XP_DEPTH_INDEX8 windows with a software backing store of its own, and its _xp_backing_scroll() computes the destination address of the copy with a 32bit (bytes_per_row * dy) multiply which is then zero extended. Any xp_copy_window() moving content upwards is therefore done ~4GB past the backing store and takes the server down: Exception Type: EXC_BAD_ACCESS (SIGBUS) Exception Codes: KERN_PROTECTION_FAILURE at 0x00000002283111e0 0 libsystem_platform.dylib _platform_memmove + 168 1 CoreGraphics CGBlt_copyBytes + 496 2 libXplugin.1.dylib _xp_backing_scroll + 156 3 libXplugin.1.dylib xp_copy_window + 516 4 X11.bin RootlessCopyWindow + 752 5 X11.bin DRICopyWindow + 180 6 X11.bin miSpriteCopyWindow + 168 7 X11.bin miMoveWindow + 388 This only affects 8bit frames; every other depth is scrolled by the window server instead. Nothing is lost by doing those copies ourselves, since libXplugin would only memcpy them as well. --- miext/rootless/rootlessWindow.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index f0b9add6b..c6aae41a9 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -668,6 +668,27 @@ RootlessNoCopyWindow(WindowPtr pWin, DDXPointRec ptOldOrg, RegionPtr prgnSrc) RegionTranslate(prgnSrc, -dx, -dy); } +/* + * RootlessCanAccelerateCopy + * Returns TRUE if the implementation may be asked to copy this window's + * contents around. + * + * 8bit frames are excluded: libXplugin keeps those in a software backing + * store of its own and _xp_backing_scroll() computes the destination + * address of the copy with a 32bit (bytes_per_row * dy) multiply which it + * then zero-extends. Any copy moving content upwards is therefore done + * ~4GB past the backing store and takes the server down with SIGBUS. + * Nothing is lost by copying such windows ourselves, libXplugin would + * only memcpy them as well. + */ +static Bool +RootlessCanAccelerateCopy(WindowPtr pWin) +{ + WindowPtr top = TopLevelParent(pWin); + + return top != NULL && top->drawable.depth != 8; +} + /* * RootlessCopyWindow * Update *new* location of window. Old location is redrawn with @@ -701,6 +722,7 @@ RootlessCopyWindow(WindowPtr pWin, DDXPointRec ptOldOrg, RegionPtr prgnSrc) /* If the area exceeds threshold, use the implementation's accelerated version. */ if (area > rootless_CopyWindow_threshold && + RootlessCanAccelerateCopy(pWin) && SCREENREC(pScreen)->imp->CopyWindow) { RootlessWindowRec *winRec; WindowPtr top; From 16c82f8f046478a18d2c32eb10ce20d7ab05b55d Mon Sep 17 00:00:00 2001 From: Antoni Sawicki Date: Wed, 26 Aug 2026 01:52:08 -0700 Subject: [PATCH 17/17] rootless: Pass global coordinates to RootlessDamageRegion() Since 74b8383 ("rootless: Use screen_x and screen_y instead of pixmap pointer hacks") RootlessCopyWindow() damages rgnDst after translating it into the pixmap's coordinate space, while RootlessDamageRegion() expects global coordinates and intersects with pWin->borderClip. For any frame which isn't at the screen origin the intersection comes up empty, so the copied contents are never flushed to the screen. Translate the region back before damaging it. --- miext/rootless/rootlessWindow.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/miext/rootless/rootlessWindow.c b/miext/rootless/rootlessWindow.c index c6aae41a9..771f7f599 100644 --- a/miext/rootless/rootlessWindow.c +++ b/miext/rootless/rootlessWindow.c @@ -761,6 +761,11 @@ RootlessCopyWindow(WindowPtr pWin, DDXPointRec ptOldOrg, RegionPtr prgnSrc) miCopyRegion(pDrawable, pDrawable, 0, &rgnDst, dx, dy, fbCopyWindowProc, 0, 0); + /* RootlessDamageRegion() wants global coordinates */ + if (pPixmap->screen_x || pPixmap->screen_y) { + RegionTranslate(&rgnDst, pPixmap->screen_x, pPixmap->screen_y); + } + RootlessDamageRegion(pWin, &rgnDst); }