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

Commit 59163fdf authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Fix UAF on every resolution switch 

Because resolution switching destroys the DisplayDevice, the commit that
finalizes the modeset dereferences stray pointers to its FrameTargeter.
Fix this by bailing out and committing again.

Fixes: 352197706
Flag: EXEMPT bugfix
Test: No SEGV_MTESERR crash on caiman with eng build
Change-Id: I7ad3b5f44041e5307781c826d11057b12913b58d
parent 8c6afcf1

services/surfaceflinger/SurfaceFlinger.cpp

+10 −5
Original line number Diff line number Diff line
@@ -1444,14 +1444,14 @@ status_t SurfaceFlinger::setActiveModeFromBackdoor(const sp<display::DisplayToke 
    return future.get();

}



void SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) {

bool SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) {

    SFTRACE_NAME(ftl::Concat(__func__, ' ', displayId.value).c_str());



    const auto pendingModeOpt = mDisplayModeController.getPendingMode(displayId);

    if (!pendingModeOpt) {

        // There is no pending mode change. This can happen if the active

        // display changed and the mode change happened on a different display.

        return;

        return true;

    }



    const auto& activeMode = pendingModeOpt->mode;
@@ -1466,8 +1466,8 @@ void SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) { 
        state.physical->activeMode = activeMode.modePtr.get();

        processDisplayChangesLocked();



        // processDisplayChangesLocked will update all necessary components so we're done here.

        return;

        // The DisplayDevice has been destroyed, so abort the commit for the now dead FrameTargeter.

        return false;

    }



    mDisplayModeController.finalizeModeChange(displayId, activeMode.modePtr->getId(),
@@ -1478,6 +1478,8 @@ void SurfaceFlinger::finalizeDisplayModeChange(PhysicalDisplayId displayId) { 
    if (pendingModeOpt->emitEvent) {

        mScheduler->onDisplayModeChanged(displayId, activeMode, /*clearContentRequirements*/ true);

    }



    return true;

}



void SurfaceFlinger::dropModeRequest(PhysicalDisplayId displayId) {
@@ -2659,7 +2661,10 @@ bool SurfaceFlinger::commit(PhysicalDisplayId pacesetterId, 


        for (const auto [displayId, _] : frameTargets) {

            if (mDisplayModeController.isModeSetPending(displayId)) {

                finalizeDisplayModeChange(displayId);

                if (!finalizeDisplayModeChange(displayId)) {

                    mScheduler->scheduleFrame();

                    return false;

                }

            }

        }

    }

services/surfaceflinger/SurfaceFlinger.h

+5 −1
Original line number Diff line number Diff line
@@ -730,7 +730,11 @@ private: 
                                       Fps maxFps);



    void initiateDisplayModeChanges() REQUIRES(kMainThreadContext) REQUIRES(mStateLock);

    void finalizeDisplayModeChange(PhysicalDisplayId) REQUIRES(kMainThreadContext)



    // Returns whether the commit stage should proceed. The return value is ignored when finalizing

    // immediate mode changes, which happen toward the end of the commit stage.

    // TODO: b/355427258 - Remove the return value once the `synced_resolution_switch` flag is live.

    bool finalizeDisplayModeChange(PhysicalDisplayId) REQUIRES(kMainThreadContext)

            REQUIRES(mStateLock);



    void dropModeRequest(PhysicalDisplayId) REQUIRES(kMainThreadContext);