From e34bd5538146b490da5efb8380783e429c7fb040 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net>
Date: Tue, 1 Oct 2024 16:46:45 -0400
Subject: [PATCH] Fix a potential race between drawing and discarding last
 buffer.

If the last is freed while the screen is being updated, and the use
after free bug is not detected, corruption could occur resulting in
bad areas of the screen that persists until the window gets an
expose event again, or the impacted pixels are updated again.

Possibly fixes an issue reported by nelgin via IRC.
---
 src/conio/x_events.c | 39 +++++++++++++++++++++------------------
 src/syncterm/CHANGES |  1 +
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/src/conio/x_events.c b/src/conio/x_events.c
index 777602dd08..09ba9e9f59 100644
--- a/src/conio/x_events.c
+++ b/src/conio/x_events.c
@@ -165,6 +165,7 @@ static int r_shift;
 static int g_shift;
 static int b_shift;
 static struct graphics_buffer *last = NULL;
+pthread_mutex_t	last_mutex;
 #ifdef WITH_XRENDER
 static XRenderPictFormat *xrender_pf = NULL;
 static Pixmap xrender_pm = None;
@@ -724,6 +725,17 @@ resize_pictures(void)
 #endif
 }
 
+static void
+free_last(void)
+{
+	pthread_mutex_lock(&last_mutex);
+	if (last) {
+		release_buffer(last);
+		last = NULL;
+	}
+	pthread_mutex_unlock(&last_mutex);
+}
+
 static void resize_xim(void)
 {
 	int width, height;
@@ -742,10 +754,7 @@ static void resize_xim(void)
 	if (xim) {
 		if (width == xim->width
 		    && height == xim->height) {
-			if (last) {
-				release_buffer(last);
-				last = NULL;
-			}
+			free_last();
 			return;
 		}
 #ifdef XDestroyImage
@@ -754,10 +763,7 @@ static void resize_xim(void)
 		x11.XDestroyImage(xim);
 #endif
 	}
-	if (last) {
-		release_buffer(last);
-		last = NULL;
-	}
+	free_last();
 	xim = x11.XCreateImage(dpy, visual, depth, ZPixmap, 0, NULL, width, height, 32, 0);
 	xim->data=(char *)calloc(1, xim->bytes_per_line*xim->height);
 }
@@ -1227,11 +1233,8 @@ static void init_mode_internal(int mode)
 	int ow, oh;
 
 	x11_get_maxsize(&mw, &mh);
+	free_last();
 	pthread_mutex_lock(&vstatlock);
-	if (last) {
-		release_buffer(last);
-		last = NULL;
-	}
 	ow = vstat.winwidth;
 	oh = vstat.winheight;
 	bitmap_drv_init_mode(mode, NULL, NULL, mw, mh);
@@ -1374,13 +1377,13 @@ local_draw_rect(struct rectlist *rect)
 		yoff = 0;
 
 	if (last && (last->w != dw || last->h != dh)) {
-		release_buffer(last);
-		last = NULL;
+		free_last();
 	}
 
 	/* TODO: Translate into local colour depth */
 	idx = 0;
 
+	pthread_mutex_lock(&last_mutex);
 	for (y = 0; y < dh; y++) {
 		for (x = 0; x < dw; x++) {
 			if (last) {
@@ -1485,6 +1488,7 @@ local_draw_rect(struct rectlist *rect)
 #endif
 	}
 	last = source;
+	pthread_mutex_lock(&last_mutex);
 }
 
 static void handle_resize_event(int width, int height, bool map)
@@ -1539,10 +1543,7 @@ static void expose_rect(int x, int y, int width, int height)
 	x11.XFillRectangle(dpy, win, gc, 0, yoff + sh, w, h);
 
 	/* Since we're exposing, we *have* to redraw */
-	if (last) {
-		release_buffer(last);
-		last = NULL;
-	}
+	free_last();
 	bitmap_drv_request_pixels();
 }
 
@@ -2271,6 +2272,8 @@ void x11_event_thread(void *args)
 		return;
 	}
 	sem_init(&event_thread_complete, 0, 0);
+	pthread_mutex_init(&last_mutex, NULL);
+
 	atexit(x11_terminate_event_thread);
 
 	if(local_pipe[0] > xfd)
diff --git a/src/syncterm/CHANGES b/src/syncterm/CHANGES
index b3fb5e55b4..658959ad5b 100644
--- a/src/syncterm/CHANGES
+++ b/src/syncterm/CHANGES
@@ -12,6 +12,7 @@ Fix focus-follows-mouse behaviour in herbstluftwm
 Fix SAUCE binary capture date
 Add Build Options menu item
 Fix saving non-integer scaling in GDI mode
+Fix X11 race between Expose event and redraw
 
 Version 1.2rc1
 --------------
-- 
GitLab