Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a724230a authored by Robert Carr's avatar Robert Carr Committed by Rob Carr
Browse files

fillInputInfo: Guard against integer overflow. 

We have a bunch of bug-reports where we are seeing an abort inside
Rect::inset during/after rotation animations. I can't think of any
reason Rect::inset would abort besides integer overflow. I'm also not
sure what exact state the layer was in, because at the time of the
report SurfaceFlinger has already crashed. I also can't reproduce. So
it's a little difficult to figure out how we actually ended up
overflowing here. I guess some math must have gone very wrong during
the rotation animation calculation, though apparently on a non-visible layer
or only for one frame or something, because no-one reports any visible
issues. For lack of anything better to do now, I'm going to sanitize
the function against integer overflow on the theory that SurfaceFlinger
should at least be resistant against invalid input. I also wonder if
something has gone wrong in the SF math, because we may be applying
a non rect preserving transform to the rect...but when I work through
examples I can't see exactly how it would lead to overflow.

Bug: 166217947
Test: Existing tests pass
Change-Id: I7d989d5be77595f88b2c52fa69c3ed29d17d3931
parent 565d089d

services/surfaceflinger/Layer.cpp

+9 −1
Original line number Original line Diff line number Diff line
@@ -2420,7 +2420,15 @@ InputWindowInfo Layer::fillInputInfo() { 
    xSurfaceInset = (xSurfaceInset >= 0) ? std::min(xSurfaceInset, layerBounds.getWidth() / 2) : 0;

    xSurfaceInset = (xSurfaceInset >= 0) ? std::min(xSurfaceInset, layerBounds.getWidth() / 2) : 0;

    ySurfaceInset = (ySurfaceInset >= 0) ? std::min(ySurfaceInset, layerBounds.getHeight() / 2) : 0;

    ySurfaceInset = (ySurfaceInset >= 0) ? std::min(ySurfaceInset, layerBounds.getHeight() / 2) : 0;





    layerBounds.inset(xSurfaceInset, ySurfaceInset, xSurfaceInset, ySurfaceInset);

    // inset while protecting from overflow TODO(b/161235021): What is going wrong

    // in the overflow scenario?

    {

    int32_t tmp;

    if (!__builtin_add_overflow(layerBounds.left, xSurfaceInset, &tmp)) layerBounds.left = tmp;

    if (!__builtin_sub_overflow(layerBounds.right, xSurfaceInset, &tmp)) layerBounds.right = tmp;

    if (!__builtin_add_overflow(layerBounds.top, ySurfaceInset, &tmp)) layerBounds.top = tmp;

    if (!__builtin_sub_overflow(layerBounds.bottom, ySurfaceInset, &tmp)) layerBounds.bottom = tmp;

    }





    // Input coordinate should match the layer bounds.

    // Input coordinate should match the layer bounds.

    info.frameLeft = layerBounds.left;

    info.frameLeft = layerBounds.left;