[dawn][wire] Crash client when using objects from another wire

This is of course the incorrect behavior (we are supposed to produce a
"[object] cannot be used with [device]" validation error), but it does
prevent old client objects from misassociating with new server objects.
This avoids any possible renderer-process security issue when that
happens (though none has been identified). (There are not believed to be
any GPU-process problems here since that interface is already fuzzed.)

This crash can only happen after the GPU process has already crashed.
This is of course possible to do intentionally, but the most likely
situation where this happens in the wild is in an application that has
*some* device loss recovery support but it's not well tested, or it
mixes up objects in a way that doesn't break the application.

It's also technically possible for an application to try to rely on
push+popErrorScope to check whether objects are from the same device.
That would work fine in most cases (e.g. device.destroy(), which would
be the typical way for applications to test their recovery behavior), it
would just not work in cases where the GPU process has crashed.

Fixed: 435630456
Bug: 440387003
Change-Id: I73a2d816358bbfe9840cfecfe954c3d3c1b34bcf
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/256757
Commit-Queue: Kai Ninomiya <[email protected]>
Reviewed-by: Corentin Wallez <[email protected]>
Reviewed-by: Loko Kung <[email protected]>

Author: kainino0x

generator/templates/dawn/wire/client/ApiProcs.cpp

@@ -85,7 +85,7 @@ namespace dawn::wire::client {
                                 , {{as_varName(arg.name)}}
                         {%- endfor -%}
                     );
-                    cmd.result = returnObject->GetWireHandle();
+                    cmd.result = returnObject->GetWireHandle(self->GetClient());
                 {% endif %}
 
                 {% for arg in method.arguments %}

generator/templates/dawn/wire/client/ClientBase.h

@@ -47,12 +47,12 @@ namespace dawn::wire::client {
                 if (object == nullptr) {
                     return WireResult::FatalError;
                 }
-                *out = reinterpret_cast<{{as_wireType(type)}}>(object)->GetWireId();
+                *out = reinterpret_cast<{{as_wireType(type)}}>(object)->GetWireHandle(this).id;
                 return WireResult::Success;
             }
             WireResult GetOptionalId({{as_cType(type.name)}} object, ObjectId* out) const final {
                 DAWN_ASSERT(out != nullptr);
-                *out = (object == nullptr ? 0 : reinterpret_cast<{{as_wireType(type)}}>(object)->GetWireId());
+                *out = (object == nullptr ? 0 : reinterpret_cast<{{as_wireType(type)}}>(object)->GetWireHandle(this).id);
                 return WireResult::Success;
             }
         {% endfor %}

generator/templates/dawn/wire/client/ClientHandlers.cpp

@@ -42,7 +42,7 @@ namespace dawn::wire::client {
 
                 {% if member.type.dict_name == "ObjectHandle" %}
                     {{Type}}* {{name}} = Get<{{Type}}>(cmd.{{name}}.id);
-                    if ({{name}} != nullptr && {{name}}->GetWireGeneration() != cmd.{{name}}.generation) {
+                    if ({{name}} != nullptr && {{name}}->GetWireHandle(this).generation != cmd.{{name}}.generation) {
                         {{name}} = nullptr;
                     }
                 {% endif %}

src/dawn/common/Assert.h

@@ -104,7 +104,7 @@
         DAWN_ASSERT(DAWN_ASSERT_LOOP_CONDITION && "Unreachable code hit"); \
         DAWN_BUILTIN_UNREACHABLE();                                        \
     } while (DAWN_ASSERT_LOOP_CONDITION)
-// Release-mode assert (similar to Chromium DAWN_CHECK).
+// Release-mode assert (similar to Chromium CHECK).
 // First does a debug-mode assert for better a better debugging experience, then hard-aborts.
 #define DAWN_CHECK(condition) DAWN_CHECK_CALLSITE_HELPER(__FILE__, __func__, __LINE__, condition)
 

src/dawn/tests/BUILD.gn

@@ -432,6 +432,7 @@ dawn_test("dawn_unittests") {
     "unittests/wire/WireArgumentTests.cpp",
     "unittests/wire/WireBasicTests.cpp",
     "unittests/wire/WireBufferMappingTests.cpp",
+    "unittests/wire/WireConfusionDeathTests.cpp",
     "unittests/wire/WireCreatePipelineAsyncTests.cpp",
     "unittests/wire/WireDeviceLifetimeTests.cpp",
     "unittests/wire/WireDisconnectTests.cpp",

src/dawn/tests/unittests/wire/WireConfusionDeathTests.cpp

@@ -0,0 +1,144 @@
+// Copyright 2025 The Dawn & Tint Authors
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+//    list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+//    this list of conditions and the following disclaimer in the documentation
+//    and/or other materials provided with the distribution.
+//
+// 3. Neither the name of the copyright holder nor the names of its
+//    contributors may be used to endorse or promote products derived from
+//    this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "dawn/tests/unittests/wire/WireTest.h"
+
+#include "dawn/utils/WGPUHelpers.h"
+#include "dawn/wire/WireClient.h"
+
+namespace dawn::wire {
+namespace {
+
+enum class TestState {
+    // Mix the two wires while both clients are connected.
+    Alive,
+    // Disconnect WireTwo before creating an object on it.
+    DisconnectBefore,
+    // Disconnect WireTwo after creating an object on it.
+    DisconnectMid,
+};
+std::ostream& operator<<(std::ostream& stream, TestState flavor) {
+    switch (flavor) {
+        case TestState::Alive:
+            stream << "Alive";
+            break;
+        case TestState::DisconnectBefore:
+            stream << "DisconnectBefore";
+            break;
+        case TestState::DisconnectMid:
+            stream << "DisconnectMid";
+            break;
+    }
+    return stream;
+}
+
+// Two copies of WireTest to set up a second parallel wire.
+// Create two classes that inherit WireTest, so that the test can inherit both
+// of them, to set up two parallel wires to test with.
+class WireOne : public WireTest {};
+class WireTwo : public WireTest {};
+
+// Tests for intentional wire client CHECK crashes when trying to use objects
+// from the wrong wire which would have bogus IDs. (The crash is intended, but
+// in principle we shouldn't actually crash. See crbug.com/440387003.)
+class WireConfusionDeathTest : public WireOne,
+                               public WireTwo,
+                               public ::testing::WithParamInterface<TestState> {
+  protected:
+    void SetUp() override {
+        WireOne::SetUp();
+        WireTwo::SetUp();
+
+        if (GetParam() == TestState::DisconnectBefore) {
+            WireTwo::GetWireClient()->Disconnect();
+        }
+    }
+
+    void TearDown() override {
+        WireTwo::TearDown();
+        WireOne::TearDown();
+    }
+
+    template <typename Lambda>
+    void MaybeDisconnectAndExpectDeath(bool expectDeath, Lambda lambda) {
+        if (GetParam() == TestState::DisconnectMid) {
+            WireTwo::GetWireClient()->Disconnect();
+        }
+        if (expectDeath) {
+#if defined(DAWN_ENABLE_ASSERTS)
+            EXPECT_DEATH(lambda(), "forClient == mClient");
+#else
+            EXPECT_DEATH(lambda(), "");
+#endif
+        } else {
+            lambda();
+        }
+    }
+};
+
+// Test calling queue.WriteBuffer using a buffer from another device.
+TEST_P(WireConfusionDeathTest, WriteBuffer) {
+    wgpu::BufferDescriptor bufDesc{
+        .usage = wgpu::BufferUsage::CopyDst,
+        .size = 4,
+    };
+    wgpu::Buffer two_buf = WireTwo::device.CreateBuffer(&bufDesc);
+
+    MaybeDisconnectAndExpectDeath(true, [&]() {
+        WireOne::queue.WriteBuffer(two_buf, 0, nullptr, 0);  //
+    });
+}
+
+// Test creating a bind group using a layout from an old device.
+TEST_P(WireConfusionDeathTest, NewBindGroupFromOldLayout) {
+    wgpu::BindGroupLayout two_bgl = utils::MakeBindGroupLayout(WireTwo::device, {});
+
+    wgpu::BindGroupDescriptor bgDesc{.layout = two_bgl};
+    MaybeDisconnectAndExpectDeath(true, [&]() {
+        WireOne::device.CreateBindGroup(&bgDesc);  //
+    });
+}
+
+// Test creating a bind group on an old device using a layout from a new device.
+TEST_P(WireConfusionDeathTest, OldBindGroupFromNewLayout) {
+    wgpu::BindGroupLayout one_bgl = utils::MakeBindGroupLayout(WireOne::device, {});
+
+    wgpu::BindGroupDescriptor bgDesc{.layout = one_bgl};
+    // Should not crash if wire two is already disconnected.
+    MaybeDisconnectAndExpectDeath(GetParam() == TestState::Alive,
+                                  [&]() { WireTwo::device.CreateBindGroup(&bgDesc); });
+}
+
+INSTANTIATE_TEST_SUITE_P(,
+                         WireConfusionDeathTest,
+                         ::testing::Values(TestState::Alive,
+                                           TestState::DisconnectBefore,
+                                           TestState::DisconnectMid),
+                         testing::PrintToStringParamName());
+
+}  // anonymous namespace
+}  // namespace dawn::wire

src/dawn/tests/unittests/wire/WireTest.cpp

@@ -49,6 +49,14 @@ using testing::WithArg;
 
 namespace dawn {
 
+namespace {
+// WireTest sets the wire proc table as the global proc table.
+// Tests that use multiple wires may inherit WireTest multiple times (see
+// WireConfusionDeathTest). Refcount how many WireTest instances are running
+// to make sure we don't unset the proc table until the test is done.
+uint32_t sWireProcTableRefCount = 0;
+}  // namespace
+
 WireTest::WireTest() {}
 
 WireTest::~WireTest() {}
@@ -84,7 +92,10 @@ void WireTest::SetUp() {
     mWireClient.reset(new wire::WireClient(clientDesc));
     mS2cBuf->SetHandler(mWireClient.get());
 
-    dawnProcSetProcs(&wire::client::GetProcs());
+    if (sWireProcTableRefCount == 0) {
+        dawnProcSetProcs(&wire::client::GetProcs());
+    }
+    ++sWireProcTableRefCount;
 
     auto reservedInstance = GetWireClient()->ReserveInstance();
     instance = wgpu::Instance::Acquire(reservedInstance.instance);
@@ -193,7 +204,11 @@ void WireTest::TearDown() {
     adapter = nullptr;
     device = nullptr;
     queue = nullptr;
-    dawnProcSetProcs(nullptr);
+
+    --sWireProcTableRefCount;
+    if (sWireProcTableRefCount == 0) {
+        dawnProcSetProcs(nullptr);
+    }
 
     // Derived classes should call the base TearDown() first. The client must
     // be reset before any mocks are deleted.

src/dawn/tests/unittests/wire/WireTest.h

@@ -141,7 +141,7 @@ namespace utils {
 class TerribleCommandBuffer;
 }  // namespace utils
 
-class WireTest : public testing::Test {
+class WireTest : virtual public testing::Test {
   protected:
     WireTest();
     ~WireTest() override;

src/dawn/wire/ChunkedCommandSerializer.cpp

@@ -30,7 +30,9 @@
 namespace dawn::wire {
 
 ChunkedCommandSerializer::ChunkedCommandSerializer(CommandSerializer* serializer)
-    : mSerializer(serializer), mMaxAllocationSize(serializer->GetMaximumAllocationSize()) {}
+    : mSerializer(serializer), mMaxAllocationSize(serializer->GetMaximumAllocationSize()) {
+    DAWN_ASSERT(mMaxAllocationSize > 0);
+}
 
 void ChunkedCommandSerializer::SerializeChunkedCommand(const char* allocatedBuffer,
                                                        size_t remainingSize) {

src/dawn/wire/WireClient.cpp

@@ -82,7 +82,7 @@ void WireClient::Disconnect() {
 
 Handle WireClient::GetWireHandle(WGPUDevice device) const {
     client::Device* wireDevice = client::FromAPI(device);
-    return {wireDevice->GetWireId(), wireDevice->GetWireGeneration()};
+    return wireDevice->GetWireHandle(mImpl.get());
 }
 
 namespace client {