From c3ecb5a77c063f898644fb2fbe51ccf071b827e6 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Mon, 10 Dec 2018 10:03:08 +0000 Subject: [PATCH] Temporarily add nullptr checks in frontend The fuzzer is able to trigger nullptr reads by failing to create objects and then using the resulting nullptr in other operations. The proper fix is to implement WebGPU error handling where creation failure returns a valid but "error" object. However implementing this error handling is a lot of work, so in the meantime we use nullptr checks in relevant places to fix the fuzzer issue. These checks will be removed once the error handling is changed. BUG=dawn:8 Change-Id: I6777a7fa40383b3d2235e071c3f0109de7605a22 Reviewed-on: https://dawn-review.googlesource.com/c/2565 Reviewed-by: Kai Ninomiya Reviewed-by: Corentin Wallez Commit-Queue: Corentin Wallez --- src/dawn_native/BindGroup.cpp | 4 ++ src/dawn_native/CommandBuffer.cpp | 37 +++++++++++++++++++ src/dawn_native/ComputePassEncoder.cpp | 5 +++ src/dawn_native/ComputePipeline.cpp | 8 ++++ src/dawn_native/Pipeline.cpp | 10 +++++ src/dawn_native/ProgrammablePassEncoder.cpp | 5 +++ src/dawn_native/Queue.cpp | 8 ++++ src/dawn_native/RenderPassDescriptor.cpp | 10 +++++ src/dawn_native/RenderPassEncoder.cpp | 17 +++++++++ src/dawn_native/RenderPipeline.cpp | 14 +++++++ .../RenderPipelineValidationTests.cpp | 2 +- 11 files changed, 119 insertions(+), 1 deletion(-) diff --git a/src/dawn_native/BindGroup.cpp b/src/dawn_native/BindGroup.cpp index 3228922fc7..5a5a1778f7 100644 --- a/src/dawn_native/BindGroup.cpp +++ b/src/dawn_native/BindGroup.cpp @@ -87,6 +87,10 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } + if (descriptor->layout == nullptr) { + return DAWN_VALIDATION_ERROR("layout cannot be null"); + } + const BindGroupLayoutBase::LayoutBindingInfo& layoutInfo = descriptor->layout->GetBindingInfo(); diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp index 88a013e15a..2f12197f0a 100644 --- a/src/dawn_native/CommandBuffer.cpp +++ b/src/dawn_native/CommandBuffer.cpp @@ -621,6 +621,11 @@ namespace dawn_native { return nullptr; } + if (info == nullptr) { + HandleError("RenderPassDescriptor cannot be null"); + return nullptr; + } + BeginRenderPassCmd* cmd = mAllocator.Allocate(Command::BeginRenderPass); new (cmd) BeginRenderPassCmd; cmd->info = info; @@ -638,6 +643,16 @@ namespace dawn_native { return; } + if (source == nullptr) { + HandleError("Source cannot be null"); + return; + } + + if (destination == nullptr) { + HandleError("Destination cannot be null"); + return; + } + CopyBufferToBufferCmd* copy = mAllocator.Allocate(Command::CopyBufferToBuffer); new (copy) CopyBufferToBufferCmd; @@ -654,6 +669,17 @@ namespace dawn_native { if (ConsumedError(ValidateCanRecordTopLevelCommands())) { return; } + + if (source->buffer == nullptr) { + HandleError("Buffer cannot be null"); + return; + } + + if (destination->texture == nullptr) { + HandleError("Texture cannot be null"); + return; + } + CopyBufferToTextureCmd* copy = mAllocator.Allocate(Command::CopyBufferToTexture); new (copy) CopyBufferToTextureCmd; @@ -677,6 +703,17 @@ namespace dawn_native { if (ConsumedError(ValidateCanRecordTopLevelCommands())) { return; } + + if (source->texture == nullptr) { + HandleError("Texture cannot be null"); + return; + } + + if (destination->buffer == nullptr) { + HandleError("Buffer cannot be null"); + return; + } + CopyTextureToBufferCmd* copy = mAllocator.Allocate(Command::CopyTextureToBuffer); new (copy) CopyTextureToBufferCmd; diff --git a/src/dawn_native/ComputePassEncoder.cpp b/src/dawn_native/ComputePassEncoder.cpp index 77774db3d5..21433007e3 100644 --- a/src/dawn_native/ComputePassEncoder.cpp +++ b/src/dawn_native/ComputePassEncoder.cpp @@ -43,6 +43,11 @@ namespace dawn_native { return; } + if (pipeline == nullptr) { + mTopLevelBuilder->HandleError("Pipeline cannot be null"); + return; + } + SetComputePipelineCmd* cmd = mAllocator->Allocate(Command::SetComputePipeline); new (cmd) SetComputePipelineCmd; diff --git a/src/dawn_native/ComputePipeline.cpp b/src/dawn_native/ComputePipeline.cpp index 6e961f19f0..fb7c8618e8 100644 --- a/src/dawn_native/ComputePipeline.cpp +++ b/src/dawn_native/ComputePipeline.cpp @@ -24,6 +24,14 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("nextInChain must be nullptr"); } + if (descriptor->module == nullptr) { + return DAWN_VALIDATION_ERROR("module cannot be null"); + } + + if (descriptor->layout == nullptr) { + return DAWN_VALIDATION_ERROR("layout cannot be null"); + } + if (descriptor->entryPoint != std::string("main")) { return DAWN_VALIDATION_ERROR("Currently the entry point has to be main()"); } diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp index 301ef0f6f3..afa388b0a2 100644 --- a/src/dawn_native/Pipeline.cpp +++ b/src/dawn_native/Pipeline.cpp @@ -106,12 +106,22 @@ namespace dawn_native { } void PipelineBuilder::SetLayout(PipelineLayoutBase* layout) { + if (layout == nullptr) { + mParentBuilder->HandleError("Layout must not be null"); + return; + } + mLayout = layout; } void PipelineBuilder::SetStage(dawn::ShaderStage stage, ShaderModuleBase* module, const char* entryPoint) { + if (module == nullptr) { + mParentBuilder->HandleError("Module must not be null"); + return; + } + if (entryPoint != std::string("main")) { mParentBuilder->HandleError("Currently the entry point has to be main()"); return; diff --git a/src/dawn_native/ProgrammablePassEncoder.cpp b/src/dawn_native/ProgrammablePassEncoder.cpp index f4c85619e7..355e9f398e 100644 --- a/src/dawn_native/ProgrammablePassEncoder.cpp +++ b/src/dawn_native/ProgrammablePassEncoder.cpp @@ -38,6 +38,11 @@ namespace dawn_native { return; } + if (group == nullptr) { + mTopLevelBuilder->HandleError("BindGroup cannot be null"); + return; + } + if (groupIndex >= kMaxBindGroups) { mTopLevelBuilder->HandleError("Setting bind group over the max"); return; diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp index 6ed84ad6fc..426c963c10 100644 --- a/src/dawn_native/Queue.cpp +++ b/src/dawn_native/Queue.cpp @@ -47,6 +47,10 @@ namespace dawn_native { MaybeError QueueBase::ValidateSubmit(uint32_t numCommands, CommandBufferBase* const* commands) { for (uint32_t i = 0; i < numCommands; ++i) { + if (commands[i] == nullptr) { + return DAWN_VALIDATION_ERROR("Command buffers cannot be null"); + } + const CommandBufferResourceUsage& usages = commands[i]->GetResourceUsages(); for (const PassResourceUsage& passUsages : usages.perPass) { @@ -70,6 +74,10 @@ namespace dawn_native { } MaybeError QueueBase::ValidateSignal(const FenceBase* fence, uint64_t signalValue) { + if (fence == nullptr) { + return DAWN_VALIDATION_ERROR("Fence cannot be null"); + } + if (signalValue <= fence->GetSignaledValue()) { return DAWN_VALIDATION_ERROR("Signal value less than or equal to fence signaled value"); } diff --git a/src/dawn_native/RenderPassDescriptor.cpp b/src/dawn_native/RenderPassDescriptor.cpp index 22f3b9f07c..b99931bad0 100644 --- a/src/dawn_native/RenderPassDescriptor.cpp +++ b/src/dawn_native/RenderPassDescriptor.cpp @@ -152,6 +152,11 @@ namespace dawn_native { return; } + if (textureView == nullptr) { + HandleError("Texture view cannot be nullptr"); + return; + } + if (!IsColorRenderableTextureFormat(textureView->GetFormat())) { HandleError( "The format of the texture view used as color attachment is not color renderable"); @@ -186,6 +191,11 @@ namespace dawn_native { void RenderPassDescriptorBuilder::SetDepthStencilAttachment(TextureViewBase* textureView, dawn::LoadOp depthLoadOp, dawn::LoadOp stencilLoadOp) { + if (textureView == nullptr) { + HandleError("Texture view cannot be nullptr"); + return; + } + if (!TextureFormatHasDepthOrStencil(textureView->GetFormat())) { HandleError( "The format of the texture view used as depth stencil attachment is not a depth " diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp index ad94bc1bc4..2a8912d68b 100644 --- a/src/dawn_native/RenderPassEncoder.cpp +++ b/src/dawn_native/RenderPassEncoder.cpp @@ -66,6 +66,11 @@ namespace dawn_native { return; } + if (pipeline == nullptr) { + mTopLevelBuilder->HandleError("Pipeline cannot be null"); + return; + } + SetRenderPipelineCmd* cmd = mAllocator->Allocate(Command::SetRenderPipeline); new (cmd) SetRenderPipelineCmd; @@ -117,6 +122,11 @@ namespace dawn_native { return; } + if (buffer == nullptr) { + mTopLevelBuilder->HandleError("Buffer cannot be null"); + return; + } + SetIndexBufferCmd* cmd = mAllocator->Allocate(Command::SetIndexBuffer); new (cmd) SetIndexBufferCmd; cmd->buffer = buffer; @@ -131,6 +141,13 @@ namespace dawn_native { return; } + for (size_t i = 0; i < count; ++i) { + if (buffers[i] == nullptr) { + mTopLevelBuilder->HandleError("Buffers cannot be null"); + return; + } + } + SetVertexBuffersCmd* cmd = mAllocator->Allocate(Command::SetVertexBuffers); new (cmd) SetVertexBuffersCmd; diff --git a/src/dawn_native/RenderPipeline.cpp b/src/dawn_native/RenderPipeline.cpp index 047ad2570b..70e300652b 100644 --- a/src/dawn_native/RenderPipeline.cpp +++ b/src/dawn_native/RenderPipeline.cpp @@ -189,6 +189,10 @@ namespace dawn_native { HandleError("Attachment index out of bounds"); return; } + if (blendState == nullptr) { + HandleError("Blend state must not be null"); + return; + } if (mBlendStatesSet[attachmentSlot]) { HandleError("Attachment blend state already set"); return; @@ -199,6 +203,11 @@ namespace dawn_native { } void RenderPipelineBuilder::SetDepthStencilState(DepthStencilStateBase* depthStencilState) { + if (depthStencilState == nullptr) { + HandleError("Depth stencil state must not be null"); + return; + } + mDepthStencilState = depthStencilState; } @@ -212,6 +221,11 @@ namespace dawn_native { } void RenderPipelineBuilder::SetInputState(InputStateBase* inputState) { + if (inputState == nullptr) { + HandleError("Input state must not be null"); + return; + } + mInputState = inputState; } diff --git a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp index 0da72080da..a3d61c1b87 100644 --- a/src/tests/unittests/validation/RenderPipelineValidationTests.cpp +++ b/src/tests/unittests/validation/RenderPipelineValidationTests.cpp @@ -24,7 +24,7 @@ class RenderPipelineValidationTest : public ValidationTest { renderpass = CreateSimpleRenderPass(); - dawn::PipelineLayout pl = utils::MakeBasicPipelineLayout(device, nullptr); + pipelineLayout = utils::MakeBasicPipelineLayout(device, nullptr); inputState = device.CreateInputStateBuilder().GetResult();