From 8b947b4db996d8d2f4db1dcdc972e83b034cfaa0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 17 Apr 2026 12:01:03 +1000 Subject: [PATCH] cursor: fix AllocARGBCursor leak/double-free for psrcbits/pmaskbits/argb AllocARGBCursor took ownership of the psrcbits/pmaskbits/argb arguments. But if the initial calloc failed none of them were freed, without the caller knowing about it. Depending on the code path, those arguments would thus either leak or be double-freed. Fix it by always freeing those on error and updating the callers accordingly. Assisted-by: Claude:claude-opus-4-6 Part-of: Signed-off-by: Enrico Weigelt, metux IT consult --- dix/cursor.c | 10 ++++++++-- dix/dispatch.c | 12 +++--------- dix/window.c | 4 ---- render/render.c | 12 +++--------- 4 files changed, 14 insertions(+), 24 deletions(-) diff --git a/dix/cursor.c b/dix/cursor.c index f682df3e66..10f8d2677f 100644 --- a/dix/cursor.c +++ b/dix/cursor.c @@ -226,7 +226,9 @@ RealizeCursorAllScreens(CursorPtr pCurs) /** * does nothing about the resource table, just creates the data structure. - * does not copy the src and mask bits + * + * Takes ownership of \p psrcbits, \p pmaskbits, and \p argb -- all three + * are freed on error, the caller must not free them after calling this. * * \param psrcbits server-defined padding * \param pmaskbits server-defined padding @@ -242,8 +244,12 @@ AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits, *ppCurs = NULL; CursorPtr pCurs = (CursorPtr) calloc(CURSOR_REC_SIZE + CURSOR_BITS_SIZE, 1); - if (!pCurs) + if (!pCurs) { + free(psrcbits); + free(pmaskbits); + free(argb); return BadAlloc; + } CursorBitsPtr bits = (CursorBitsPtr) ((char *) pCurs + CURSOR_REC_SIZE); dixInitPrivates(pCurs, pCurs + 1, PRIVATE_CURSOR); diff --git a/dix/dispatch.c b/dix/dispatch.c index 24d5b6a264..4cfafd0373 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -3182,17 +3182,11 @@ ProcCreateCursor(ClientPtr client) &pCursor, client, stuff->cid); if (rc != Success) - goto bail; - if (!AddResource(stuff->cid, X11_RESTYPE_CURSOR, (void *) pCursor)) { - rc = BadAlloc; - goto bail; - } + return rc; + if (!AddResource(stuff->cid, X11_RESTYPE_CURSOR, (void *) pCursor)) + return BadAlloc; return Success; - bail: - free(srcbits); - free(mskbits); - return rc; } int diff --git a/dix/window.c b/dix/window.c index dc41c6321c..49f6b13df6 100644 --- a/dix/window.c +++ b/dix/window.c @@ -3241,10 +3241,6 @@ TileScreenSaver(ScreenPtr pScreen, int kind) else cursor = 0; } - else { - free(srcbits); - free(mskbits); - } } pWin = pScreen->screensaver.pWindow = diff --git a/render/render.c b/render/render.c index 69611e092a..f56708d420 100644 --- a/render/render.c +++ b/render/render.c @@ -1474,17 +1474,11 @@ ProcRenderCreateCursor(ClientPtr client) GetColor(twocolor[1], 0), &pCursor, client, stuff->cid); if (rc != Success) - goto bail; - if (!AddResource(stuff->cid, X11_RESTYPE_CURSOR, (void *) pCursor)) { - rc = BadAlloc; - goto bail; - } + return rc; + if (!AddResource(stuff->cid, X11_RESTYPE_CURSOR, (void *) pCursor)) + return BadAlloc; return Success; - bail: - free(srcbits); - free(mskbits); - return rc; } static int