From 2b87e0e53a1cf5d03512d495baa0125b71521be4 Mon Sep 17 00:00:00 2001 From: Marc Date: Mon, 8 Jun 2026 17:14:17 +0000 Subject: [PATCH] Fix surface ref leak and double-release in HashTexture2D multisample path The D3DPOOL_DEFAULT multisample branch aliased pSurfaceLevel_orig onto pResolvedSurface, leaking the GetSurfaceLevel reference and releasing the resolved surface twice (use-after-free). Track the read source in a separate non-owning pointer so each surface is released exactly once on every path. Co-Authored-By: Claude Opus 4.8 (1M context) --- source/D3D9Hooks.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/source/D3D9Hooks.cpp b/source/D3D9Hooks.cpp index 928d80a..fe3ec89 100644 --- a/source/D3D9Hooks.cpp +++ b/source/D3D9Hooks.cpp @@ -416,6 +416,11 @@ namespace { return {}; } + // The surface GetRenderTargetData reads from: either the level itself, or a + // non-multisampled resolve of it. Non-owning; pSurfaceLevel_orig and + // pResolvedSurface each own a ref and are released exactly once below. + IDirect3DSurface9* pCopySource = pSurfaceLevel_orig; + if (desc.MultiSampleType != D3DMULTISAMPLE_NONE) { if (D3D_OK != device->CreateRenderTarget(desc.Width, desc.Height, desc.Format, D3DMULTISAMPLE_NONE, 0, FALSE, &pResolvedSurface, nullptr)) { pSurfaceLevel_orig->Release(); @@ -423,31 +428,39 @@ namespace { return {}; } if (D3D_OK != device->StretchRect(pSurfaceLevel_orig, nullptr, pResolvedSurface, nullptr, D3DTEXF_NONE)) { + pResolvedSurface->Release(); + pResolvedSurface = nullptr; pSurfaceLevel_orig->Release(); Warning("GetTextureHash(2D) Failed: StretchRect (D3DPOOL_DEFAULT)\n"); return {}; } - pSurfaceLevel_orig = pResolvedSurface; + pCopySource = pResolvedSurface; } if (D3D_OK != device->CreateOffscreenPlainSurface(desc.Width, desc.Height, desc.Format, D3DPOOL_SYSTEMMEM, &pOffscreenSurface, nullptr)) { - pSurfaceLevel_orig->Release(); if (pResolvedSurface != nullptr) pResolvedSurface->Release(); + pSurfaceLevel_orig->Release(); Warning("GetTextureHash(2D) Failed: CreateOffscreenPlainSurface (D3DPOOL_DEFAULT)\n"); return {}; } - if (D3D_OK != device->GetRenderTargetData(pSurfaceLevel_orig, pOffscreenSurface)) { - pSurfaceLevel_orig->Release(); - if (pResolvedSurface != nullptr) pResolvedSurface->Release(); + if (D3D_OK != device->GetRenderTargetData(pCopySource, pOffscreenSurface)) { pOffscreenSurface->Release(); + if (pResolvedSurface != nullptr) pResolvedSurface->Release(); + pSurfaceLevel_orig->Release(); Warning("GetTextureHash(2D) Failed: GetRenderTargetData (D3DPOOL_DEFAULT)\n"); return {}; } + + // Copy is done; both source surfaces are finished with. Null pResolvedSurface + // so the shared cleanup below treats this as the pOffscreenSurface case. + if (pResolvedSurface != nullptr) { + pResolvedSurface->Release(); + pResolvedSurface = nullptr; + } pSurfaceLevel_orig->Release(); if (pOffscreenSurface->LockRect(&d3dlr, nullptr, D3DLOCK_READONLY) != D3D_OK) { - if (pResolvedSurface != nullptr) pResolvedSurface->Release(); pOffscreenSurface->Release(); Warning("GetTextureHash(2D) Failed: LockRect (D3DPOOL_DEFAULT)\n"); return {}; @@ -472,7 +485,6 @@ namespace { if (pOffscreenSurface != nullptr) { pOffscreenSurface->UnlockRect(); pOffscreenSurface->Release(); - if (pResolvedSurface != nullptr) pResolvedSurface->Release(); } else if (pResolvedSurface != nullptr) { pResolvedSurface->UnlockRect();