From 33da9468354b9b530712d4157298a8e0479cf2e5 Mon Sep 17 00:00:00 2001 From: Prabir Pradhan Date: Tue, 14 Jun 2022 14:55:57 +0000 Subject: Layer: Guard against Region offset overflows When applying a transform to a Region, if the Region is translated to bounds that overlow the int32_t type, the process crashes. For now, we guard against process crashes by manually applying the offset to the Region and checking for overflows. If we detect an overflow in one of the Rects in a Region, we remove the Rect from the resulting Region. Bug: 234247210 Test: atest libgui_test Change-Id: Icd47539accae2e59a7dbd9c9621201bd040fc402 --- libs/gui/tests/EndToEndNativeInputTest.cpp | 49 ++++++++++++++++++++++++++++++ services/surfaceflinger/Layer.cpp | 34 ++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/libs/gui/tests/EndToEndNativeInputTest.cpp b/libs/gui/tests/EndToEndNativeInputTest.cpp index 262987fd27..c6cdeb7706 100644 --- a/libs/gui/tests/EndToEndNativeInputTest.cpp +++ b/libs/gui/tests/EndToEndNativeInputTest.cpp @@ -566,6 +566,55 @@ TEST_F(InputSurfacesTest, input_respects_scaled_surface_insets_overflow) { bgSurface->expectTap(12, 24); } +TEST_F(InputSurfacesTest, touchable_region) { + std::unique_ptr surface = makeSurface(100, 100); + + surface->mInputInfo.touchableRegion.set(Rect{19, 29, 21, 31}); + + surface->showAt(11, 22); + + // A tap within the surface but outside the touchable region should not be sent to the surface. + injectTap(20, 30); + EXPECT_EQ(surface->consumeEvent(200 /*timeoutMs*/), nullptr); + + injectTap(31, 52); + surface->expectTap(20, 30); +} + +TEST_F(InputSurfacesTest, input_respects_touchable_region_offset_overflow) { + std::unique_ptr bgSurface = makeSurface(100, 100); + std::unique_ptr fgSurface = makeSurface(100, 100); + bgSurface->showAt(100, 100); + + // Set the touchable region to the values at the limit of its corresponding type. + // Since the surface is offset from the origin, the touchable region will be transformed into + // display space, which would trigger an overflow or an underflow. Ensure that we are protected + // against such a situation. + fgSurface->mInputInfo.touchableRegion.orSelf(Rect{INT32_MIN, INT32_MIN, INT32_MAX, INT32_MAX}); + + fgSurface->showAt(100, 100); + + // Expect no crash for overflow. The overflowed touchable region is ignored, so the background + // surface receives touch. + injectTap(112, 124); + bgSurface->expectTap(12, 24); +} + +TEST_F(InputSurfacesTest, input_respects_scaled_touchable_region_overflow) { + std::unique_ptr bgSurface = makeSurface(100, 100); + std::unique_ptr fgSurface = makeSurface(100, 100); + bgSurface->showAt(0, 0); + + fgSurface->mInputInfo.touchableRegion.orSelf(Rect{INT32_MIN, INT32_MIN, INT32_MAX, INT32_MAX}); + fgSurface->showAt(0, 0); + + fgSurface->doTransaction([&](auto &t, auto &sc) { t.setMatrix(sc, 2.0, 0, 0, 2.0); }); + + // Expect no crash for overflow. + injectTap(12, 24); + fgSurface->expectTap(6, 12); +} + // Ensure we ignore transparent region when getting screen bounds when positioning input frame. TEST_F(InputSurfacesTest, input_ignores_transparent_region) { std::unique_ptr surface = makeSurface(100, 100); diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp index 6ed4a94f25..be16942d40 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -2194,6 +2194,37 @@ Rect Layer::getInputBounds() const { return getCroppedBufferSize(getDrawingState()); } +// Applies the given transform to the region, while protecting against overflows caused by any +// offsets. If applying the offset in the transform to any of the Rects in the region would result +// in an overflow, they are not added to the output Region. +static Region transformTouchableRegionSafely(const ui::Transform& t, const Region& r, + const std::string& debugWindowName) { + // Round the translation using the same rounding strategy used by ui::Transform. + const auto tx = static_cast(t.tx() + 0.5); + const auto ty = static_cast(t.ty() + 0.5); + + ui::Transform transformWithoutOffset = t; + transformWithoutOffset.set(0.f, 0.f); + + const Region transformed = transformWithoutOffset.transform(r); + + // Apply the translation to each of the Rects in the region while discarding any that overflow. + Region ret; + for (const auto& rect : transformed) { + Rect newRect; + if (__builtin_add_overflow(rect.left, tx, &newRect.left) || + __builtin_add_overflow(rect.top, ty, &newRect.top) || + __builtin_add_overflow(rect.right, tx, &newRect.right) || + __builtin_add_overflow(rect.bottom, ty, &newRect.bottom)) { + ALOGE("Applying transform to touchable region of window '%s' resulted in an overflow.", + debugWindowName.c_str()); + continue; + } + ret.orSelf(newRect); + } + return ret; +} + void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& screenToDisplay) { Rect tmpBounds = getInputBounds(); if (!tmpBounds.isValid()) { @@ -2256,7 +2287,8 @@ void Layer::fillInputFrameInfo(WindowInfo& info, const ui::Transform& screenToDi info.transform = inputToDisplay.inverse(); // The touchable region is specified in the input coordinate space. Change it to display space. - info.touchableRegion = inputToDisplay.transform(info.touchableRegion); + info.touchableRegion = + transformTouchableRegionSafely(inputToDisplay, info.touchableRegion, mName); } void Layer::fillTouchOcclusionMode(WindowInfo& info) { -- cgit v1.2.3