From 821f3cac9feafc9979622cce3578425217fc7f63 Mon Sep 17 00:00:00 2001 From: Vasiliy Telezhnikov Date: Mon, 16 Jan 2023 15:41:34 +0000 Subject: [Backport] CVE-2023-0929: Use after free in Vulkan Cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/4143606: CHECK that YUV readback finished synchronously DoReadbackYUVImagePixelsINTERNAL is implemented using skia asynchronous readback and to make it synchronous we use sync cpu and gpu. In some edge cases on linux we saw that doesn't happen if readback triggered vulkan device lost. To avoid use after free, CHECK that callback was actually called. In case of device-lost gpu process will restart anyway, so while this is not proper fix of the problem, it doesn't result in worse user visible behaviour. Bug: 1399742 Change-Id: Ie2172539bb907b9696ef62c70d398aca3967177c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4143606 Reviewed-by: Peng Huang Commit-Queue: Vasiliy Telezhnikov Cr-Commit-Position: refs/heads/main@{#1093064} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/462766 Reviewed-by: Michal Klocek --- chromium/gpu/command_buffer/service/raster_decoder.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/chromium/gpu/command_buffer/service/raster_decoder.cc b/chromium/gpu/command_buffer/service/raster_decoder.cc index 805416b5998..d5388496671 100644 --- a/chromium/gpu/command_buffer/service/raster_decoder.cc +++ b/chromium/gpu/command_buffer/service/raster_decoder.cc @@ -2978,6 +2978,7 @@ void RasterDecoderImpl::DoReadbackARGBImagePixelsINTERNAL( namespace { struct YUVReadbackResult { std::unique_ptr async_result; + bool finished = false; }; void OnReadYUVImagePixelsDone( @@ -2985,6 +2986,7 @@ void OnReadYUVImagePixelsDone( std::unique_ptr async_result) { YUVReadbackResult* context = reinterpret_cast(raw_ctx); context->async_result = std::move(async_result); + context->finished = true; } } // namespace @@ -3158,6 +3160,10 @@ void RasterDecoderImpl::DoReadbackYUVImagePixelsINTERNAL( // asynchronous by removing this flush and implementing a query that can // signal back to client process. gr_context()->flushAndSubmit(true); + + // The call above will sync up gpu and CPU, resulting in callback being run + // during flushAndSubmit. To prevent UAF make sure it indeed happened. + CHECK(yuv_result.finished); if (!yuv_result.async_result) { LOCAL_SET_GL_ERROR(GL_INVALID_OPERATION, "glReadbackYUVImagePixels", "Failed to read pixels from SkImage"); -- cgit v1.2.1