From 52232eaf409a28d77947df5622af274e1ef770c6 Mon Sep 17 00:00:00 2001
From: Charlie Lao <[email protected]>
Date: Wed, 03 Jun 2026 16:37:04 -0700
Subject: [PATCH] Vulkan: Fix UAF bug associated with orphaned sampler clean up

This change addresses a lifetime issue in the Vulkan backend where
samplers could be destroyed while still in use by a command buffer in a
different share group. This only occurs when using the
EGL_ANGLE_display_texture_share_group extension. Since this extension is
only used by chromium, and it guarantees its usage is always thread
safe, this CL adds back a per renderer mSamplerCache. If the extension
is used, per renderer sampler cache is used, otherwise per share group
cache is used. This way we will no longer have orphaned sampler, thus
avoids the threading problem and synchronization problem associated with
cleanupOrphanedSamplers.

Bug: chromium:517935753
Change-Id: I5f6b3ec9dc526a72d11eff7b0a939f40a0c01bc4
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/7910772
Commit-Queue: Charlie Lao <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Reviewed-by: Geoff Lang <[email protected]>
---

diff --git a/src/libANGLE/renderer/vulkan/ContextVk.h b/src/libANGLE/renderer/vulkan/ContextVk.h
index f88b655..097243e 100644
--- a/src/libANGLE/renderer/vulkan/ContextVk.h
+++ b/src/libANGLE/renderer/vulkan/ContextVk.h
@@ -253,6 +253,22 @@
     {
         return mShareGroupVk->getMetaDescriptorPools();
     }
+    SamplerCache &getSamplerCache()
+    {
+        if (hasDisplayTextureShareGroup())
+        {
+            return mRenderer->getSamplerCache();
+        }
+        return mShareGroupVk->getSamplerCache();
+    }
+    SamplerYcbcrConversionCache &getYuvConversionCache()
+    {
+        if (hasDisplayTextureShareGroup())
+        {
+            return mRenderer->getYuvConversionCache();
+        }
+        return mShareGroupVk->getYuvConversionCache();
+    }
 
     // Device loss
     gl::GraphicsResetStatus getResetStatus() override;
diff --git a/src/libANGLE/renderer/vulkan/SamplerVk.cpp b/src/libANGLE/renderer/vulkan/SamplerVk.cpp
index f15b039..1344921 100644
--- a/src/libANGLE/renderer/vulkan/SamplerVk.cpp
+++ b/src/libANGLE/renderer/vulkan/SamplerVk.cpp
@@ -40,7 +40,7 @@
     }
 
     vk::SamplerDesc desc(contextVk, mState, false, nullptr, static_cast<angle::FormatID>(0));
-    ANGLE_TRY(contextVk->getShareGroup()->getSamplerCache().getSampler(contextVk, desc, &mSampler));
+    ANGLE_TRY(contextVk->getSamplerCache().getSampler(contextVk, desc, &mSampler));
 
     return angle::Result::Continue;
 }
diff --git a/src/libANGLE/renderer/vulkan/ShareGroupVk.cpp b/src/libANGLE/renderer/vulkan/ShareGroupVk.cpp
index e7a430e..dee9b6e 100644
--- a/src/libANGLE/renderer/vulkan/ShareGroupVk.cpp
+++ b/src/libANGLE/renderer/vulkan/ShareGroupVk.cpp
@@ -176,8 +176,8 @@
     mPipelineLayoutCache.destroy(mRenderer);
     mDescriptorSetLayoutCache.destroy(mRenderer);
 
-    mSamplerCache.destroy(mRenderer, hasDisplayTextureShareGroup);
-    mYuvConversionCache.destroy(mRenderer, hasDisplayTextureShareGroup);
+    mSamplerCache.destroy(mRenderer);
+    mYuvConversionCache.destroy(mRenderer);
 
     mMetaDescriptorPools[DescriptorSetIndex::UniformsAndXfb].destroy(mRenderer);
     mMetaDescriptorPools[DescriptorSetIndex::Texture].destroy(mRenderer);
diff --git a/src/libANGLE/renderer/vulkan/TextureVk.cpp b/src/libANGLE/renderer/vulkan/TextureVk.cpp
index e46cba8..23c3b9b 100644
--- a/src/libANGLE/renderer/vulkan/TextureVk.cpp
+++ b/src/libANGLE/renderer/vulkan/TextureVk.cpp
@@ -2822,8 +2822,7 @@
     vk::SharedSamplerPtr sampler;
     vk::SamplerDesc samplerDesc(contextVk, samplerState, false, nullptr,
                                 static_cast<angle::FormatID>(0));
-    ANGLE_TRY(
-        contextVk->getShareGroup()->getSamplerCache().getSampler(contextVk, samplerDesc, &sampler));
+    ANGLE_TRY(contextVk->getSamplerCache().getSampler(contextVk, samplerDesc, &sampler));
 
     // If the image has more levels than supported, generate as many mips as possible at a time.
     const vk::LevelIndex maxGenerateLevels(UtilsVk::GetGenerateMipmapMaxLevels(contextVk));
@@ -3960,10 +3959,9 @@
     vk::SamplerDesc samplerDescSamplerExternal2DY2YEXT(contextVk, mState.getSamplerState(),
                                                        mState.isStencilMode(), &y2yConversionDesc,
                                                        mImage->getIntendedFormatID());
-    ANGLE_TRY(contextVk->getShareGroup()->getSamplerCache().getSampler(contextVk, samplerDesc,
-                                                                       &mSampler));
-    ANGLE_TRY(contextVk->getShareGroup()->getSamplerCache().getSampler(
-        contextVk, samplerDescSamplerExternal2DY2YEXT, &mY2YSampler));
+    ANGLE_TRY(contextVk->getSamplerCache().getSampler(contextVk, samplerDesc, &mSampler));
+    ANGLE_TRY(contextVk->getSamplerCache().getSampler(contextVk, samplerDescSamplerExternal2DY2YEXT,
+                                                      &mY2YSampler));
 
     updateCachedImageViewSerials();
 
diff --git a/src/libANGLE/renderer/vulkan/UtilsVk.cpp b/src/libANGLE/renderer/vulkan/UtilsVk.cpp
index cb97e0e..8317f1b 100644
--- a/src/libANGLE/renderer/vulkan/UtilsVk.cpp
+++ b/src/libANGLE/renderer/vulkan/UtilsVk.cpp
@@ -1602,8 +1602,7 @@
     }
 
     vk::SharedSamplerPtr sampler;
-    ANGLE_TRY(
-        contextVk->getShareGroup()->getSamplerCache().getSampler(contextVk, samplerDesc, &sampler));
+    ANGLE_TRY(contextVk->getSamplerCache().getSampler(contextVk, samplerDesc, &sampler));
 
     vk::DescriptorSetLayoutDesc descriptorSetDesc;
     descriptorSetDesc.addBinding(0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 1,
diff --git a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
index edee73d..97611ad 100644
--- a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
@@ -5797,7 +5797,7 @@
         ASSERT((contextVk->getFeatures().supportsYUVSamplerConversion.enabled));
         samplerYcbcrConversionInfo.sType = VK_STRUCTURE_TYPE_SAMPLER_YCBCR_CONVERSION_INFO;
         samplerYcbcrConversionInfo.pNext = nullptr;
-        ANGLE_TRY(contextVk->getShareGroup()->getYuvConversionCache().getSamplerYcbcrConversion(
+        ANGLE_TRY(contextVk->getYuvConversionCache().getSamplerYcbcrConversion(
             contextVk, mYcbcrConversionDesc, &samplerYcbcrConversionInfo.conversion));
         AddToPNextChain(&createInfo, &samplerYcbcrConversionInfo);
 
@@ -8556,44 +8556,27 @@
     ASSERT(mExternalFormatPayload.empty() && mVkFormatPayload.empty());
 }
 
-void SamplerYcbcrConversionCache::destroy(vk::Renderer *renderer, bool orphanConversionInfo)
+void SamplerYcbcrConversionCache::destroy(vk::Renderer *renderer)
 {
     renderer->accumulateCacheStats(VulkanCacheType::SamplerYcbcrConversion, mCacheStats);
 
-    // If the EGL_ANGLE_display_texture_share_group extension is causing some samplers to
-    // stay alive, there is no way to know which conversion info object needs to stay alive.
-    // stash them all in the renderer to be destroyed when possible.
-    if (orphanConversionInfo)
-    {
-        for (auto &iter : mExternalFormatPayload)
-        {
-            renderer->addSamplerYcbcrConversionToOrphanList(iter.second.release());
-        }
-        for (auto &iter : mVkFormatPayload)
-        {
-            renderer->addSamplerYcbcrConversionToOrphanList(iter.second.release());
-        }
-    }
-    else
-    {
-        VkDevice device = renderer->getDevice();
+    VkDevice device = renderer->getDevice();
 
-        uint32_t count = static_cast<uint32_t>(mExternalFormatPayload.size());
-        for (auto &iter : mExternalFormatPayload)
-        {
-            vk::SamplerYcbcrConversion &samplerYcbcrConversion = iter.second;
-            samplerYcbcrConversion.destroy(device);
-        }
-        renderer->onDeallocateHandle(vk::HandleType::SamplerYcbcrConversion, count);
-
-        count = static_cast<uint32_t>(mExternalFormatPayload.size());
-        for (auto &iter : mVkFormatPayload)
-        {
-            vk::SamplerYcbcrConversion &samplerYcbcrConversion = iter.second;
-            samplerYcbcrConversion.destroy(device);
-        }
-        renderer->onDeallocateHandle(vk::HandleType::SamplerYcbcrConversion, count);
+    uint32_t count = static_cast<uint32_t>(mExternalFormatPayload.size());
+    for (auto &iter : mExternalFormatPayload)
+    {
+        vk::SamplerYcbcrConversion &samplerYcbcrConversion = iter.second;
+        samplerYcbcrConversion.destroy(device);
     }
+    renderer->onDeallocateHandle(vk::HandleType::SamplerYcbcrConversion, count);
+
+    count = static_cast<uint32_t>(mVkFormatPayload.size());
+    for (auto &iter : mVkFormatPayload)
+    {
+        vk::SamplerYcbcrConversion &samplerYcbcrConversion = iter.second;
+        samplerYcbcrConversion.destroy(device);
+    }
+    renderer->onDeallocateHandle(vk::HandleType::SamplerYcbcrConversion, count);
 
     mExternalFormatPayload.clear();
     mVkFormatPayload.clear();
@@ -8642,33 +8625,18 @@
     ASSERT(mPayload.empty());
 }
 
-void SamplerCache::destroy(vk::Renderer *renderer, bool orphanReferencedSamplers)

diff --git a/src/tests/egl_tests/EGLContextSharingTest.cpp b/src/tests/egl_tests/EGLContextSharingTest.cpp
index c4b6de0..3d3edd5 100644
--- a/src/tests/egl_tests/EGLContextSharingTest.cpp
+++ b/src/tests/egl_tests/EGLContextSharingTest.cpp
@@ -411,6 +411,205 @@
     eglDestroyContext(display, context2);
 }
 
+// Regression test for sampler lifetime bug when EGL_ANGLE_display_texture_share_group is used.
+TEST_P(EGLContextSharingTest, DisplayShareGroupSamplerInFlightWhenOwningShareGroupDestroyed)
+{
+    EGLDisplay display = getEGLWindow()->getDisplay();
+    ANGLE_SKIP_TEST_IF(
+        !IsEGLDisplayExtensionEnabled(display, "EGL_ANGLE_display_texture_share_group"));
+    ANGLE_SKIP_TEST_IF(!IsVulkan());
+
+    EGLConfig config   = getEGLWindow()->getConfig();
+    EGLSurface surface = getEGLWindow()->getSurface();
+
+    const EGLint inShareGroupContextAttribs[] = {
+        EGL_CONTEXT_CLIENT_VERSION, 2, EGL_DISPLAY_TEXTURE_SHARE_GROUP_ANGLE, EGL_TRUE, EGL_NONE};
+
+    // Two contexts in the *display* texture share group but each in its own share group
+    EGLContext contextA = eglCreateContext(display, config, nullptr, inShareGroupContextAttribs);
+    EGLContext contextB = eglCreateContext(display, config, nullptr, inShareGroupContextAttribs);
+    ASSERT_NE(contextA, EGL_NO_CONTEXT);
+    ASSERT_NE(contextB, EGL_NO_CONTEXT);
+
+    // A larger render target widens the GPU window for the heavy draw in B.
+    constexpr int kHeavyDim       = 256;
+    const EGLint pbufferAttribs[] = {EGL_WIDTH, kHeavyDim, EGL_HEIGHT, kHeavyDim, EGL_NONE};
+    EGLSurface pbufferB           = eglCreatePbufferSurface(display, config, pbufferAttribs);
+    ASSERT_NE(pbufferB, EGL_NO_SURFACE);
+
+    // Context A: create the shared texture and draw with it once.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, contextA));
+    GLTexture sharedTex;
+    glBindTexture(GL_TEXTURE_2D, sharedTex);
+    constexpr GLsizei kTexSize = 4;
+    std::vector<GLColor> texData(kTexSize * kTexSize, GLColor::green);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, kTexSize, kTexSize, 0, GL_RGBA, GL_UNSIGNED_BYTE,
+                 texData.data());
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+
+    {
+        ANGLE_GL_PROGRAM(progA, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+        drawQuad(progA, essl1_shaders::PositionAttrib(), 0.5f);
+        EXPECT_GL_NO_ERROR();
+        glFlush();
+    }
+
+    // Context B: bind the same display-shared texture and submit a long running draw.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, pbufferB, pbufferB, contextB));
+    glViewport(0, 0, kHeavyDim, kHeavyDim);
+    glBindTexture(GL_TEXTURE_2D, sharedTex);
+
+    constexpr char kHeavyFS[] = R"(precision highp float;
+varying vec2 v_texCoord;
+uniform sampler2D u_tex2D;
+void main()
+{
+    vec4 acc = vec4(0.0);
+    // Busy work to make sure the GPU is using the texture when context A is destroyed.
+    for (int i = 0; i < 4000; ++i)
+    {
+        acc += texture2D(u_tex2D, v_texCoord + vec2(float(i) * 0.000001, 0.0));
+    }
+    gl_FragColor = acc * 0.00025;
+})";
+    ANGLE_GL_PROGRAM(progHeavy, essl1_shaders::vs::Texture2D(), kHeavyFS);
+    glUseProgram(progHeavy);
+    glUniform1i(glGetUniformLocation(progHeavy, "u_tex2D"), 0);
+    // Several expensive draw calls so the GPU is using the texture when context A is destroyed
+    for (int i = 0; i < 16; ++i)
+    {
+        drawQuad(progHeavy, essl1_shaders::PositionAttrib(), 0.5f);
+    }
+    EXPECT_GL_NO_ERROR();
+    glFlush();  // batch_B submitted, GPU now executing with S_A in its descriptor set.
+
+    // Still in B: dirty a sampler-state bit and draw again.
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
+    {
+        ANGLE_GL_PROGRAM(progB2, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+        drawQuad(progB2, essl1_shaders::PositionAttrib(), 0.5f);
+        EXPECT_GL_NO_ERROR();
+        glFlush();
+    }
+
+    // Destroy context A. The texture's sampler currently in use by context B should not be freed.
+    SafeDestroyContext(display, contextA);
+
+    // Finish, to make sure the work is done.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, pbufferB, pbufferB, contextB));
+    glFinish();
+    sharedTex.reset();
+    EXPECT_GL_NO_ERROR();
+
+    eglMakeCurrent(display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    eglDestroySurface(display, pbufferB);
+    SafeDestroyContext(display, contextB);
+    getEGLWindow()->makeCurrent();
+}
+
+// Variant of DisplayShareGroupSamplerInFlightWhenOwningShareGroupDestroyed, except the texture's
+// sampler state is not modified
+TEST_P(EGLContextSharingTest, DisplayShareGroupOrphanedSamplerReapedWhileInFlight)
+{
+    EGLDisplay display = getEGLWindow()->getDisplay();
+    ANGLE_SKIP_TEST_IF(
+        !IsEGLDisplayExtensionEnabled(display, "EGL_ANGLE_display_texture_share_group"));
+    ANGLE_SKIP_TEST_IF(!IsVulkan());
+
+    EGLConfig config   = getEGLWindow()->getConfig();
+    EGLSurface surface = getEGLWindow()->getSurface();
+
+    const EGLint inShareGroupContextAttribs[] = {
+        EGL_CONTEXT_CLIENT_VERSION, 2, EGL_DISPLAY_TEXTURE_SHARE_GROUP_ANGLE, EGL_TRUE, EGL_NONE};
+
+    EGLContext contextA = eglCreateContext(display, config, nullptr, inShareGroupContextAttribs);
+    EGLContext contextB = eglCreateContext(display, config, nullptr, inShareGroupContextAttribs);
+    EGLContext contextC = eglCreateContext(display, config, nullptr, inShareGroupContextAttribs);
+    ASSERT_NE(contextA, EGL_NO_CONTEXT);
+    ASSERT_NE(contextB, EGL_NO_CONTEXT);
+    ASSERT_NE(contextC, EGL_NO_CONTEXT);
+
+    constexpr int kHeavyDim       = 256;
+    const EGLint pbufferAttribs[] = {EGL_WIDTH, kHeavyDim, EGL_HEIGHT, kHeavyDim, EGL_NONE};
+    EGLSurface pbufferB           = eglCreatePbufferSurface(display, config, pbufferAttribs);
+    ASSERT_NE(pbufferB, EGL_NO_SURFACE);
+
+    // A: Create texture and draw: this creates an internal sampler for the texture
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, surface, surface, contextA));
+    GLTexture sharedTex;
+    glBindTexture(GL_TEXTURE_2D, sharedTex);
+    std::vector<GLColor> texData(16, GLColor::cyan);
+    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, 4, 4, 0, GL_RGBA, GL_UNSIGNED_BYTE, texData.data());
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
+    {
+        ANGLE_GL_PROGRAM(p, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+        drawQuad(p, essl1_shaders::PositionAttrib(), 0.5f);
+        EXPECT_GL_NO_ERROR();
+        glFlush();
+    }
+
+    // B: Draw with texture, using an expensive shader so the GPU is busy when context C is
+    // destroyed
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, pbufferB, pbufferB, contextB));
+    glViewport(0, 0, kHeavyDim, kHeavyDim);
+    glBindTexture(GL_TEXTURE_2D, sharedTex);
+    constexpr char kHeavyFS[] = R"(precision highp float;
+varying vec2 v_texCoord;
+uniform sampler2D u_tex2D;
+void main()
+{
+    vec4 acc = vec4(0.0);
+    for (int i = 0; i < 4000; ++i)
+    {
+        acc += texture2D(u_tex2D, v_texCoord + vec2(float(i) * 0.000001, 0.0));
+    }
+    gl_FragColor = acc * 0.00025;
+})";
+    ANGLE_GL_PROGRAM(progHeavy, essl1_shaders::vs::Texture2D(), kHeavyFS);
+    glUseProgram(progHeavy);
+    glUniform1i(glGetUniformLocation(progHeavy, "u_tex2D"), 0);
+    for (int i = 0; i < 16; ++i)
+    {
+        drawQuad(progHeavy, essl1_shaders::PositionAttrib(), 0.5f);
+    }
+    EXPECT_GL_NO_ERROR();
+    glFlush();
+
+    // Destroy A first; context B still references the texture's internal sampler.
+    SafeDestroyContext(display, contextA);
+
+    // B: dirty sampler state and draw so the texture's internal sampler is recreated.
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, pbufferB, pbufferB, contextB));
+    glBindTexture(GL_TEXTURE_2D, sharedTex);
+    glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
+    {
+        ANGLE_GL_PROGRAM(p2, essl1_shaders::vs::Texture2D(), essl1_shaders::fs::Texture2D());
+        drawQuad(p2, essl1_shaders::PositionAttrib(), 0.5f);
+        EXPECT_GL_NO_ERROR();
+        glFlush();
+    }
+
+    // Destroy unrelated context C.  The original texture's internal sampler, still in use by the
+    // GPU, should not be freed.
+    SafeDestroyContext(display, contextC);
+
+    ASSERT_EGL_TRUE(eglMakeCurrent(display, pbufferB, pbufferB, contextB));
+    glFinish();
+    sharedTex.reset();
+    EXPECT_GL_NO_ERROR();
+
+    eglMakeCurrent(display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    eglDestroySurface(display, pbufferB);
+    SafeDestroyContext(display, contextB);
+    getEGLWindow()->makeCurrent();
+}
+
 // Tests that after creating a texture using EGL_ANGLE_display_texture_share_group,
 // and use it for sampling, and then deleting the Context (which destroys shareGroup) works. If
 // anything cached in ShareGroup, it should be handled nicely if texture can outlive ShareGroup (for