SF: Fix thread safety in ISurfaceComposer APIs

Fix a few cases of ISurfaceComposer functions looking up displays and
calling into HWC without holding mStateLock. Clean up error handling
for consistent messages and accurate status codes.

Bug: 123715322
Test: Boot (no repro, just an audit)
Change-Id: I6432e15c4166b27398137b681b4ea7c056fea5e4
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2979322..cd78c1a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -466,17 +466,17 @@
 }
 
 void SurfaceFlinger::destroyDisplay(const sp<IBinder>& displayToken) {
-    Mutex::Autolock _l(mStateLock);
+    Mutex::Autolock lock(mStateLock);
 
-    ssize_t index = mCurrentState.displays.indexOfKey(displayToken);
+    const ssize_t index = mCurrentState.displays.indexOfKey(displayToken);
     if (index < 0) {
-        ALOGE("destroyDisplay: Invalid display token %p", displayToken.get());
+        ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
         return;
     }
 
     const DisplayDeviceState& state = mCurrentState.displays.valueAt(index);
-    if (!state.isVirtual()) {
-        ALOGE("destroyDisplay called for non-virtual display");
+    if (state.physical) {
+        ALOGE("%s: Invalid operation on physical display", __FUNCTION__);
         return;
     }
     mInterceptor->saveDisplayDeletion(state.sequenceId);
@@ -908,20 +908,29 @@
 }
 
 int SurfaceFlinger::getActiveConfig(const sp<IBinder>& displayToken) {
-    const auto display = getDisplayDevice(displayToken);
-    if (!display) {
-        ALOGE("getActiveConfig: Invalid display token %p", displayToken.get());
-        return BAD_VALUE;
+    int activeConfig;
+    bool isPrimary;
+
+    {
+        Mutex::Autolock lock(mStateLock);
+
+        if (const auto display = getDisplayDeviceLocked(displayToken)) {
+            activeConfig = display->getActiveConfig().value();
+            isPrimary = display->isPrimary();
+        } else {
+            ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+            return NAME_NOT_FOUND;
+        }
     }
 
-    if (display->isPrimary()) {
+    if (isPrimary) {
         std::lock_guard<std::mutex> lock(mActiveConfigLock);
         if (mDesiredActiveConfigChanged) {
             return mDesiredActiveConfig.configId.value();
         }
     }
 
-    return display->getActiveConfig().value();
+    return activeConfig;
 }
 
 void SurfaceFlinger::setDesiredActiveConfig(const ActiveConfigInfo& info) {
@@ -972,17 +981,16 @@
         return BAD_VALUE;
     }
 
-    status_t result = NO_ERROR;
+    status_t result = NAME_NOT_FOUND;
 
     postMessageSync(new LambdaMessage([&]() {
         const auto display = getDisplayDeviceLocked(displayToken);
         if (!display) {
             ALOGE("Attempt to set allowed display configs for invalid display token %p",
                   displayToken.get());
-            result = BAD_VALUE;
         } else if (display->isVirtual()) {
             ALOGW("Attempt to set allowed display configs for virtual display");
-            result = BAD_VALUE;
+            result = INVALID_OPERATION;
         } else {
             HwcConfigIndexType config(mode);
             const auto& refreshRate = mRefreshRateConfigs->getRefreshRateFromConfigId(config);
@@ -1161,7 +1169,7 @@
 
     // Currently we only support this API for a single internal display.
     if (getInternalDisplayToken() != displayToken) {
-        return BAD_VALUE;
+        return NAME_NOT_FOUND;
     }
 
     memcpy(&primaries, &mInternalDisplayPrimaries, sizeof(ui::DisplayPrimaries));
@@ -1169,7 +1177,9 @@
 }
 
 ColorMode SurfaceFlinger::getActiveColorMode(const sp<IBinder>& displayToken) {
-    if (const auto display = getDisplayDevice(displayToken)) {
+    Mutex::Autolock lock(mStateLock);
+
+    if (const auto display = getDisplayDeviceLocked(displayToken)) {
         return display->getCompositionDisplay()->getState().colorMode;
     }
     return static_cast<ColorMode>(BAD_VALUE);
@@ -1185,7 +1195,7 @@
                   decodeColorMode(mode).c_str(), mode, displayToken.get());
             return;
         }
-        const auto display = getDisplayDevice(displayToken);
+        const auto display = getDisplayDeviceLocked(displayToken);
         if (!display) {
             ALOGE("Attempt to set active color mode %s (%d) for invalid display token %p",
                   decodeColorMode(mode).c_str(), mode, displayToken.get());
@@ -1205,16 +1215,14 @@
 
 status_t SurfaceFlinger::getAutoLowLatencyModeSupport(const sp<IBinder>& displayToken,
                                                       bool* outSupport) const {
-    Mutex::Autolock _l(mStateLock);
-
     if (!displayToken) {
-        ALOGE("getAutoLowLatencyModeSupport() failed. Missing display token.");
         return BAD_VALUE;
     }
+
+    Mutex::Autolock lock(mStateLock);
+
     const auto displayId = getPhysicalDisplayIdLocked(displayToken);
     if (!displayId) {
-        ALOGE("getAutoLowLatencyModeSupport() failed. Display id for display token %p not found.",
-              displayToken.get());
         return NAME_NOT_FOUND;
     }
     *outSupport = getHwComposer().hasDisplayCapability(*displayId,
@@ -1223,64 +1231,45 @@
 }
 
 void SurfaceFlinger::setAutoLowLatencyMode(const sp<IBinder>& displayToken, bool on) {
-    postMessageAsync(new LambdaMessage([=] { setAutoLowLatencyModeInternal(displayToken, on); }));
-}
-
-void SurfaceFlinger::setAutoLowLatencyModeInternal(const sp<IBinder>& displayToken, bool on) {
-    if (!displayToken) {
-        ALOGE("setAutoLowLatencyMode() failed. Missing display token.");
-        return;
-    }
-    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
-    if (!displayId) {
-        ALOGE("setAutoLowLatencyMode() failed. Display id for display token %p not found.",
-              displayToken.get());
-        return;
-    }
-
-    getHwComposer().setAutoLowLatencyMode(*displayId, on);
+    postMessageAsync(new LambdaMessage([=] {
+        if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
+            getHwComposer().setAutoLowLatencyMode(*displayId, on);
+        } else {
+            ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+        }
+    }));
 }
 
 status_t SurfaceFlinger::getGameContentTypeSupport(const sp<IBinder>& displayToken,
                                                    bool* outSupport) const {
-    Mutex::Autolock _l(mStateLock);
-
     if (!displayToken) {
-        ALOGE("getGameContentTypeSupport() failed. Missing display token.");
         return BAD_VALUE;
     }
+
+    Mutex::Autolock lock(mStateLock);
+
     const auto displayId = getPhysicalDisplayIdLocked(displayToken);
     if (!displayId) {
-        ALOGE("getGameContentTypeSupport() failed. Display id for display token %p not found.",
-              displayToken.get());
         return NAME_NOT_FOUND;
     }
 
-    std::vector<HWC2::ContentType> outSupportedContentTypes;
-    getHwComposer().getSupportedContentTypes(*displayId, &outSupportedContentTypes);
-    *outSupport = std::find(outSupportedContentTypes.begin(), outSupportedContentTypes.end(),
-                            HWC2::ContentType::Game) != outSupportedContentTypes.end();
+    std::vector<HWC2::ContentType> types;
+    getHwComposer().getSupportedContentTypes(*displayId, &types);
+
+    *outSupport = std::any_of(types.begin(), types.end(),
+                              [](auto type) { return type == HWC2::ContentType::Game; });
     return NO_ERROR;
 }
 
 void SurfaceFlinger::setGameContentType(const sp<IBinder>& displayToken, bool on) {
-    postMessageAsync(new LambdaMessage([=] { setGameContentTypeInternal(displayToken, on); }));
-}
-
-void SurfaceFlinger::setGameContentTypeInternal(const sp<IBinder>& displayToken, bool on) {
-    if (!displayToken) {
-        ALOGE("setGameContentType() failed. Missing display token.");
-        return;
-    }
-    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
-    if (!displayId) {
-        ALOGE("setGameContentType() failed. Display id for display token %p not found.",
-              displayToken.get());
-        return;
-    }
-
-    const HWC2::ContentType type = on ? HWC2::ContentType::Game : HWC2::ContentType::None;
-    getHwComposer().setContentType(*displayId, type);
+    postMessageAsync(new LambdaMessage([=] {
+        if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
+            const auto type = on ? HWC2::ContentType::Game : HWC2::ContentType::None;
+            getHwComposer().setContentType(*displayId, type);
+        } else {
+            ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+        }
+    }));
 }
 
 status_t SurfaceFlinger::clearAnimationFrameStats() {
@@ -1297,15 +1286,15 @@
 
 status_t SurfaceFlinger::getHdrCapabilities(const sp<IBinder>& displayToken,
                                             HdrCapabilities* outCapabilities) const {
-    Mutex::Autolock _l(mStateLock);
+    Mutex::Autolock lock(mStateLock);
 
     const auto display = getDisplayDeviceLocked(displayToken);
     if (!display) {
-        ALOGE("getHdrCapabilities: Invalid display token %p", displayToken.get());
-        return BAD_VALUE;
+        ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+        return NAME_NOT_FOUND;
     }
 
-    // At this point the DisplayDeivce should already be set up,
+    // At this point the DisplayDevice should already be set up,
     // meaning the luminance information is already queried from
     // hardware composer and stored properly.
     const HdrCapabilities& capabilities = display->getHdrCapabilities();
@@ -1348,39 +1337,46 @@
     if (!outFormat || !outDataspace || !outComponentMask) {
         return BAD_VALUE;
     }
-    const auto display = getDisplayDevice(displayToken);
-    if (!display || !display->getId()) {
-        ALOGE("getDisplayedContentSamplingAttributes: Bad display token: %p", display.get());
-        return BAD_VALUE;
+
+    Mutex::Autolock lock(mStateLock);
+
+    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
+    if (!displayId) {
+        return NAME_NOT_FOUND;
     }
-    return getHwComposer().getDisplayedContentSamplingAttributes(*display->getId(), outFormat,
+
+    return getHwComposer().getDisplayedContentSamplingAttributes(*displayId, outFormat,
                                                                  outDataspace, outComponentMask);
 }
 
 status_t SurfaceFlinger::setDisplayContentSamplingEnabled(const sp<IBinder>& displayToken,
                                                           bool enable, uint8_t componentMask,
-                                                          uint64_t maxFrames) const {
-    const auto display = getDisplayDevice(displayToken);
-    if (!display || !display->getId()) {
-        ALOGE("setDisplayContentSamplingEnabled: Bad display token: %p", display.get());
-        return BAD_VALUE;
-    }
+                                                          uint64_t maxFrames) {
+    status_t result = NAME_NOT_FOUND;
 
-    return getHwComposer().setDisplayContentSamplingEnabled(*display->getId(), enable,
-                                                            componentMask, maxFrames);
+    postMessageSync(new LambdaMessage([&] {
+        if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
+            result = getHwComposer().setDisplayContentSamplingEnabled(*displayId, enable,
+                                                                      componentMask, maxFrames);
+        } else {
+            ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+        }
+    }));
+
+    return result;
 }
 
 status_t SurfaceFlinger::getDisplayedContentSample(const sp<IBinder>& displayToken,
                                                    uint64_t maxFrames, uint64_t timestamp,
                                                    DisplayedFrameStats* outStats) const {
-    const auto display = getDisplayDevice(displayToken);
-    if (!display || !display->getId()) {
-        ALOGE("getDisplayContentSample: Bad display token: %p", displayToken.get());
-        return BAD_VALUE;
+    Mutex::Autolock lock(mStateLock);
+
+    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
+    if (!displayId) {
+        return NAME_NOT_FOUND;
     }
 
-    return getHwComposer().getDisplayedContentSample(*display->getId(), maxFrames, timestamp,
-                                                     outStats);
+    return getHwComposer().getDisplayedContentSample(*displayId, maxFrames, timestamp, outStats);
 }
 
 status_t SurfaceFlinger::getProtectedContentSupport(bool* outSupported) const {
@@ -1396,19 +1392,15 @@
     if (!displayToken || !outIsWideColorDisplay) {
         return BAD_VALUE;
     }
-    Mutex::Autolock _l(mStateLock);
+
+    Mutex::Autolock lock(mStateLock);
     const auto display = getDisplayDeviceLocked(displayToken);
     if (!display) {
-        return BAD_VALUE;
+        return NAME_NOT_FOUND;
     }
 
-    // Use hasWideColorDisplay to override built-in display.
-    const auto displayId = display->getId();
-    if (displayId && displayId == getInternalDisplayIdLocked()) {
-        *outIsWideColorDisplay = hasWideColorDisplay;
-        return NO_ERROR;
-    }
-    *outIsWideColorDisplay = display->hasWideColorGamut();
+    *outIsWideColorDisplay =
+            display->isPrimary() ? hasWideColorDisplay : display->hasWideColorGamut();
     return NO_ERROR;
 }
 
@@ -1483,6 +1475,9 @@
     if (!displayToken || !outSupport) {
         return BAD_VALUE;
     }
+
+    Mutex::Autolock lock(mStateLock);
+
     const auto displayId = getPhysicalDisplayIdLocked(displayToken);
     if (!displayId) {
         return NAME_NOT_FOUND;
@@ -1492,16 +1487,22 @@
     return NO_ERROR;
 }
 
-status_t SurfaceFlinger::setDisplayBrightness(const sp<IBinder>& displayToken,
-                                              float brightness) const {
+status_t SurfaceFlinger::setDisplayBrightness(const sp<IBinder>& displayToken, float brightness) {
     if (!displayToken) {
         return BAD_VALUE;
     }
-    const auto displayId = getPhysicalDisplayIdLocked(displayToken);
-    if (!displayId) {
-        return NAME_NOT_FOUND;
-    }
-    return getHwComposer().setDisplayBrightness(*displayId, brightness);
+
+    status_t result = NAME_NOT_FOUND;
+
+    postMessageSync(new LambdaMessage([&] {
+        if (const auto displayId = getPhysicalDisplayIdLocked(displayToken)) {
+            result = getHwComposer().setDisplayBrightness(*displayId, brightness);
+        } else {
+            ALOGE("%s: Invalid display token %p", __FUNCTION__, displayToken.get());
+        }
+    }));
+
+    return result;
 }
 
 status_t SurfaceFlinger::notifyPowerHint(int32_t hintId) {
@@ -4230,7 +4231,7 @@
 
 void SurfaceFlinger::setPowerMode(const sp<IBinder>& displayToken, int mode) {
     postMessageSync(new LambdaMessage([&]() NO_THREAD_SAFETY_ANALYSIS {
-        const auto display = getDisplayDevice(displayToken);
+        const auto display = getDisplayDeviceLocked(displayToken);
         if (!display) {
             ALOGE("Attempt to set power mode %d for invalid display token %p", mode,
                   displayToken.get());
@@ -5343,10 +5344,10 @@
 
     sp<DisplayDevice> display;
     {
-        Mutex::Autolock _l(mStateLock);
+        Mutex::Autolock lock(mStateLock);
 
         display = getDisplayDeviceLocked(displayToken);
-        if (!display) return BAD_VALUE;
+        if (!display) return NAME_NOT_FOUND;
 
         // set the requested width/height to the logical display viewport size
         // by default
@@ -5422,10 +5423,10 @@
     uint32_t height;
     ui::Transform::RotationFlags captureOrientation;
     {
-        Mutex::Autolock _l(mStateLock);
+        Mutex::Autolock lock(mStateLock);
         display = getDisplayByIdOrLayerStack(displayOrLayerStack);
         if (!display) {
-            return BAD_VALUE;
+            return NAME_NOT_FOUND;
         }
 
         width = uint32_t(display->getViewport().width());
@@ -5560,7 +5561,7 @@
     std::unordered_set<sp<Layer>, ISurfaceComposer::SpHash<Layer>> excludeLayers;
     Rect displayViewport;
     {
-        Mutex::Autolock _l(mStateLock);
+        Mutex::Autolock lock(mStateLock);
 
         parent = fromHandle(layerHandleBinder);
         if (parent == nullptr || parent->isRemovedFromCurrentState()) {
@@ -5604,9 +5605,9 @@
             }
         }
 
-        auto display = getDisplayByLayerStack(parent->getLayerStack());
+        const auto display = getDisplayByLayerStack(parent->getLayerStack());
         if (!display) {
-            return BAD_VALUE;
+            return NAME_NOT_FOUND;
         }
 
         displayViewport = display->getViewport();
@@ -5989,17 +5990,16 @@
         return BAD_VALUE;
     }
 
-    status_t result = NO_ERROR;
+    status_t result = NAME_NOT_FOUND;
 
     postMessageSync(new LambdaMessage([&]() {
         const auto display = getDisplayDeviceLocked(displayToken);
         if (!display) {
-            result = BAD_VALUE;
             ALOGE("Attempt to set desired display configs for invalid display token %p",
                   displayToken.get());
         } else if (display->isVirtual()) {
-            result = BAD_VALUE;
             ALOGW("Attempt to set desired display configs for virtual display");
+            result = INVALID_OPERATION;
         } else {
             result = setDesiredDisplayConfigSpecsInternal(display,
                                                           scheduler::RefreshRateConfigs::
@@ -6038,12 +6038,11 @@
         *outMaxRefreshRate = policy.maxRefreshRate;
         return NO_ERROR;
     } else if (display->isVirtual()) {
-        return BAD_VALUE;
+        return INVALID_OPERATION;
     } else {
         const auto displayId = display->getId();
-        if (!displayId) {
-            return BAD_VALUE;
-        }
+        LOG_FATAL_IF(!displayId);
+
         *outDefaultConfig = getHwComposer().getActiveConfigIndex(*displayId);
         auto vsyncPeriod = getHwComposer().getActiveConfig(*displayId)->getVsyncPeriod();
         *outMinRefreshRate = 1e9f / vsyncPeriod;