diff options
author | Prabir Pradhan <prabirmsp@google.com> | 2022-06-14 14:55:57 +0000 |
---|---|---|
committer | Prabir Pradhan <prabirmsp@google.com> | 2022-06-14 16:27:24 +0000 |
commit | 33da9468354b9b530712d4157298a8e0479cf2e5 (patch) | |
tree | 4ac2655af45af516376ec539e291ab6b521b7546 | |
parent | 8f4e41d679d67436ee3868cab8785a35b3cc613f (diff) | |
download | native-33da9468354b9b530712d4157298a8e0479cf2e5.tar.gz |
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
-rw-r--r-- | libs/gui/tests/EndToEndNativeInputTest.cpp | 49 | ||||
-rw-r--r-- | services/surfaceflinger/Layer.cpp | 34 |
2 files changed, 82 insertions, 1 deletions
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<InputSurface> 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<InputSurface> bgSurface = makeSurface(100, 100); + std::unique_ptr<InputSurface> 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<InputSurface> bgSurface = makeSurface(100, 100); + std::unique_ptr<InputSurface> 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<InputSurface> 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<int32_t>(t.tx() + 0.5); + const auto ty = static_cast<int32_t>(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) { |