From 3c069465c40d71edb47289a948f902b6e79114b0 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Fri, 23 Feb 2024 17:38:00 +0000 Subject: [PATCH] x11: Prevent memory corruption in XrmSetDatabase Despite what the XrmSetDatabase docs say, it may try to free the database, resulting in memory corruption. Prevent that by making sure it has no db to act on. If it's past the first run, we will have done XrmDestroyDatabase above anyway. This causes a segfault, reported in issue #1256. From the XrmSetDatabase man page: > The database previously associated with the display (if any) is not > destroyed. But this is patently false, from the source in Xlib's src/Xrm.c: void XrmSetDatabase( Display *display, XrmDatabase database) { LockDisplay(display); /* destroy database if set up implicitly by XGetDefault() */ if (display->db && (display->flags & XlibDisplayDfltRMDB)) { XrmDestroyDatabase(display->db); display->flags &= ~XlibDisplayDfltRMDB; } display->db = database; UnlockDisplay(display); } This is broken in Xlib since at least 2004[0], so now it's unfortunately a backwards compatibility issue... Fixes #1256. 0: https://lists.freedesktop.org/archives/xorg-commit-diffs/2004-March/000239.html --- src/x11/x.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/x11/x.c b/src/x11/x.c index 6176605ce..745d1db9b 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -489,6 +489,13 @@ static void XRM_update_db(void) XrmDestroyDatabase(db); } + // Despite what the XrmSetDatabase docs say, it may try to free + // the database, resulting in memory corruption. Prevent that + // by making sure it has no db to act on. If it's past the + // first run, we will have done XrmDestroyDatabase above + // anyway. + xctx.dpy->db = NULL; + db = XrmGetStringDatabase((const char*)prop.value); XrmSetDatabase(xctx.dpy, db); }