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

Commit 32d2b5f9 authored by Chia-I Wu's avatar Chia-I Wu Committed by Luca Stefani
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
Merged-In: I8b361847c44373ce08930d906ec4a995efd1c21b
parent a91970e4

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 & 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 Transform::orientation_flags mRotationFlags;

};

services/surfaceflinger/SurfaceFlinger.cpp

+45 −11
Original line number Diff line number Diff line
@@ -4880,8 +4880,12 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display, sp<GraphicBuf 


    auto renderAreaRotation = fromSurfaceComposerRotation(rotation);



    const sp<const DisplayDevice> device(getDisplayDeviceLocked(display));

    if (CC_UNLIKELY(device == 0)) return BAD_VALUE;

    sp<DisplayDevice> device;

    {

        Mutex::Autolock _l(mStateLock);



        device = getDisplayDeviceLocked(display);

        if (!device) return BAD_VALUE;



        const Rect& dispScissor = device->getScissor();

        if (!dispScissor.isEmpty()) {
@@ -4893,6 +4897,30 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display, sp<GraphicBuf 
            }

        }



        // get screen geometry

        uint32_t width = device->getWidth();

        uint32_t height = device->getHeight();



        if (renderAreaRotation & 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(device, sourceCrop, reqWidth, reqHeight, renderAreaRotation);



    auto traverseLayers = std::bind(std::mem_fn(&SurfaceFlinger::traverseLayersInDisplay), this,
@@ -4999,6 +5027,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) {
@@ -5020,8 +5056,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(),