From a0683f849d4f6ea24ddfa848d03a9796c6cb5580 Mon Sep 17 00:00:00 2001
From: Sunny Sachanandani <sunnyps@chromium.org>
Date: Fri, 15 May 2026 13:36:29 -0700
Subject: [PATCH] [gpu] Fix GPU synchronization bypass in D3DImageBacking

GetPendingWaitFences returned an empty list of fences if one of the
internal waits failed (e.g., due to an invalid injected fence).
Callers interpreted this as no synchronization being required, leading
to potential data races and information leaks.

This CL changes GetPendingWaitFences to return std::optional and
returns std::nullopt on failure. Callers are updated to fail the
access if GetPendingWaitFences returns std::nullopt.

It also refactors the lazy initialization of the texture device fence
to only insert it into the map when everything succeeds, avoiding
partial state mutation.

Bug: 498721671
Test: D3DImageBackingFactoryTest.InvalidExternalFence
Link: https://chromium-review.googlesource.com/id/I2d3ff59eaa59d7a29e484056be70b26b6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7823350
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Auto-Submit: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1631525}
---

diff --git a/gpu/command_buffer/service/shared_image/d3d_image_backing.cc b/gpu/command_buffer/service/shared_image/d3d_image_backing.cc
index c9746faba..cf9c985 100644
--- a/gpu/command_buffer/service/shared_image/d3d_image_backing.cc
+++ b/gpu/command_buffer/service/shared_image/d3d_image_backing.cc
@@ -1138,7 +1138,7 @@
                                                        device, src_texture);
 }
 
-std::vector<scoped_refptr<gfx::D3DSharedFence>>
+std::optional<std::vector<scoped_refptr<gfx::D3DSharedFence>>>
 D3DImageBacking::GetPendingWaitFences(
     const Microsoft::WRL::ComPtr<ID3D11Device>& wait_d3d11_device,
     const wgpu::Device& wait_dawn_device,
@@ -1148,33 +1148,40 @@
   // (i.e. scanout cases) means we always need to check for the presence of the
   // availability fence.
   if (!use_cross_device_fence_synchronization() && !dcomp_texture_) {
-    return {};
+    return std::vector<scoped_refptr<gfx::D3DSharedFence>>{};
   }
 
   // Lazily create and signal the D3D11 fence on the texture's original device
   // if not present and we're using the backing on another device.
-  auto& texture_device_fence = d3d11_signaled_fence_map_[texture_d3d11_device_];
+  auto it = d3d11_signaled_fence_map_.find(texture_d3d11_device_);
+  scoped_refptr<gfx::D3DSharedFence> texture_device_fence =
+      it != d3d11_signaled_fence_map_.end() ? it->second : nullptr;
+
   if (wait_d3d11_device != texture_d3d11_device_ && !texture_device_fence) {
     texture_device_fence =
         gfx::D3DSharedFence::CreateForD3D11(texture_d3d11_device_);
     if (!texture_device_fence) {
       LOG(ERROR) << "Failed to retrieve D3D11 signal fence";
-      return {};
+      return std::nullopt;
     }
     // Make D3D11 device wait for |write_fences_| since we'll replace it below.
     for (auto& fence : write_fences_) {
       if (!fence->WaitD3D11(texture_d3d11_device_)) {
         LOG(ERROR) << "Failed to wait for write fence";
-        return {};
+        return std::nullopt;
       }
     }
     if (!texture_device_fence->IncrementAndSignalD3D11()) {
       LOG(ERROR) << "Failed to signal D3D11 signal fence";
-      return {};
+      return std::nullopt;
     }
     // Store it in |write_fences_| so it's waited on for all subsequent access.
     write_fences_.clear();
     write_fences_.insert(texture_device_fence);
+
+    // Insert in the map only when everything succeeds.
+    d3d11_signaled_fence_map_.insert_or_assign(texture_d3d11_device_,
+                                               std::move(texture_device_fence));
   }
 
   // TODO(crbug.com/335003893): Investigate how to avoid passing any fences back
@@ -1299,11 +1306,15 @@
   CHECK(shared_texture_memory);
 
   // Defer clearing fences until later to handle Dawn failure to import texture.
-  std::vector<scoped_refptr<gfx::D3DSharedFence>> wait_fences =
+  auto wait_fences =
       GetPendingWaitFences(dawn_d3d11_device, device, write_access);
+  if (!wait_fences) {
+    LOG(ERROR) << "Failed to get pending wait fences";
+    return nullptr;
+  }
   std::vector<wgpu::SharedFence> shared_fences;
   std::vector<uint64_t> signaled_values;
-  for (auto& wait_fence : wait_fences) {
+  for (auto& wait_fence : *wait_fences) {
     // TODO(crbug.com/335003893): Look into caching the wgpu::SharedFence object
     // in gfx::D3DSharedFence.
     shared_fences.push_back(CreateDawnSharedFence(device, wait_fence));
@@ -1548,10 +1559,13 @@
   auto unwrapped_d3d12_resource = EnsureD3D12Resource();
 
   // Defer clearing fences until later to handle failure to synchronize.
-  std::vector<scoped_refptr<gfx::D3DSharedFence>> wait_fences =
-      GetPendingWaitFences(d3d11on12_device, /*dawn_device=*/nullptr,
-                           write_access);
-  for (auto& wait_fence : wait_fences) {
+  auto wait_fences = GetPendingWaitFences(
+      d3d11on12_device, /*dawn_device=*/nullptr, write_access);
+  if (!wait_fences) {
+    LOG(ERROR) << "Failed to get pending wait fences";
+    return false;
+  }
+  for (auto& wait_fence : *wait_fences) {
     if (!wait_fence->WaitD3D11(d3d11on12_device)) {
       LOG(ERROR) << "Failed to wait for fence";
       return false;
@@ -1605,9 +1619,13 @@
   }
 
   // Defer clearing fences until later to handle D3D11 failure to synchronize.
-  std::vector<scoped_refptr<gfx::D3DSharedFence>> wait_fences =
+  auto wait_fences =
       GetPendingWaitFences(d3d11_device, /*dawn_device=*/nullptr, write_access);
-  for (auto& wait_fence : wait_fences) {
+  if (!wait_fences) {
+    LOG(ERROR) << "Failed to get pending wait fences";
+    return false;
+  }
+  for (auto& wait_fence : *wait_fences) {
     if (!wait_fence->WaitD3D11(d3d11_device)) {
       LOG(ERROR) << "Failed to wait for fence";
       return false;
diff --git a/gpu/command_buffer/service/shared_image/d3d_image_backing.h b/gpu/command_buffer/service/shared_image/d3d_image_backing.h
index 58f5987..4d69281a 100644
--- a/gpu/command_buffer/service/shared_image/d3d_image_backing.h
+++ b/gpu/command_buffer/service/shared_image/d3d_image_backing.h
@@ -343,7 +343,8 @@
   // no-op. Similarly, |wait_dawn_device| can be provided to skip over waits on
   // fences previously signaled on the same Dawn device which are cached in
   // |dawn_signaled_fence_map_|.
-  std::vector<scoped_refptr<gfx::D3DSharedFence>> GetPendingWaitFences(
+  std::optional<std::vector<scoped_refptr<gfx::D3DSharedFence>>>
+  GetPendingWaitFences(
       const Microsoft::WRL::ComPtr<ID3D11Device>& wait_d3d11_device,
       const wgpu::Device& wait_dawn_device,
       bool write_access) EXCLUSIVE_LOCKS_REQUIRED(lock_);
diff --git a/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc b/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
index de85bff..235b4e45 100644
--- a/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
+++ b/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
@@ -1044,6 +1044,87 @@
   RunCreateSharedImageFromHandleTest(DXGI_FORMAT_R8G8B8A8_TYPELESS);
 }
 
+TEST_P(D3DImageBackingFactoryTest, InvalidExternalFence) {
+  Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device =
+      shared_image_factory_->GetDeviceForTesting();
+  if (!gfx::D3DSharedFence::IsSupported(d3d11_device.Get())) {
+    GTEST_SKIP();
+  }
+
+  const auto format = viz::SinglePlaneFormat::kRGBA_8888;
+  const gfx::Size size(1, 1);
+  const auto color_space = gfx::ColorSpace::CreateSRGB();
+  const gpu::SharedImageUsageSet usage =
+      SHARED_IMAGE_USAGE_GLES2_READ | SHARED_IMAGE_USAGE_DISPLAY_READ;
+
+  D3D11_TEXTURE2D_DESC desc;
+  desc.Width = size.width();
+  desc.Height = size.height();
+  desc.MipLevels = 1;
+  desc.ArraySize = 1;
+  desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
+  desc.SampleDesc.Count = 1;
+  desc.SampleDesc.Quality = 0;
+  desc.Usage = D3D11_USAGE_DEFAULT;
+  desc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET;
+  desc.CPUAccessFlags = 0;
+  desc.MiscFlags =
+      D3D11_RESOURCE_MISC_SHARED_NTHANDLE | D3D11_RESOURCE_MISC_SHARED;
+  Microsoft::WRL::ComPtr<ID3D11Texture2D> d3d11_texture;
+  HRESULT hr = d3d11_device->CreateTexture2D(&desc, nullptr, &d3d11_texture);
+  ASSERT_EQ(hr, S_OK);
+
+  Microsoft::WRL::ComPtr<IDXGIResource1> dxgi_resource;
+  hr = d3d11_texture.As(&dxgi_resource);
+  ASSERT_EQ(hr, S_OK);
+
+  HANDLE shared_handle;
+  hr = dxgi_resource->CreateSharedHandle(
+      nullptr, DXGI_SHARED_RESOURCE_READ | DXGI_SHARED_RESOURCE_WRITE, nullptr,
+      &shared_handle);
+  ASSERT_EQ(hr, S_OK);

diff --git a/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc b/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
index de85bff..235b4e45 100644
--- a/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
+++ b/gpu/command_buffer/service/shared_image/d3d_image_backing_factory_unittest.cc
@@ -1044,6 +1044,87 @@
   RunCreateSharedImageFromHandleTest(DXGI_FORMAT_R8G8B8A8_TYPELESS);
 }
 
+TEST_P(D3DImageBackingFactoryTest, InvalidExternalFence) {
+  Microsoft::WRL::ComPtr<ID3D11Device> d3d11_device =
+      shared_image_factory_->GetDeviceForTesting();
+  if (!gfx::D3DSharedFence::IsSupported(d3d11_device.Get())) {
+    GTEST_SKIP();
+  }
+
+  const auto format = viz::SinglePlaneFormat::kRGBA_8888;
+  const gfx::Size size(1, 1);
+  const auto color_space = gfx::ColorSpace::CreateSRGB();
+  const gpu::SharedImageUsageSet usage =
+      SHARED_IMAGE_USAGE_GLES2_READ | SHARED_IMAGE_USAGE_DISPLAY_READ;
+
+  D3D11_TEXTURE2D_DESC desc;
+  desc.Width = size.width();
+  desc.Height = size.height();
+  desc.MipLevels = 1;
+  desc.ArraySize = 1;
+  desc.Format = DXGI_FORMAT_R8G8B8A8_UNORM;
+  desc.SampleDesc.Count = 1;
+  desc.SampleDesc.Quality = 0;
+  desc.Usage = D3D11_USAGE_DEFAULT;
+  desc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET;
+  desc.CPUAccessFlags = 0;
+  desc.MiscFlags =
+      D3D11_RESOURCE_MISC_SHARED_NTHANDLE | D3D11_RESOURCE_MISC_SHARED;
+  Microsoft::WRL::ComPtr<ID3D11Texture2D> d3d11_texture;
+  HRESULT hr = d3d11_device->CreateTexture2D(&desc, nullptr, &d3d11_texture);
+  ASSERT_EQ(hr, S_OK);
+
+  Microsoft::WRL::ComPtr<IDXGIResource1> dxgi_resource;
+  hr = d3d11_texture.As(&dxgi_resource);
+  ASSERT_EQ(hr, S_OK);
+
+  HANDLE shared_handle;
+  hr = dxgi_resource->CreateSharedHandle(
+      nullptr, DXGI_SHARED_RESOURCE_READ | DXGI_SHARED_RESOURCE_WRITE, nullptr,
+      &shared_handle);
+  ASSERT_EQ(hr, S_OK);
+
+  gfx::GpuMemoryBufferHandle gmb_handle{
+      gfx::DXGIHandle(base::win::ScopedHandle(shared_handle))};
+
+  auto mailbox = Mailbox::Generate();
+  auto backing = shared_image_factory_->CreateSharedImage(
+      mailbox,
+      SharedImageInfo(format, size, color_space, kTopLeft_GrSurfaceOrigin,
+                      kPremul_SkAlphaType, usage, "TestLabel"),
+      /*is_thread_safe=*/false, std::move(gmb_handle));
+  ASSERT_NE(backing, nullptr);
+
+  D3DImageBacking* d3d_backing = static_cast<D3DImageBacking*>(backing.get());
+
+  // Create a dummy event handle to use as an invalid fence handle.
+  HANDLE dummy_handle = ::CreateEvent(nullptr, FALSE, FALSE, nullptr);
+  ASSERT_NE(dummy_handle, nullptr);
+  auto invalid_fence = gfx::D3DSharedFence::CreateFromScopedHandle(
+      base::win::ScopedHandle(dummy_handle), gfx::DXGIHandleToken());
+
+  // Inject the invalid fence.
+  d3d_backing->UpdateExternalFence(invalid_fence);
+
+  std::unique_ptr<SharedImageRepresentationFactoryRef> factory_ref =
+      shared_image_manager_.Register(std::move(backing),
+                                     memory_type_tracker_.get());
+
+  // Produce a representation and try to BeginAccess.
+  auto gl_representation =
+      shared_image_representation_factory_->ProduceGLTexturePassthrough(
+          mailbox);
+  ASSERT_TRUE(gl_representation);
+
+  // BeginAccess should fail because GetPendingWaitFences will fail on the
+  // invalid fence.
+  std::unique_ptr<GLTexturePassthroughImageRepresentation::ScopedAccess>
+      scoped_access = gl_representation->BeginScopedAccess(
+          GL_SHARED_IMAGE_ACCESS_MODE_READ_CHROMIUM,
+          SharedImageRepresentation::AllowUnclearedAccess::kYes);
+  EXPECT_FALSE(scoped_access);
+}
+
 // Tests that writing to a Skia representation of a D3DImageBacking created
 // from a shared handle is reflected in a second backing created from the
 // same handle.