Add the disallow_unsafe_apis toggle.

Some APIs exposed by Dawn are not expected to be fully secured until
after the first Origin Trial of WebGPU. To prevent their usage we add a
new toggle that will be set by default by Chromium. This toggle throws a
validation error when an unsafe API is used.

Bug: chromium:1138528

Change-Id: I831db70bdac5128ebc32d36d55a0eaefc42c1807
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/31443
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/BindGroupLayout.cpp

@@ -148,6 +148,15 @@ namespace dawn_native {
                 return DAWN_VALIDATION_ERROR("Binding type cannot be used with this visibility.");
             }
 
+            // Dynamic storage buffers aren't bounds checked properly in D3D12. Disallow them as
+            // unsafe until the bounds checks are implemented.
+            if (device->IsToggleEnabled(Toggle::DisallowUnsafeAPIs) &&
+                entry.type == wgpu::BindingType::StorageBuffer && entry.hasDynamicOffset) {
+                return DAWN_VALIDATION_ERROR(
+                    "Dynamic storage buffers are disallowed because they aren't secure yet. See "
+                    "https://crbug.com/dawn/429");
+            }
+
             IncrementBindingCounts(&bindingCounts, entry);
 
             bindingsSet.insert(bindingNumber);

src/dawn_native/ComputePassEncoder.cpp

@@ -70,6 +70,14 @@ namespace dawn_native {
         mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
             DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
 
+            // Indexed dispatches need a compute-shader based validation to check that the dispatch
+            // sizes aren't too big. Disallow them as unsafe until the validation is implemented.
+            if (GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+                return DAWN_VALIDATION_ERROR(
+                    "DispatchIndirect is disallowed because it doesn't validate that the dispatch "
+                    "size is valid yet.");
+            }
+
             if (indirectOffset % 4 != 0) {
                 return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4");
             }

src/dawn_native/RenderEncoderBase.cpp

@@ -112,6 +112,15 @@ namespace dawn_native {
         mEncodingContext->TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
             DAWN_TRY(GetDevice()->ValidateObject(indirectBuffer));
 
+            // Indexed indirect draws need a compute-shader based validation check that the range of
+            // indices is contained inside the index buffer on Metal. Disallow them as unsafe until
+            // the validation is implemented.
+            if (GetDevice()->IsToggleEnabled(Toggle::DisallowUnsafeAPIs)) {
+                return DAWN_VALIDATION_ERROR(
+                    "DrawIndexedIndirect is disallowed because it doesn't validate that the index "
+                    "range is valid yet.");
+            }
+
             if (indirectOffset % 4 != 0) {
                 return DAWN_VALIDATION_ERROR("Indirect offset must be a multiple of 4");
             }

src/dawn_native/Toggles.cpp

@@ -29,8 +29,8 @@ namespace dawn_native {
         using ToggleEnumAndInfoList =
             std::array<ToggleEnumAndInfo, static_cast<size_t>(Toggle::EnumCount)>;
 
-        static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {
-            {{Toggle::EmulateStoreAndMSAAResolve,
+        static constexpr ToggleEnumAndInfoList kToggleNameAndInfoList = {{
+            {Toggle::EmulateStoreAndMSAAResolve,
              {"emulate_store_and_msaa_resolve",
               "Emulate storing into multisampled color attachments and doing MSAA resolve "
               "simultaneously. This workaround is enabled by default on the Metal drivers that do "
@@ -133,7 +133,14 @@ namespace dawn_native {
             {Toggle::MetalEnableVertexPulling,
              {"metal_enable_vertex_pulling",
               "Uses vertex pulling to protect out-of-bounds reads on Metal",
-               "https://crbug.com/dawn/480"}}}};
+              "https://crbug.com/dawn/480"}},
+            {Toggle::DisallowUnsafeAPIs,
+             {"disallow_unsafe_apis",
+              "Produces validation errors on API entry points or parameter combinations that "
+              "aren't considered secure yet.",
+              "http://crbug.com/1138528"}}
+            // Dummy comment to separate the }} so it is clearer what to copy-paste to add a toggle.
+        }};
 
     }  // anonymous namespace
 

src/dawn_native/Toggles.h

@@ -43,6 +43,7 @@ namespace dawn_native {
         UseDXC,
         DisableRobustness,
         MetalEnableVertexPulling,
+        DisallowUnsafeAPIs,
 
         EnumCount,
         InvalidEnum = EnumCount,

src/tests/BUILD.gn

@@ -208,6 +208,7 @@ test("dawn_unittests") {
     "unittests/validation/TextureValidationTests.cpp",
     "unittests/validation/TextureViewValidationTests.cpp",
     "unittests/validation/ToggleValidationTests.cpp",
+    "unittests/validation/UnsafeAPIValidationTests.cpp",
     "unittests/validation/ValidationTest.cpp",
     "unittests/validation/ValidationTest.h",
     "unittests/validation/VertexBufferValidationTests.cpp",

src/tests/unittests/validation/UnsafeAPIValidationTests.cpp

@@ -0,0 +1,169 @@
+// Copyright 2020 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tests/unittests/validation/ValidationTest.h"
+
+#include "utils/ComboRenderBundleEncoderDescriptor.h"
+#include "utils/ComboRenderPipelineDescriptor.h"
+#include "utils/WGPUHelpers.h"
+
+class UnsafeAPIValidationTest : public ValidationTest {
+  protected:
+    wgpu::Device CreateTestDevice() override {
+        dawn_native::DeviceDescriptor descriptor;
+        descriptor.forceEnabledToggles.push_back("disallow_unsafe_apis");
+        return wgpu::Device::Acquire(adapter.CreateDevice(&descriptor));
+    }
+};
+
+// Check that DrawIndexedIndirect is disallowed as part of unsafe APIs.
+TEST_F(UnsafeAPIValidationTest, DrawIndexedIndirectDisallowed) {
+    // Create the index and indirect buffers.
+    wgpu::BufferDescriptor indexBufferDesc;
+    indexBufferDesc.size = 4;
+    indexBufferDesc.usage = wgpu::BufferUsage::Index;
+    wgpu::Buffer indexBuffer = device.CreateBuffer(&indexBufferDesc);
+
+    wgpu::BufferDescriptor indirectBufferDesc;
+    indirectBufferDesc.size = 64;
+    indirectBufferDesc.usage = wgpu::BufferUsage::Indirect;
+    wgpu::Buffer indirectBuffer = device.CreateBuffer(&indirectBufferDesc);
+
+    // The RenderPassDescriptor, RenderBundleDescriptor and pipeline for all sub-tests below.
+    DummyRenderPass renderPass(device);
+
+    utils::ComboRenderBundleEncoderDescriptor bundleDesc = {};
+    bundleDesc.colorFormatsCount = 1;
+    bundleDesc.cColorFormats[0] = renderPass.attachmentFormat;
+
+    utils::ComboRenderPipelineDescriptor desc(device);
+    desc.vertexStage.module = utils::CreateShaderModule(device, utils::SingleShaderStage::Vertex,
+                                                        "#version 450\nvoid main() {}");
+    desc.cFragmentStage.module = utils::CreateShaderModule(
+        device, utils::SingleShaderStage::Fragment, "#version 450\nvoid main() {}");
+    wgpu::RenderPipeline pipeline = device.CreateRenderPipeline(&desc);
+
+    // Control cases: DrawIndirect and DrawIndexed are allowed inside a render pass.
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetPipeline(pipeline);
+
+        pass.SetIndexBufferWithFormat(indexBuffer, wgpu::IndexFormat::Uint32);
+        pass.DrawIndexed(1);
+
+        pass.DrawIndirect(indirectBuffer, 0);
+        pass.EndPass();
+        encoder.Finish();
+    }
+
+    // Control case: DrawIndirect and DrawIndexed are allowed inside a render bundle.
+    {
+        wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&bundleDesc);
+        encoder.SetPipeline(pipeline);
+
+        encoder.SetIndexBufferWithFormat(indexBuffer, wgpu::IndexFormat::Uint32);
+        encoder.DrawIndexed(1);
+
+        encoder.DrawIndirect(indirectBuffer, 0);
+        encoder.Finish();
+    }
+
+    // Error case, DrawIndexedIndirect is disallowed inside a render pass.
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+
+        pass.SetPipeline(pipeline);
+        pass.SetIndexBufferWithFormat(indexBuffer, wgpu::IndexFormat::Uint32);
+        pass.DrawIndexedIndirect(indirectBuffer, 0);
+
+        pass.EndPass();
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+
+    // Error case, DrawIndexedIndirect is disallowed inside a render bundle.
+    {
+        wgpu::RenderBundleEncoder encoder = device.CreateRenderBundleEncoder(&bundleDesc);
+
+        encoder.SetPipeline(pipeline);
+        encoder.SetIndexBufferWithFormat(indexBuffer, wgpu::IndexFormat::Uint32);
+        encoder.DrawIndexedIndirect(indirectBuffer, 0);
+
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+}
+
+// Check that DispatchIndirect is disallowed as part of unsafe APIs.
+TEST_F(UnsafeAPIValidationTest, DispatchIndirectDisallowed) {
+    // Create the index and indirect buffers.
+    wgpu::BufferDescriptor indirectBufferDesc;
+    indirectBufferDesc.size = 64;
+    indirectBufferDesc.usage = wgpu::BufferUsage::Indirect;
+    wgpu::Buffer indirectBuffer = device.CreateBuffer(&indirectBufferDesc);
+
+    // Create the dummy compute pipeline.
+    wgpu::ComputePipelineDescriptor pipelineDesc;
+    pipelineDesc.computeStage.entryPoint = "main";
+    pipelineDesc.computeStage.module = utils::CreateShaderModule(
+        device, utils::SingleShaderStage::Compute, "#version 450\nvoid main() {}");
+    wgpu::ComputePipeline pipeline = device.CreateComputePipeline(&pipelineDesc);
+
+    // Control case: dispatch is allowed.
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
+
+        pass.SetPipeline(pipeline);
+        pass.Dispatch(1, 1, 1);
+
+        pass.EndPass();
+        encoder.Finish();
+    }
+
+    // Error case: dispatch indirect is disallowed.
+    {
+        wgpu::CommandEncoder encoder = device.CreateCommandEncoder();
+        wgpu::ComputePassEncoder pass = encoder.BeginComputePass();
+
+        pass.SetPipeline(pipeline);
+        pass.DispatchIndirect(indirectBuffer, 0);
+
+        pass.EndPass();
+        ASSERT_DEVICE_ERROR(encoder.Finish());
+    }
+}
+
+// Check that dynamic storage buffers are disallowed.
+TEST_F(UnsafeAPIValidationTest, DynamicStorageBuffer) {
+    wgpu::BindGroupLayoutEntry entry;
+    entry.type = wgpu::BindingType::StorageBuffer;
+    entry.visibility = wgpu::ShaderStage::Fragment;
+
+    wgpu::BindGroupLayoutDescriptor desc;
+    desc.entries = &entry;
+    desc.entryCount = 1;
+
+    // Control case: storage buffer without a dynamic offset is allowed.
+    {
+        entry.hasDynamicOffset = false;
+        device.CreateBindGroupLayout(&desc);
+    }
+
+    // Control case: storage buffer with a dynamic offset is disallowed.
+    {
+        entry.hasDynamicOffset = true;
+        ASSERT_DEVICE_ERROR(device.CreateBindGroupLayout(&desc));
+    }
+}