From 93d1441487db2a32c5332e51b2442a9cd0449a9e 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: --- 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 c7afc51be..b103f8ed5 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 @@ -245,8 +247,12 @@ AllocARGBCursor(unsigned char *psrcbits, unsigned char *pmaskbits, *ppCurs = NULL; pCurs = (CursorPtr) calloc(CURSOR_REC_SIZE + CURSOR_BITS_SIZE, 1); - if (!pCurs) + if (!pCurs) { + free(psrcbits); + free(pmaskbits); + free(argb); return BadAlloc; + } bits = (CursorBitsPtr) ((char *) pCurs + CURSOR_REC_SIZE); dixInitPrivates(pCurs, pCurs + 1, PRIVATE_CURSOR); diff --git a/dix/dispatch.c b/dix/dispatch.c index 2b77db41f..eefece0c8 100644 --- a/dix/dispatch.c +++ b/dix/dispatch.c @@ -3060,17 +3060,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 866c4a5d5..c2142dae9 100644 --- a/dix/window.c +++ b/dix/window.c @@ -3270,10 +3270,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 1efe11862..ca3e4d812 100644 --- a/render/render.c +++ b/render/render.c @@ -1636,17 +1636,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