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

Commit 20261cb5 authored by Chia-I Wu's avatar Chia-I Wu
Browse files

surfaceflinger: fix race conditions in captureScreen 

The display was looked up without holding the state lock.  It was
also used (including indirectly from updateDimensions) without
holding the lock either.

Inline updateDimensions in captureScreen with the state lock held.
Stop calling updateDimensions in captureLayers.

Bug: 113041375
Test: take screenshot, rotate screen, screencap
Change-Id: I8b361847c44373ce08930d906ec4a995efd1c21b
parent c80dcbb5

services/surfaceflinger/RenderArea.cpp

+0 −34
Original line number Diff line number Diff line 
#include "RenderArea.h"



#include <gui/LayerState.h>



namespace android {



float RenderArea::getCaptureFillValue(CaptureFill captureFill) {
@@ -13,37 +11,5 @@ float RenderArea::getCaptureFillValue(CaptureFill captureFill) { 
            return 1.0f;

    }

}

/*

 * Checks that the requested width and height are valid and updates them to the render area

 * dimensions if they are set to 0

 */

status_t RenderArea::updateDimensions(int displayRotation) {

    // get screen geometry



    uint32_t width = getWidth();

    uint32_t height = getHeight();



    if (mRotationFlags & ui::Transform::ROT_90) {

        std::swap(width, height);

    }



    if (displayRotation & DisplayState::eOrientationSwapMask) {

        std::swap(width, height);

    }



    if ((mReqWidth > width) || (mReqHeight > height)) {

        ALOGE("size mismatch (%d, %d) > (%d, %d)", mReqWidth, mReqHeight, width, height);

        return BAD_VALUE;

    }



    if (mReqWidth == 0) {

        mReqWidth = width;

    }

    if (mReqHeight == 0) {

        mReqHeight = height;

    }



    return NO_ERROR;

}



} // namespace android

services/surfaceflinger/RenderArea.h

+2 −4
Original line number Diff line number Diff line
@@ -70,11 +70,9 @@ public: 
    // covered by any rendered layer should be filled with this color.

    CaptureFill getCaptureFill() const { return mCaptureFill; };



    status_t updateDimensions(int displayRotation);



private:

    uint32_t mReqWidth;

    uint32_t mReqHeight;

    const uint32_t mReqWidth;

    const uint32_t mReqHeight;

    const CaptureFill mCaptureFill;

    const ui::Transform::orientation_flags mRotationFlags;

};

services/surfaceflinger/SurfaceFlinger.cpp

+45 −11
Original line number Diff line number Diff line
@@ -4919,7 +4919,11 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& displayToken, 


    auto renderAreaRotation = fromSurfaceComposerRotation(rotation);



    const auto display = getDisplayDeviceLocked(displayToken);

    sp<DisplayDevice> display;

    {

        Mutex::Autolock _l(mStateLock);



        display = getDisplayDeviceLocked(displayToken);

        if (!display) return BAD_VALUE;



        const Rect& dispScissor = display->getScissor();
@@ -4932,6 +4936,30 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& displayToken, 
            }

        }



        // get screen geometry

        uint32_t width = display->getWidth();

        uint32_t height = display->getHeight();



        if (renderAreaRotation & ui::Transform::ROT_90) {

            std::swap(width, height);

        }



        if (mPrimaryDisplayOrientation & DisplayState::eOrientationSwapMask) {

            std::swap(width, height);

        }



        if ((reqWidth > width) || (reqHeight > height)) {

            ALOGE("size mismatch (%d, %d) > (%d, %d)", reqWidth, reqHeight, width, height);

        } else {

            if (reqWidth == 0) {

                reqWidth = width;

            }

            if (reqHeight == 0) {

                reqHeight = height;

            }

        }

    }



    DisplayRenderArea renderArea(display, sourceCrop, reqWidth, reqHeight, renderAreaRotation);



    auto traverseLayers = std::bind(std::mem_fn(&SurfaceFlinger::traverseLayersInDisplay), this,
@@ -5040,6 +5068,14 @@ status_t SurfaceFlinger::captureLayers(const sp<IBinder>& layerHandleBinder, 
    int32_t reqWidth = crop.width() * frameScale;

    int32_t reqHeight = crop.height() * frameScale;



    // really small crop or frameScale

    if (reqWidth <= 0) {

        reqWidth = 1;

    }

    if (reqHeight <= 0) {

        reqHeight = 1;

    }



    LayerRenderArea renderArea(this, parent, crop, reqWidth, reqHeight, childrenOnly);



    auto traverseLayers = [parent, childrenOnly](const LayerVector::Visitor& visitor) {
@@ -5061,8 +5097,6 @@ status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea, 
                                             bool useIdentityTransform) {

    ATRACE_CALL();



    renderArea.updateDimensions(mPrimaryDisplayOrientation);



    const uint32_t usage = GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN |

            GRALLOC_USAGE_HW_RENDER | GRALLOC_USAGE_HW_TEXTURE;

    *outBuffer = new GraphicBuffer(renderArea.getReqWidth(), renderArea.getReqHeight(),