Skip to content

Commit cb42f3f

Browse files
committed
rootless: fix dangling screen pixmap on RootlessUpdateScreenPixmap() OOM
On a calloc() failure while growing the rootless screen pixmap buffer, the old buffer was already freed and pixmap_data_size already bumped to the new (larger) size before the failure check -- leaving s->pixmap_data (and the screen pixmap pPix, still pointing at the freed block via its prior ModifyPixmapHeader() call) dangling. Worse, because pixmap_data_size was already bumped, a later same-or-smaller-size call sees pixmap_data_size < rowbytes as false and skips reallocating forever, permanently pinning the dangling state instead of retrying. Trigger: real host memory pressure during a rootless (Xquartz-style DDX) geometry change. Fix: allocate the replacement into a temporary first; only free the old buffer and update pixmap_data_size/pixmap_data together once the new allocation has succeeded. Found via a fleet-directed alloc-fail/UAF sweep of Xext/, mi/, and miext/, not from a live crash report. Verification note: miext/rootless/ only compiles into hw/xquartz, which this Linux build has disabled -- checked with a standalone `gcc -fsyntax-only` pass instead of the normal ninja+meson-test build/test cycle used for the other fixes in this sweep. Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net> (cherry picked from commit d536dde)
1 parent f844d7a commit cb42f3f

1 file changed

Lines changed: 16 additions & 4 deletions

File tree

miext/rootless/rootlessScreen.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,24 @@ RootlessUpdateScreenPixmap(ScreenPtr pScreen)
113113
rowbytes = PixmapBytePad(pScreen->width, pScreen->rootDepth);
114114

115115
if (s->pixmap_data_size < rowbytes) {
116-
free(s->pixmap_data);
116+
/*
117+
* Allocate the replacement before freeing/resizing the old one:
118+
* on failure this used to free s->pixmap_data, bump
119+
* pixmap_data_size to the new (larger) size, then return with
120+
* s->pixmap_data (and the screen pixmap pPix, still pointing at
121+
* the freed block via its old ModifyPixmapHeader() call) left
122+
* dangling -- and because pixmap_data_size was already bumped, a
123+
* later same-or-smaller-size call would see pixmap_data_size <
124+
* rowbytes as false and skip reallocating forever, pinning the
125+
* dangling state permanently.
126+
*/
127+
void *new_data = calloc(1, rowbytes);
128+
if (new_data == NULL)
129+
return;
117130

131+
free(s->pixmap_data);
118132
s->pixmap_data_size = rowbytes;
119-
s->pixmap_data = calloc(1, s->pixmap_data_size);
120-
if (s->pixmap_data == NULL)
121-
return;
133+
s->pixmap_data = new_data;
122134

123135
memset(s->pixmap_data, 0xFF, s->pixmap_data_size);
124136

0 commit comments

Comments
 (0)