From 8ed09861dc44dd79ed67bec19e564901f38a80f6 Mon Sep 17 00:00:00 2001 From: stefan11111 Date: Sat, 3 Jan 2026 03:06:17 +0200 Subject: [PATCH] present: Call driver `check_flip` after we finish with our checks We don't really gain anything by calling `check_flip` early, but it turns out that doing so causes problems. Thanks to @jipok for finding this issue: ``` // gcc crash_x11libre.c -o crash_x11libre -lX11 -lXext -lXpresent // Code written by LLM! // WARNING: THIS PROGRAM IS INTENDED TO CRASH THE X SERVER (via XPresent + Glamor bug) void attempt_crash_via_present(Display* dpy, Window win, int width, int height, int depth) { XShmSegmentInfo shminfo; // 1. Calculate TIGHT stride (width * 4 bytes). // Misalignment here + XPresent is the killer combination. int stride = width * 4; size_t size = stride * height; // 2. Resize the window to match the buffer (simulating JTK resize behavior) XResizeWindow(dpy, win, width, height); // XSync(dpy, False); // Optional: let window manager catch up, but hammering triggers races better // 3. Allocate Shared Memory memset(&shminfo, 0, sizeof(shminfo)); shminfo.shmid = shmget(IPC_PRIVATE, size, IPC_CREAT | 0777); if (shminfo.shmid < 0) { perror("shmget"); return; } shminfo.shmaddr = (char*)shmat(shminfo.shmid, 0, 0); shminfo.readOnly = False; shmctl(shminfo.shmid, IPC_RMID, 0); // Auto-cleanup if (!XShmAttach(dpy, &shminfo)) { fprintf(stderr, "XShmAttach failed\n"); shmdt(shminfo.shmaddr); return; } // 4. Create the Misaligned Pixmap Pixmap pixmap = XShmCreatePixmap(dpy, win, shminfo.shmaddr, &shminfo, width, height, depth); // 5. THE TRIGGER: XPresentPixmap // This forces Glamor to treat the SHM segment as a presentable surface. // If strided alignment logic inside miCopyRegion/glamor is broken, this dies. XPresentPixmap(dpy, win, pixmap, 0, 0, 0, 0, 0, None, None, None, 0, 0, 0, 0, 0, 0); // Flush to make sure the server received the command XSync(dpy, False); // Cleanup for next iteration XFreePixmap(dpy, pixmap); XShmDetach(dpy, &shminfo); shmdt(shminfo.shmaddr); } int main() { Display* dpy = XOpenDisplay(NULL); if (!dpy) { fprintf(stderr, "No display\n"); return 1; } // Check extensions int major, minor, pixmaps; if (!XShmQueryVersion(dpy, &major, &minor, &pixmaps) || !pixmaps) { fprintf(stderr, "XShm not supported!\n"); return 1; } int present_opcode, event_base, error_base; if (!XQueryExtension(dpy, "Present", &present_opcode, &event_base, &error_base)) { fprintf(stderr, "XPresent extension not supported! (Can't repro bug)\n"); return 1; } int screen = DefaultScreen(dpy); XVisualInfo vinfo; if (!XMatchVisualInfo(dpy, screen, 32, TrueColor, &vinfo)) { printf("32-bit visual not found, using default.\n"); vinfo.visual = DefaultVisual(dpy, screen); vinfo.depth = DefaultDepth(dpy, screen); } XSetWindowAttributes attrs; attrs.colormap = XCreateColormap(dpy, RootWindow(dpy, screen), vinfo.visual, AllocNone); attrs.background_pixel = 0; // Black attrs.border_pixel = 0; Window win = XCreateWindow(dpy, RootWindow(dpy, screen), 0, 0, 100, 100, 0, vinfo.depth, InputOutput, vinfo.visual, CWColormap | CWBackPixel | CWBorderPixel, &attrs); XMapWindow(dpy, win); XSync(dpy, False); printf("Starting XPresent Stress Test...\n"); sleep(1); // Fixed height (enough to have data), varying width int height = 500; // Iterate widths. We expect crashes on non-standard alignments. // XLibre often crashes at 1266 width (stride 5064). for (int w = 1000; w < 2000; w++) { int stride = w * 4; printf("\rTesting Width: %4d | Stride: %5d (mod 64: %2d)", w, stride, stride % 64); fflush(stdout); attempt_crash_via_present(dpy, win, w, height, vinfo.depth); // Small delay to let the server actually try to process the Present request // before we trash the memory, though XSync should handle it. usleep(1000); } printf("\nTest finished successfully (No crash).\n"); XDestroyWindow(dpy, win); XCloseDisplay(dpy); return 0; } ``` Fixes: https://github.com/X11Libre/xserver/issues/1754 Fixes: https://github.com/X11Libre/xserver/commit/9108a2bf55f531572ce8b1a6480aa4e5d3a04eb5 Signed-off-by: stefan11111 --- present/present_scmd.c | 55 +++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/present/present_scmd.c b/present/present_scmd.c index c528d1319d..e762e02328 100644 --- a/present/present_scmd.c +++ b/present/present_scmd.c @@ -58,6 +58,29 @@ present_flip_pending_pixmap(ScreenPtr screen) return screen_priv->flip_pending->pixmap; } +static inline Bool +present_check_driver_flip(RRCrtcPtr crtc, + WindowPtr window, + PixmapPtr pixmap, + Bool sync_flip, + present_screen_priv_ptr screen_priv, + PresentFlipReason *reason) +{ + if (screen_priv->info->version >= 1 && screen_priv->info->check_flip2) { + if (!(*screen_priv->info->check_flip2) (crtc, window, pixmap, sync_flip, reason)) { + DebugPresent(("\td %08" PRIx32 " -> %08" PRIx32 "\n", window->drawable.id, pixmap ? pixmap->drawable.id : 0)); + return FALSE; + } + } else if (screen_priv->info->check_flip) { + if (!(*screen_priv->info->check_flip) (crtc, window, pixmap, sync_flip)) { + DebugPresent(("\td %08" PRIx32 " -> %08" PRIx32 "\n", window->drawable.id, pixmap ? pixmap->drawable.id : 0)); + return FALSE; + } + } + + return TRUE; +} + static Bool present_check_flip(RRCrtcPtr crtc, WindowPtr window, @@ -72,7 +95,6 @@ present_check_flip(RRCrtcPtr crtc, PixmapPtr window_pixmap; WindowPtr root = screen->root; present_screen_priv_ptr screen_priv = present_screen_priv(screen); - PresentFlipReason tmp_reason = PRESENT_FLIP_REASON_UNKNOWN; if (crtc) { screen_priv = present_screen_priv(crtc->pScreen); @@ -93,27 +115,6 @@ present_check_flip(RRCrtcPtr crtc, if (!screen_priv->info->flip) return FALSE; - /* Ask the driver for permission. Do this now to see if there's TearFree. */ - if (screen_priv->info->version >= 1 && screen_priv->info->check_flip2) { - if (!(*screen_priv->info->check_flip2) (crtc, window, pixmap, sync_flip, &tmp_reason)) { - DebugPresent(("\td %08" PRIx32 " -> %08" PRIx32 "\n", window->drawable.id, pixmap ? pixmap->drawable.id : 0)); - /* It's fine to return now unless the page flip failure reason is - * PRESENT_FLIP_REASON_BUFFER_FORMAT; we must only output that - * reason if all the other checks pass. - */ - if (!reason || tmp_reason != PRESENT_FLIP_REASON_BUFFER_FORMAT) { - if (reason) - *reason = tmp_reason; - return FALSE; - } - } - } else if (screen_priv->info->check_flip) { - if (!(*screen_priv->info->check_flip) (crtc, window, pixmap, sync_flip)) { - DebugPresent(("\td %08" PRIx32 " -> %08" PRIx32 "\n", window->drawable.id, pixmap ? pixmap->drawable.id : 0)); - return FALSE; - } - } - /* Make sure the window hasn't been redirected with Composite */ window_pixmap = screen->GetWindowPixmap(window); if (window_pixmap != screen->GetScreenPixmap(screen) && @@ -144,9 +145,13 @@ present_check_flip(RRCrtcPtr crtc, return FALSE; } - if (tmp_reason == PRESENT_FLIP_REASON_BUFFER_FORMAT) { - if (reason) - *reason = tmp_reason; + /** + * Ask the driver for permission + * + * We need to call the driver check flip after the full-screen window check. + * See: https://github.com/X11Libre/xserver/issues/1754 + */ + if (!present_check_driver_flip(crtc, window, pixmap, sync_flip, screen_priv, reason)) { return FALSE; }