From 3f4c308d8152ad5649056e10fd2ebf6cd8f397f4 Mon Sep 17 00:00:00 2001 From: Olivier Fourdan Date: Mon, 13 Jan 2025 16:09:43 +0100 Subject: [PATCH] composite: initialize border clip even when pixmap alloc fails If it fails to allocate the pixmap, the function compAllocPixmap() would return early and leave the borderClip region uninitialized, which may lead to the use of uninitialized value as reported by valgrind: Conditional jump or move depends on uninitialised value(s) at 0x4F9B33: compClipNotify (compwindow.c:317) by 0x484FC9: miComputeClips (mivaltree.c:476) by 0x48559A: miValidateTree (mivaltree.c:679) by 0x4F0685: MapWindow (window.c:2693) by 0x4A344A: ProcMapWindow (dispatch.c:922) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Uninitialised value was created by a heap allocation at 0x4841866: malloc (vg_replace_malloc.c:446) by 0x4F47BC: compRedirectWindow (compalloc.c:171) by 0x4FA8AD: compCreateWindow (compwindow.c:592) by 0x4EBB89: CreateWindow (window.c:925) by 0x4A2E6E: ProcCreateWindow (dispatch.c:768) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Conditional jump or move depends on uninitialised value(s) at 0x48EEDBC: pixman_region_translate (pixman-region.c:2233) by 0x4F9255: RegionTranslate (regionstr.h:312) by 0x4F9B7E: compClipNotify (compwindow.c:319) by 0x484FC9: miComputeClips (mivaltree.c:476) by 0x48559A: miValidateTree (mivaltree.c:679) by 0x4F0685: MapWindow (window.c:2693) by 0x4A344A: ProcMapWindow (dispatch.c:922) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Uninitialised value was created by a heap allocation at 0x4841866: malloc (vg_replace_malloc.c:446) by 0x4F47BC: compRedirectWindow (compalloc.c:171) by 0x4FA8AD: compCreateWindow (compwindow.c:592) by 0x4EBB89: CreateWindow (window.c:925) by 0x4A2E6E: ProcCreateWindow (dispatch.c:768) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Conditional jump or move depends on uninitialised value(s) at 0x48EEE33: UnknownInlinedFun (pixman-region.c:2241) by 0x48EEE33: pixman_region_translate (pixman-region.c:2225) by 0x4F9255: RegionTranslate (regionstr.h:312) by 0x4F9B7E: compClipNotify (compwindow.c:319) by 0x484FC9: miComputeClips (mivaltree.c:476) by 0x48559A: miValidateTree (mivaltree.c:679) by 0x4F0685: MapWindow (window.c:2693) by 0x4A344A: ProcMapWindow (dispatch.c:922) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Uninitialised value was created by a heap allocation at 0x4841866: malloc (vg_replace_malloc.c:446) by 0x4F47BC: compRedirectWindow (compalloc.c:171) by 0x4FA8AD: compCreateWindow (compwindow.c:592) by 0x4EBB89: CreateWindow (window.c:925) by 0x4A2E6E: ProcCreateWindow (dispatch.c:768) by 0x4A25B5: Dispatch (dispatch.c:560) by 0x4B082A: dix_main (main.c:282) by 0x429233: main (stubmain.c:34) Fix compAllocPixmap() to initialize the border clip even if the creation of the backing pixmap has failed, to avoid depending later on uninitialized border clip values. Related to CVE-2025-26599, ZDI-CAN-25851 Signed-off-by: Olivier Fourdan Acked-by: Peter Hutterer Part-of: (cherry picked from commit b07192a8bedb90b039dc0f70ae69daf047ff9598) --- composite/compalloc.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/composite/compalloc.c b/composite/compalloc.c index 2aded9b9d..63b3059e9 100644 --- a/composite/compalloc.c +++ b/composite/compalloc.c @@ -607,9 +607,12 @@ compAllocPixmap(WindowPtr pWin) int h = pWin->drawable.height + (bw << 1); PixmapPtr pPixmap = compNewPixmap(pWin, x, y, w, h); CompWindowPtr cw = GetCompWindow(pWin); + Bool status; - if (!pPixmap) - return FALSE; + if (!pPixmap) { + status = FALSE; + goto out; + } if (cw->update == CompositeRedirectAutomatic) pWin->redirectDraw = RedirectDrawAutomatic; else @@ -623,14 +626,16 @@ compAllocPixmap(WindowPtr pWin) DamageRegister(&pWin->drawable, cw->damage); cw->damageRegistered = TRUE; } + status = TRUE; +out: /* Make sure our borderClip is up to date */ RegionUninit(&cw->borderClip); RegionCopy(&cw->borderClip, &pWin->borderClip); cw->borderClipX = pWin->drawable.x; cw->borderClipY = pWin->drawable.y; - return TRUE; + return status; } void