Skip to content

Commit 37973d9

Browse files
committed
Fix ownership for RenderArea in screenshots.
RenderArea was previously owned by captureScreenshot(), and was borrowed by renderScreenImpl(). But renderScreenImpl asynchronously executes screenshot rendering which relies on the render area, which causes a use-after-free. Instead, pass ownership to renderScreenImpl. Bug: 389887557 Change-Id: I37035b34d55f4847db9722371ea492364f7706fe Flag: EXEMPT bug fix Test: courage
1 parent 1da7821 commit 37973d9

3 files changed

Lines changed: 7 additions & 5 deletions

File tree

services/surfaceflinger/SurfaceFlinger.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7575,7 +7575,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
75757575

75767576
if (hdrBuffer && gainmapBuffer) {
75777577
ftl::SharedFuture<FenceResult> hdrRenderFuture =
7578-
renderScreenImpl(renderArea.get(), hdrBuffer, regionSampling, grayscale,
7578+
renderScreenImpl(std::move(renderArea), hdrBuffer, regionSampling, grayscale,
75797579
isProtected, captureResults, displayState, layers);
75807580
captureResults.buffer = buffer->getBuffer();
75817581
captureResults.optionalGainMap = gainmapBuffer->getBuffer();
@@ -7599,7 +7599,7 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
75997599
})
76007600
.share();
76017601
} else {
7602-
renderFuture = renderScreenImpl(renderArea.get(), buffer, regionSampling, grayscale,
7602+
renderFuture = renderScreenImpl(std::move(renderArea), buffer, regionSampling, grayscale,
76037603
isProtected, captureResults, displayState, layers);
76047604
}
76057605

@@ -7620,7 +7620,8 @@ ftl::SharedFuture<FenceResult> SurfaceFlinger::captureScreenshot(
76207620
}
76217621

76227622
ftl::SharedFuture<FenceResult> SurfaceFlinger::renderScreenImpl(
7623-
const RenderArea* renderArea, const std::shared_ptr<renderengine::ExternalTexture>& buffer,
7623+
std::unique_ptr<const RenderArea> renderArea,
7624+
const std::shared_ptr<renderengine::ExternalTexture>& buffer,
76247625
bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults& captureResults,
76257626
const std::optional<OutputCompositionState>& displayState,
76267627
const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers) {

services/surfaceflinger/SurfaceFlinger.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,8 @@ class SurfaceFlinger : public BnSurfaceComposer,
894894
const std::shared_ptr<renderengine::ExternalTexture>& gainmapBuffer = nullptr);
895895

896896
ftl::SharedFuture<FenceResult> renderScreenImpl(
897-
const RenderArea*, const std::shared_ptr<renderengine::ExternalTexture>&,
897+
std::unique_ptr<const RenderArea> renderArea,
898+
const std::shared_ptr<renderengine::ExternalTexture>&,
898899
bool regionSampling, bool grayscale, bool isProtected, ScreenCaptureResults&,
899900
const std::optional<OutputCompositionState>& displayState,
900901
const std::vector<std::pair<Layer*, sp<LayerFE>>>& layers);

services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ class TestableSurfaceFlinger {
473473
auto displayState = std::optional{display->getCompositionDisplay()->getState()};
474474
auto layers = getLayerSnapshotsFn();
475475

476-
return mFlinger->renderScreenImpl(renderArea.get(), buffer, regionSampling,
476+
return mFlinger->renderScreenImpl(std::move(renderArea), buffer, regionSampling,
477477
false /* grayscale */, false /* isProtected */,
478478
captureResults, displayState, layers);
479479
}

0 commit comments

Comments
 (0)