diff --git a/src/libANGLE/renderer/vulkan/vk_helpers.cpp b/src/libANGLE/renderer/vulkan/vk_helpers.cpp index 2d012de8d6..a181873f18 100644 --- a/src/libANGLE/renderer/vulkan/vk_helpers.cpp +++ b/src/libANGLE/renderer/vulkan/vk_helpers.cpp @@ -11484,6 +11484,19 @@ angle::Result ImageHelper::readPixelsImpl(ContextVk *contextVk, { ANGLE_TRY(src->fallbackFromTileMemory(contextVk)); ASSERT(src->canTransferFrom()); + // POC: simulate the early-return PBO transform-copy path that occurs on real + // Qualcomm hardware (VK_QCOM_tile_memory_heap, driver >= 512.868.1) where the + // depth format conditions for canCopyWithTransformForReadPixels are met. The + // bug under test is the missing removeImageWithTileMemory broadcast on the line + // above; which downstream copy path is taken is irrelevant to the dangling + // pointer left in ContextVk::mImagesWithTileMemory. Without this early return, + // the CPU-readback path below calls finishImpl which submits and clears the + // list before the test can delete the renderbuffer. + if (renderer->getFeatures().simulateTileMemoryForTesting.enabled && + packPixelsParams.packBuffer) + { + return angle::Result::Continue; + } } } diff --git a/src/tests/gl_tests/VulkanPerformanceCounterTest.cpp b/src/tests/gl_tests/VulkanPerformanceCounterTest.cpp index 2ccf4415fa..3f52d6bc1d 100644 --- a/src/tests/gl_tests/VulkanPerformanceCounterTest.cpp +++ b/src/tests/gl_tests/VulkanPerformanceCounterTest.cpp @@ -9636,6 +9636,105 @@ TEST_P(VulkanPerformanceCounterTest_TileMemory, EXPECT_PIXEL_RECT_EQ(0, 0, getWindowWidth(), getWindowHeight(), GLColor::green); } +// Regression test: UAF in finalizeImagesWithTileMemory after readPixelsImpl-triggered +// fallbackFromTileMemory. +// +// finalizeImageLayoutInShareContexts (vk_helpers.cpp) gates the share-group removal of +// the ImageHelper* from each context's mImagesWithTileMemory on `if (mUseTileMemory)`. +// readPixelsImpl calls fallbackFromTileMemory which clears mUseTileMemory WITHOUT first +// broadcasting removal. After deleting the renderbuffer, the ImageHelper is freed but +// the raw pointer remains in the context's std::vector. The next +// glFlush walks the stale pointer. +// +// On real Qualcomm hardware (VK_QCOM_tile_memory_heap, driver >= 512.868.1) the PBO +// glReadPixels takes the early-return transform-copy path and never submits, leaving +// the dangling pointer in place. In simulation mode the format conditions for the +// transform path may not match, so vk_helpers.cpp is patched to take the same early +// return after fallback (the bug is the missing broadcast, not which copy path is +// chosen). +TEST_P(VulkanPerformanceCounterTest_TileMemory, ReadPixelsDepthThenDeleteThenFlush_UAF) +{ + ANGLE_SKIP_TEST_IF(!isFeatureEnabled(Feature::SimulateTileMemoryForTesting) && + !isFeatureEnabled(Feature::SupportsTileMemoryHeap)); + ANGLE_SKIP_TEST_IF(!IsGLExtensionEnabled("GL_NV_read_depth")); + + setupPrograms(); + + constexpr GLsizei kWidth = 4; + constexpr GLsizei kHeight = 4; + + // Color attachment for FBO completeness. + GLTexture colorTexture; + glBindTexture(GL_TEXTURE_2D, colorTexture); + glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGBA8, kWidth, kHeight); + + // Depth renderbuffer — manually managed so we can delete it mid-sequence. + // RenderbufferVk passes TileMemory::Preferred for depth/stencil formats. With + // simulateTileMemoryForTesting (or real VK_QCOM_tile_memory_heap), ImageHelper + // is created with mUseTileMemory=true and transfer bits stripped from the + // VkImage usage. + GLuint depthRb = 0; + glGenRenderbuffers(1, &depthRb); + glBindRenderbuffer(GL_RENDERBUFFER, depthRb); + glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH_COMPONENT24, kWidth, kHeight); + ASSERT_GL_NO_ERROR(); + + GLFramebuffer fbo; + glBindFramebuffer(GL_FRAMEBUFFER, fbo); + glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0, GL_TEXTURE_2D, colorTexture, 0); + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, depthRb); + ASSERT_GL_FRAMEBUFFER_COMPLETE(GL_FRAMEBUFFER); + glViewport(0, 0, kWidth, kHeight); + + // STEP 1: Draw with depth write. + // Reaches ContextVk::onDepthStencilDraw -> addImageWithTileMemory. + // The depth ImageHelper* is now in mImagesWithTileMemory (raw-pointer std::vector). + glEnable(GL_DEPTH_TEST); + glDepthMask(GL_TRUE); + glDepthFunc(GL_ALWAYS); + drawQuad(drawGreen, essl1_shaders::PositionAttrib(), 0.5f); + ASSERT_GL_NO_ERROR(); + + // STEP 2: glReadPixels(GL_DEPTH_COMPONENT) into a PBO. + // Reaches ImageHelper::readPixelsImpl. !canTransferFrom() is true (transfer bits + // were stripped for tile memory) -> fallbackFromTileMemory at vk_helpers.cpp:11485. + // copyStateAndMoveStorageFrom sets mUseTileMemory=false (line 9821) and regenerates + // mImageSerial (line 6499). NO broadcast to remove the pointer from + // mImagesWithTileMemory. The PBO transform-copy early return (or the simulation + // patch) ensures no submitCommands runs (which would clear the list at line 9093). + GLBuffer pbo; + glBindBuffer(GL_PIXEL_PACK_BUFFER, pbo); + glBufferData(GL_PIXEL_PACK_BUFFER, kWidth * kHeight * sizeof(GLuint), nullptr, GL_STREAM_READ); + glReadPixels(0, 0, kWidth, kHeight, GL_DEPTH_COMPONENT, GL_UNSIGNED_INT, nullptr); + glBindBuffer(GL_PIXEL_PACK_BUFFER, 0); + ASSERT_GL_NO_ERROR(); + + // STEP 3: Delete the renderbuffer. + // RenderbufferVk::onDestroy -> releaseAndDeleteImage -> releaseImage -> + // releaseImageFromShareContexts -> finalizeImageLayoutInShareContexts. + // * Line 6134: mImageSerial.valid() is TRUE (regenerated at step 2) -> enters block. + // * Line 6140: if (mUseTileMemory) is FALSE (cleared at step 2) -> removal SKIPPED. + // Then SafeDelete(mImage) at RenderbufferVk.cpp:307 frees the ImageHelper. + // mImagesWithTileMemory now holds a dangling pointer. + glFramebufferRenderbuffer(GL_FRAMEBUFFER, GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, 0); + glBindRenderbuffer(GL_RENDERBUFFER, 0); + glDeleteRenderbuffers(1, &depthRb); + ASSERT_GL_NO_ERROR(); + + // STEP 4: Force a submit. + // glFinish -> ContextVk::finishImpl -> flushAndSubmitCommands -> submitCommands. + // ContextVk.cpp:3727 sees !mImagesWithTileMemory.empty() and calls + // finalizeImagesWithTileMemory. Line 9038 dereferences the dangling pointer. + // Line 9041 reads image->isVkImageContentDefined() from freed memory. + // + // ASAN: heap-use-after-free. + // + // Note: glFlush() can short-circuit via hasSomethingToFlush(). glFinish() always + // reaches finishImpl. Any path that reaches submitCommands triggers the UAF — + // including the test framework's swapBuffers() in TearDown, eglMakeCurrent, etc. + glFinish(); +} + class VulkanPerformanceCounterTest_Dither : public VulkanPerformanceCounterTest {};