Commit b1b3ccbd authored by Aaron Krajeski's avatar Aaron Krajeski Committed by Commit Bot

Don't create providers if context is lost

CanvasResourceProvider::CreateSharedImageProvider receives a weak pointer
to the ContextProviderWrapper and returns nullptr if it does not exist.

Unfortunately SharedGpuContext::IsGpuCompositingEnabled can re-create
the ContextProviderWrapper after this check happens, leading to potential
use-after-frees.

To me it simply makes the most sense to not create a CRP if context is
lost, as the created provider would be invalid and nullptr would get
returned anyway.

Bug: 1126424
Change-Id: Ic92709d7a38d94e5e7529efac3a09405d64eaa34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2417097Reviewed-by: default avatarJuanmi Huertas <juanmihd@chromium.org>
Reviewed-by: default avatarFernando Serboncini <fserb@chromium.org>
Commit-Queue: Aaron Krajeski <aaronhk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809327}
parent 0b252310
......@@ -890,10 +890,16 @@ CanvasResourceProvider::CreateSharedImageProvider(
RasterMode raster_mode,
bool is_origin_top_left,
uint32_t shared_image_usage_flags) {
if (!context_provider_wrapper)
return nullptr;
if (size.Width() <= 0 || size.Height() <= 0)
// IsGpuCompositingEnabled can re-create the context if it has been lost, do
// this up front so that we can fail early and not expose ourselves to
// use after free bugs (crbug.com/1126424)
const bool is_gpu_compositing_enabled =
SharedGpuContext::IsGpuCompositingEnabled();
// If the context is lost we don't want to re-create it here, the resulting
// resource provider would be invalid anyway
if (!context_provider_wrapper ||
context_provider_wrapper->ContextProvider()->IsContextLost())
return nullptr;
const auto& capabilities =
......@@ -909,7 +915,7 @@ CanvasResourceProvider::CreateSharedImageProvider(
}
const bool is_gpu_memory_buffer_image_allowed =
SharedGpuContext::IsGpuCompositingEnabled() &&
is_gpu_compositing_enabled &&
IsGMBAllowed(size, color_params, capabilities) &&
Platform::Current()->GetGpuMemoryBufferManager();
......@@ -946,6 +952,9 @@ CanvasResourceProvider::CreatePassThroughProvider(
base::WeakPtr<WebGraphicsContext3DProviderWrapper> context_provider_wrapper,
base::WeakPtr<CanvasResourceDispatcher> resource_dispatcher,
bool is_origin_top_left) {
// SharedGpuContext::IsGpuCompositingEnabled can potentially replace the
// context_provider_wrapper, so it's important to call that first as it can
// invalidate the weak pointer.
if (!SharedGpuContext::IsGpuCompositingEnabled() || !context_provider_wrapper)
return nullptr;
......@@ -986,6 +995,9 @@ CanvasResourceProvider::CreateSwapChainProvider(
base::WeakPtr<CanvasResourceDispatcher> resource_dispatcher,
bool is_origin_top_left) {
DCHECK(is_origin_top_left);
// SharedGpuContext::IsGpuCompositingEnabled can potentially replace the
// context_provider_wrapper, so it's important to call that first as it can
// invalidate the weak pointer.
if (!SharedGpuContext::IsGpuCompositingEnabled() || !context_provider_wrapper)
return nullptr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment