diff options
author | 2022-10-05 11:42:30 -0700 | |
---|---|---|
committer | 2022-12-07 20:07:42 +0000 | |
commit | 6bb12824db3c540775b2b737331ed3f448a50e2e (patch) | |
tree | 80fe473715979ca49138c3666479db15586700d1 | |
parent | a84abc85435e3b3519e77cbebf396bf3e2d14d6e (diff) |
Add security check to getPhysicalDisplayToken binder function.
- There is a possible way to take over the screen display and swap the
display content due to a missing permission check.
- Add a short-term fix for WCG checking failure because of new
permission check added to SF::getPhysicalDisplayToken: change two
function signatures (getStaticDisplayInfo and getDynamicDisplayInfo).
- To make short-term fix workable, split getDynamicDisplayInfo binder
call into two, one is to take display id, one is to take display token
as old codes show to avoid huge modification on other callees.
Bug: 248031255
Test: test using displaytoken app manually on the phone, test shell
screenrecord during using displaytoken; atest
android.hardware.camera2.cts.FastBasicsTest
Change-Id: Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df
-rw-r--r-- | libs/gui/LayerState.cpp | 21 | ||||
-rw-r--r-- | libs/gui/SurfaceComposerClient.cpp | 117 | ||||
-rw-r--r-- | libs/gui/aidl/android/gui/ISurfaceComposer.aidl | 6 | ||||
-rw-r--r-- | libs/gui/fuzzer/libgui_fuzzer_utils.h | 6 | ||||
-rw-r--r-- | libs/gui/include/gui/LayerState.h | 1 | ||||
-rw-r--r-- | libs/gui/include/gui/SurfaceComposerClient.h | 12 | ||||
-rw-r--r-- | libs/gui/tests/Surface_test.cpp | 11 | ||||
-rw-r--r-- | libs/nativedisplay/ADisplay.cpp | 23 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.cpp | 200 | ||||
-rw-r--r-- | services/surfaceflinger/SurfaceFlinger.h | 21 | ||||
-rw-r--r-- | services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h | 18 | ||||
-rw-r--r-- | services/surfaceflinger/tests/Credentials_test.cpp | 37 | ||||
-rw-r--r-- | services/surfaceflinger/tests/DisplayConfigs_test.cpp | 6 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/SurfaceFlinger_ExcludeDolbyVisionTest.cpp | 6 | ||||
-rw-r--r-- | services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h | 6 |
15 files changed, 298 insertions, 193 deletions
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index 95962afda1..59b62fe58c 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -388,6 +388,27 @@ void DisplayState::merge(const DisplayState& other) { } } +void DisplayState::sanitize(int32_t permissions) { + if (what & DisplayState::eLayerStackChanged) { + if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) { + what &= ~DisplayState::eLayerStackChanged; + ALOGE("Stripped attempt to set eLayerStackChanged in sanitize"); + } + } + if (what & DisplayState::eDisplayProjectionChanged) { + if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) { + what &= ~DisplayState::eDisplayProjectionChanged; + ALOGE("Stripped attempt to set eDisplayProjectionChanged in sanitize"); + } + } + if (what & DisplayState::eSurfaceChanged) { + if (!(permissions & layer_state_t::Permission::ACCESS_SURFACE_FLINGER)) { + what &= ~DisplayState::eSurfaceChanged; + ALOGE("Stripped attempt to set eSurfaceChanged in sanitize"); + } + } +} + void layer_state_t::sanitize(int32_t permissions) { // TODO: b/109894387 // diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp index 325c294762..abe5d35d0e 100644 --- a/libs/gui/SurfaceComposerClient.cpp +++ b/libs/gui/SurfaceComposerClient.cpp @@ -2260,12 +2260,12 @@ status_t SurfaceComposerClient::getDisplayState(const sp<IBinder>& display, return statusTFromBinderStatus(status); } -status_t SurfaceComposerClient::getStaticDisplayInfo(const sp<IBinder>& display, +status_t SurfaceComposerClient::getStaticDisplayInfo(int64_t displayId, ui::StaticDisplayInfo* outInfo) { using Tag = android::gui::DeviceProductInfo::ManufactureOrModelDate::Tag; gui::StaticDisplayInfo ginfo; binder::Status status = - ComposerServiceAIDL::getComposerService()->getStaticDisplayInfo(display, &ginfo); + ComposerServiceAIDL::getComposerService()->getStaticDisplayInfo(displayId, &ginfo); if (status.isOk()) { // convert gui::StaticDisplayInfo to ui::StaticDisplayInfo outInfo->connectionType = static_cast<ui::DisplayConnectionType>(ginfo.connectionType); @@ -2309,56 +2309,74 @@ status_t SurfaceComposerClient::getStaticDisplayInfo(const sp<IBinder>& display, return statusTFromBinderStatus(status); } -status_t SurfaceComposerClient::getDynamicDisplayInfo(const sp<IBinder>& display, - ui::DynamicDisplayInfo* outInfo) { +void SurfaceComposerClient::getDynamicDisplayInfoInternal(gui::DynamicDisplayInfo& ginfo, + ui::DynamicDisplayInfo*& outInfo) { + // convert gui::DynamicDisplayInfo to ui::DynamicDisplayInfo + outInfo->supportedDisplayModes.clear(); + outInfo->supportedDisplayModes.reserve(ginfo.supportedDisplayModes.size()); + for (const auto& mode : ginfo.supportedDisplayModes) { + ui::DisplayMode outMode; + outMode.id = mode.id; + outMode.resolution.width = mode.resolution.width; + outMode.resolution.height = mode.resolution.height; + outMode.xDpi = mode.xDpi; + outMode.yDpi = mode.yDpi; + outMode.refreshRate = mode.refreshRate; + outMode.appVsyncOffset = mode.appVsyncOffset; + outMode.sfVsyncOffset = mode.sfVsyncOffset; + outMode.presentationDeadline = mode.presentationDeadline; + outMode.group = mode.group; + std::transform(mode.supportedHdrTypes.begin(), mode.supportedHdrTypes.end(), + std::back_inserter(outMode.supportedHdrTypes), + [](const int32_t& value) { return static_cast<ui::Hdr>(value); }); + outInfo->supportedDisplayModes.push_back(outMode); + } + + outInfo->activeDisplayModeId = ginfo.activeDisplayModeId; + outInfo->renderFrameRate = ginfo.renderFrameRate; + + outInfo->supportedColorModes.clear(); + outInfo->supportedColorModes.reserve(ginfo.supportedColorModes.size()); + for (const auto& cmode : ginfo.supportedColorModes) { + outInfo->supportedColorModes.push_back(static_cast<ui::ColorMode>(cmode)); + } + + outInfo->activeColorMode = static_cast<ui::ColorMode>(ginfo.activeColorMode); + + std::vector<ui::Hdr> types; + types.reserve(ginfo.hdrCapabilities.supportedHdrTypes.size()); + for (const auto& hdr : ginfo.hdrCapabilities.supportedHdrTypes) { + types.push_back(static_cast<ui::Hdr>(hdr)); + } + outInfo->hdrCapabilities = HdrCapabilities(types, ginfo.hdrCapabilities.maxLuminance, + ginfo.hdrCapabilities.maxAverageLuminance, + ginfo.hdrCapabilities.minLuminance); + + outInfo->autoLowLatencyModeSupported = ginfo.autoLowLatencyModeSupported; + outInfo->gameContentTypeSupported = ginfo.gameContentTypeSupported; + outInfo->preferredBootDisplayMode = ginfo.preferredBootDisplayMode; +} + +status_t SurfaceComposerClient::getDynamicDisplayInfoFromId(int64_t displayId, + ui::DynamicDisplayInfo* outInfo) { gui::DynamicDisplayInfo ginfo; binder::Status status = - ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfo(display, &ginfo); + ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfoFromId(displayId, + &ginfo); if (status.isOk()) { - // convert gui::DynamicDisplayInfo to ui::DynamicDisplayInfo - outInfo->supportedDisplayModes.clear(); - outInfo->supportedDisplayModes.reserve(ginfo.supportedDisplayModes.size()); - for (const auto& mode : ginfo.supportedDisplayModes) { - ui::DisplayMode outMode; - outMode.id = mode.id; - outMode.resolution.width = mode.resolution.width; - outMode.resolution.height = mode.resolution.height; - outMode.xDpi = mode.xDpi; - outMode.yDpi = mode.yDpi; - outMode.refreshRate = mode.refreshRate; - outMode.appVsyncOffset = mode.appVsyncOffset; - outMode.sfVsyncOffset = mode.sfVsyncOffset; - outMode.presentationDeadline = mode.presentationDeadline; - outMode.group = mode.group; - std::transform(mode.supportedHdrTypes.begin(), mode.supportedHdrTypes.end(), - std::back_inserter(outMode.supportedHdrTypes), - [](const int32_t& value) { return static_cast<ui::Hdr>(value); }); - outInfo->supportedDisplayModes.push_back(outMode); - } - - outInfo->activeDisplayModeId = ginfo.activeDisplayModeId; - outInfo->renderFrameRate = ginfo.renderFrameRate; - - outInfo->supportedColorModes.clear(); - outInfo->supportedColorModes.reserve(ginfo.supportedColorModes.size()); - for (const auto& cmode : ginfo.supportedColorModes) { - outInfo->supportedColorModes.push_back(static_cast<ui::ColorMode>(cmode)); - } - - outInfo->activeColorMode = static_cast<ui::ColorMode>(ginfo.activeColorMode); - - std::vector<ui::Hdr> types; - types.reserve(ginfo.hdrCapabilities.supportedHdrTypes.size()); - for (const auto& hdr : ginfo.hdrCapabilities.supportedHdrTypes) { - types.push_back(static_cast<ui::Hdr>(hdr)); - } - outInfo->hdrCapabilities = HdrCapabilities(types, ginfo.hdrCapabilities.maxLuminance, - ginfo.hdrCapabilities.maxAverageLuminance, - ginfo.hdrCapabilities.minLuminance); + getDynamicDisplayInfoInternal(ginfo, outInfo); + } + return statusTFromBinderStatus(status); +} - outInfo->autoLowLatencyModeSupported = ginfo.autoLowLatencyModeSupported; - outInfo->gameContentTypeSupported = ginfo.gameContentTypeSupported; - outInfo->preferredBootDisplayMode = ginfo.preferredBootDisplayMode; +status_t SurfaceComposerClient::getDynamicDisplayInfoFromToken(const sp<IBinder>& display, + ui::DynamicDisplayInfo* outInfo) { + gui::DynamicDisplayInfo ginfo; + binder::Status status = + ComposerServiceAIDL::getComposerService()->getDynamicDisplayInfoFromToken(display, + &ginfo); + if (status.isOk()) { + getDynamicDisplayInfoInternal(ginfo, outInfo); } return statusTFromBinderStatus(status); } @@ -2366,7 +2384,8 @@ status_t SurfaceComposerClient::getDynamicDisplayInfo(const sp<IBinder>& display status_t SurfaceComposerClient::getActiveDisplayMode(const sp<IBinder>& display, ui::DisplayMode* mode) { ui::DynamicDisplayInfo info; - status_t result = getDynamicDisplayInfo(display, &info); + + status_t result = getDynamicDisplayInfoFromToken(display, &info); if (result != NO_ERROR) { return result; } diff --git a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl index 40410fb59e..488a1486cc 100644 --- a/libs/gui/aidl/android/gui/ISurfaceComposer.aidl +++ b/libs/gui/aidl/android/gui/ISurfaceComposer.aidl @@ -126,12 +126,14 @@ interface ISurfaceComposer { /** * Gets immutable information about given physical display. */ - StaticDisplayInfo getStaticDisplayInfo(IBinder display); + StaticDisplayInfo getStaticDisplayInfo(long displayId); /** * Gets dynamic information about given physical display. */ - DynamicDisplayInfo getDynamicDisplayInfo(IBinder display); + DynamicDisplayInfo getDynamicDisplayInfoFromId(long displayId); + + DynamicDisplayInfo getDynamicDisplayInfoFromToken(IBinder display); DisplayPrimaries getDisplayNativePrimaries(IBinder display); diff --git a/libs/gui/fuzzer/libgui_fuzzer_utils.h b/libs/gui/fuzzer/libgui_fuzzer_utils.h index 9d1ee8f65b..8810e4e83a 100644 --- a/libs/gui/fuzzer/libgui_fuzzer_utils.h +++ b/libs/gui/fuzzer/libgui_fuzzer_utils.h @@ -79,9 +79,11 @@ public: (override)); MOCK_METHOD(binder::Status, getDisplayState, (const sp<IBinder>&, gui::DisplayState*), (override)); - MOCK_METHOD(binder::Status, getStaticDisplayInfo, (const sp<IBinder>&, gui::StaticDisplayInfo*), + MOCK_METHOD(binder::Status, getStaticDisplayInfo, (int64_t, gui::StaticDisplayInfo*), (override)); - MOCK_METHOD(binder::Status, getDynamicDisplayInfo, + MOCK_METHOD(binder::Status, getDynamicDisplayInfoFromId, (int64_t, gui::DynamicDisplayInfo*), + (override)); + MOCK_METHOD(binder::Status, getDynamicDisplayInfoFromToken, (const sp<IBinder>&, gui::DynamicDisplayInfo*), (override)); MOCK_METHOD(binder::Status, getDisplayNativePrimaries, (const sp<IBinder>&, gui::DisplayPrimaries*), (override)); diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 6ec6bd70db..45a84f6c66 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -359,6 +359,7 @@ struct DisplayState { DisplayState(); void merge(const DisplayState& other); + void sanitize(int32_t permissions); uint32_t what = 0; uint32_t flags = 0; diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h index 2038f1477a..df47002b3b 100644 --- a/libs/gui/include/gui/SurfaceComposerClient.h +++ b/libs/gui/include/gui/SurfaceComposerClient.h @@ -149,10 +149,10 @@ public: static status_t getDisplayState(const sp<IBinder>& display, ui::DisplayState*); // Get immutable information about given physical display. - static status_t getStaticDisplayInfo(const sp<IBinder>& display, ui::StaticDisplayInfo*); + static status_t getStaticDisplayInfo(int64_t, ui::StaticDisplayInfo*); - // Get dynamic information about given physical display. - static status_t getDynamicDisplayInfo(const sp<IBinder>& display, ui::DynamicDisplayInfo*); + // Get dynamic information about given physical display from display id + static status_t getDynamicDisplayInfoFromId(int64_t, ui::DynamicDisplayInfo*); // Shorthand for the active display mode from getDynamicDisplayInfo(). // TODO(b/180391891): Update clients to use getDynamicDisplayInfo and remove this function. @@ -714,6 +714,12 @@ protected: ReleaseCallbackThread mReleaseCallbackThread; private: + // Get dynamic information about given physical display from token + static status_t getDynamicDisplayInfoFromToken(const sp<IBinder>& display, + ui::DynamicDisplayInfo*); + + static void getDynamicDisplayInfoInternal(gui::DynamicDisplayInfo& ginfo, + ui::DynamicDisplayInfo*& outInfo); virtual void onFirstRef(); mutable Mutex mLock; diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp index 6d3b42515b..55242dfd39 100644 --- a/libs/gui/tests/Surface_test.cpp +++ b/libs/gui/tests/Surface_test.cpp @@ -782,13 +782,18 @@ public: return binder::Status::ok(); } - binder::Status getStaticDisplayInfo(const sp<IBinder>& /*display*/, + binder::Status getStaticDisplayInfo(int64_t /*displayId*/, gui::StaticDisplayInfo* /*outInfo*/) override { return binder::Status::ok(); } - binder::Status getDynamicDisplayInfo(const sp<IBinder>& /*display*/, - gui::DynamicDisplayInfo* /*outInfo*/) override { + binder::Status getDynamicDisplayInfoFromId(int64_t /*displayId*/, + gui::DynamicDisplayInfo* /*outInfo*/) override { + return binder::Status::ok(); + } + + binder::Status getDynamicDisplayInfoFromToken(const sp<IBinder>& /*display*/, + gui::DynamicDisplayInfo* /*outInfo*/) override { return binder::Status::ok(); } diff --git a/libs/nativedisplay/ADisplay.cpp b/libs/nativedisplay/ADisplay.cpp index 60328e48a6..bf0805b46c 100644 --- a/libs/nativedisplay/ADisplay.cpp +++ b/libs/nativedisplay/ADisplay.cpp @@ -117,15 +117,6 @@ using namespace android::display::impl; #define CHECK_NOT_NULL(name) \ LOG_ALWAYS_FATAL_IF(name == nullptr, "nullptr passed as " #name " argument"); -namespace { - -sp<IBinder> getToken(ADisplay* display) { - DisplayImpl* impl = reinterpret_cast<DisplayImpl*>(display); - return SurfaceComposerClient::getPhysicalDisplayToken(impl->id); -} - -} // namespace - namespace android { int ADisplay_acquirePhysicalDisplays(ADisplay*** outDisplays) { @@ -139,10 +130,9 @@ int ADisplay_acquirePhysicalDisplays(ADisplay*** outDisplays) { ui::DisplayConnectionType displayConnectionTypes[size]; int numModes = 0; for (int i = 0; i < size; ++i) { - const sp<IBinder> token = SurfaceComposerClient::getPhysicalDisplayToken(ids[i]); - ui::StaticDisplayInfo staticInfo; - if (const status_t status = SurfaceComposerClient::getStaticDisplayInfo(token, &staticInfo); + if (const status_t status = + SurfaceComposerClient::getStaticDisplayInfo(ids[i].value, &staticInfo); status != OK) { return status; } @@ -150,7 +140,7 @@ int ADisplay_acquirePhysicalDisplays(ADisplay*** outDisplays) { ui::DynamicDisplayInfo dynamicInfo; if (const status_t status = - SurfaceComposerClient::getDynamicDisplayInfo(token, &dynamicInfo); + SurfaceComposerClient::getDynamicDisplayInfoFromId(ids[i].value, &dynamicInfo); status != OK) { return status; } @@ -260,14 +250,15 @@ void ADisplay_getPreferredWideColorFormat(ADisplay* display, ADataSpace* outData int ADisplay_getCurrentConfig(ADisplay* display, ADisplayConfig** outConfig) { CHECK_NOT_NULL(display); - sp<IBinder> token = getToken(display); ui::DynamicDisplayInfo info; - if (const auto status = SurfaceComposerClient::getDynamicDisplayInfo(token, &info); + DisplayImpl* impl = reinterpret_cast<DisplayImpl*>(display); + + if (const auto status = + SurfaceComposerClient::getDynamicDisplayInfoFromId(impl->id.value, &info); status != OK) { return status; } - DisplayImpl* impl = reinterpret_cast<DisplayImpl*>(display); for (size_t i = 0; i < impl->numConfigs; i++) { auto* config = impl->configs + i; if (config->id == info.activeDisplayModeId) { diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index 36ff3ec5c2..6da6022a83 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -974,17 +974,14 @@ status_t SurfaceFlinger::getDisplayState(const sp<IBinder>& displayToken, ui::Di return NO_ERROR; } -status_t SurfaceFlinger::getStaticDisplayInfo(const sp<IBinder>& displayToken, - ui::StaticDisplayInfo* info) { - if (!displayToken || !info) { +status_t SurfaceFlinger::getStaticDisplayInfo(int64_t displayId, ui::StaticDisplayInfo* info) { + if (!info) { return BAD_VALUE; } Mutex::Autolock lock(mStateLock); - - const auto displayOpt = ftl::find_if(mPhysicalDisplays, PhysicalDisplay::hasToken(displayToken)) - .transform(&ftl::to_mapped_ref<PhysicalDisplays>) - .and_then(getDisplayDeviceAndSnapshot()); + const auto id = DisplayId::fromValue<PhysicalDisplayId>(static_cast<uint64_t>(displayId)); + const auto displayOpt = mPhysicalDisplays.get(*id).and_then(getDisplayDeviceAndSnapshot()); if (!displayOpt) { return NAME_NOT_FOUND; @@ -1011,26 +1008,10 @@ status_t SurfaceFlinger::getStaticDisplayInfo(const sp<IBinder>& displayToken, return NO_ERROR; } -status_t SurfaceFlinger::getDynamicDisplayInfo(const sp<IBinder>& displayToken, - ui::DynamicDisplayInfo* info) { - if (!displayToken || !info) { - return BAD_VALUE; - } - - Mutex::Autolock lock(mStateLock); - - const auto displayOpt = ftl::find_if(mPhysicalDisplays, PhysicalDisplay::hasToken(displayToken)) - .transform(&ftl::to_mapped_ref<PhysicalDisplays>) - .and_then(getDisplayDeviceAndSnapshot()); - if (!displayOpt) { - return NAME_NOT_FOUND; - } - - const auto& [display, snapshotRef] = *displayOpt; - const auto& snapshot = snapshotRef.get(); - +void SurfaceFlinger::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*& info, + const sp<DisplayDevice>& display, + const display::DisplaySnapshot& snapshot) { const auto& displayModes = snapshot.displayModes(); - info->supportedDisplayModes.clear(); info->supportedDisplayModes.reserve(displayModes.size()); @@ -1104,7 +1085,47 @@ status_t SurfaceFlinger::getDynamicDisplayInfo(const sp<IBinder>& displayToken, } } } +} + +status_t SurfaceFlinger::getDynamicDisplayInfoFromId(int64_t physicalDisplayId, + ui::DynamicDisplayInfo* info) { + if (!info) { + return BAD_VALUE; + } + + Mutex::Autolock lock(mStateLock); + + const auto id_ = + DisplayId::fromValue<PhysicalDisplayId>(static_cast<uint64_t>(physicalDisplayId)); + const auto displayOpt = mPhysicalDisplays.get(*id_).and_then(getDisplayDeviceAndSnapshot()); + + if (!displayOpt) { + return NAME_NOT_FOUND; + } + + const auto& [display, snapshotRef] = *displayOpt; + getDynamicDisplayInfoInternal(info, display, snapshotRef.get()); + return NO_ERROR; +} + +status_t SurfaceFlinger::getDynamicDisplayInfoFromToken(const sp<IBinder>& displayToken, + ui::DynamicDisplayInfo* info) { + if (!displayToken || !info) { + return BAD_VALUE; + } + + Mutex::Autolock lock(mStateLock); + + const auto displayOpt = ftl::find_if(mPhysicalDisplays, PhysicalDisplay::hasToken(displayToken)) + .transform(&ftl::to_mapped_ref<PhysicalDisplays>) + .and_then(getDisplayDeviceAndSnapshot()); + + if (!displayOpt) { + return NAME_NOT_FOUND; + } + const auto& [display, snapshotRef] = *displayOpt; + getDynamicDisplayInfoInternal(info, display, snapshotRef.get()); return NO_ERROR; } @@ -4036,7 +4057,7 @@ status_t SurfaceFlinger::setTransactionState( bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo, std::vector<ResolvedComposerState>& states, - const Vector<DisplayState>& displays, uint32_t flags, + Vector<DisplayState>& displays, uint32_t flags, const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime, bool isAutoTimestamp, const client_cache_t& uncacheBuffer, @@ -4045,7 +4066,8 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin const std::vector<ListenerCallbacks>& listenerCallbacks, int originPid, int originUid, uint64_t transactionId) { uint32_t transactionFlags = 0; - for (const DisplayState& display : displays) { + for (DisplayState& display : displays) { + display.sanitize(permissions); transactionFlags |= setDisplayStateLocked(display); } @@ -7283,6 +7305,10 @@ binder::Status SurfaceComposerAIDL::getPhysicalDisplayIds(std::vector<int64_t>* binder::Status SurfaceComposerAIDL::getPhysicalDisplayToken(int64_t displayId, sp<IBinder>* outDisplay) { + status_t status = checkAccessPermission(); + if (status != OK) { + return binderStatusFromStatusT(status); + } const auto id = DisplayId::fromValue<PhysicalDisplayId>(static_cast<uint64_t>(displayId)); *outDisplay = mFlinger->getPhysicalDisplayToken(*id); return binder::Status::ok(); @@ -7333,11 +7359,12 @@ binder::Status SurfaceComposerAIDL::getDisplayState(const sp<IBinder>& display, return binderStatusFromStatusT(status); } -binder::Status SurfaceComposerAIDL::getStaticDisplayInfo(const sp<IBinder>& display, +binder::Status SurfaceComposerAIDL::getStaticDisplayInfo(int64_t displayId, gui::StaticDisplayInfo* outInfo) { using Tag = gui::DeviceProductInfo::ManufactureOrModelDate::Tag; ui::StaticDisplayInfo info; - status_t status = mFlinger->getStaticDisplayInfo(display, &info); + + status_t status = mFlinger->getStaticDisplayInfo(displayId, &info); if (status == NO_ERROR) { // convert ui::StaticDisplayInfo to gui::StaticDisplayInfo outInfo->connectionType = static_cast<gui::DisplayConnectionType>(info.connectionType); @@ -7376,58 +7403,71 @@ binder::Status SurfaceComposerAIDL::getStaticDisplayInfo(const sp<IBinder>& disp return binderStatusFromStatusT(status); } -binder::Status SurfaceComposerAIDL::getDynamicDisplayInfo(const sp<IBinder>& display, - gui::DynamicDisplayInfo* outInfo) { +void SurfaceComposerAIDL::getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo& info, + gui::DynamicDisplayInfo*& outInfo) { + // convert ui::DynamicDisplayInfo to gui::DynamicDisplayInfo + outInfo->supportedDisplayModes.clear(); + outInfo->supportedDisplayModes.reserve(info.supportedDisplayModes.size()); + for (const auto& mode : info.supportedDisplayModes) { + gui::DisplayMode outMode; + outMode.id = mode.id; + outMode.resolution.width = mode.resolution.width; + outMode.resolution.height = mode.resolution.height; + outMode.xDpi = mode.xDpi; + outMode.yDpi = mode.yDpi; + outMode.refreshRate = mode.refreshRate; + outMode.appVsyncOffset = mode.appVsyncOffset; + outMode.sfVsyncOffset = mode.sfVsyncOffset; + outMode.presentationDeadline = mode.presentationDeadline; + outMode.group = mode.group; + std::transform(mode.supportedHdrTypes.begin(), mode.supportedHdrTypes.end(), + std::back_inserter(outMode.supportedHdrTypes), + [](const ui::Hdr& value) { return static_cast<int32_t>(value); }); + outInfo->supportedDisplayModes.push_back(outMode); + } + + outInfo->activeDisplayModeId = info.activeDisplayModeId; + outInfo->renderFrameRate = info.renderFrameRate; + + outInfo->supportedColorModes.clear(); + outInfo->supportedColorModes.reserve(info.supportedColorModes.size()); + for (const auto& cmode : info.supportedColorModes) { + outInfo->supportedColorModes.push_back(static_cast<int32_t>(cmode)); + } + + outInfo->activeColorMode = static_cast<int32_t>(info.activeColorMode); + + gui::HdrCapabilities& hdrCapabilities = outInfo->hdrCapabilities; + hdrCapabilities.supportedHdrTypes.clear(); + hdrCapabilities.supportedHdrTypes.reserve(info.hdrCapabilities.getSupportedHdrTypes().size()); + for (const auto& hdr : info.hdrCapabilities.getSupportedHdrTypes()) { + hdrCapabilities.supportedHdrTypes.push_back(static_cast<int32_t>(hdr)); + } + hdrCapabilities.maxLuminance = info.hdrCapabilities.getDesiredMaxLuminance(); + hdrCapabilities.maxAverageLuminance = info.hdrCapabilities.getDesiredMaxAverageLuminance(); + hdrCapabilities.minLuminance = info.hdrCapabilities.getDesiredMinLuminance(); + + outInfo->autoLowLatencyModeSupported = info.autoLowLatencyModeSupported; + outInfo->gameContentTypeSupported = info.gameContentTypeSupported; + outInfo->preferredBootDisplayMode = info.preferredBootDisplayMode; +} + +binder::Status SurfaceComposerAIDL::getDynamicDisplayInfoFromToken( + const sp<IBinder>& display, gui::DynamicDisplayInfo* outInfo) { + ui::DynamicDisplayInfo info; + status_t status = mFlinger->getDynamicDisplayInfoFromToken(display, &info); + if (status == NO_ERROR) { + getDynamicDisplayInfoInternal(info, outInfo); + } + return binderStatusFromStatusT(status); +} + +binder::Status SurfaceComposerAIDL::getDynamicDisplayInfoFromId(int64_t displayId, + gui::DynamicDisplayInfo* outInfo) { ui::DynamicDisplayInfo info; - status_t status = mFlinger->getDynamicDisplayInfo(display, &info); + status_t status = mFlinger->getDynamicDisplayInfoFromId(displayId, &info); if (status == NO_ERROR) { - // convert ui::DynamicDisplayInfo to gui::DynamicDisplayInfo - outInfo->supportedDisplayModes.clear(); - outInfo->supportedDisplayModes.reserve(info.supportedDisplayModes.size()); - for (const auto& mode : info.supportedDisplayModes) { - gui::DisplayMode outMode; - outMode.id = mode.id; - outMode.resolution.width = mode.resolution.width; - outMode.resolution.height = mode.resolution.height; - outMode.xDpi = mode.xDpi; - outMode.yDpi = mode.yDpi; - outMode.refreshRate = mode.refreshRate; - outMode.appVsyncOffset = mode.appVsyncOffset; - outMode.sfVsyncOffset = mode.sfVsyncOffset; - outMode.presentationDeadline = mode.presentationDeadline; - outMode.group = mode.group; - std::transform(mode.supportedHdrTypes.begin(), mode.supportedHdrTypes.end(), - std::back_inserter(outMode.supportedHdrTypes), - [](const ui::Hdr& value) { return static_cast<int32_t>(value); }); - - outInfo->supportedDisplayModes.push_back(outMode); - } - - outInfo->activeDisplayModeId = info.activeDisplayModeId; - outInfo->renderFrameRate = info.renderFrameRate; - - outInfo->supportedColorModes.clear(); - outInfo->supportedColorModes.reserve(info.supportedColorModes.size()); - for (const auto& cmode : info.supportedColorModes) { - outInfo->supportedColorModes.push_back(static_cast<int32_t>(cmode)); - } - - outInfo->activeColorMode = static_cast<int32_t>(info.activeColorMode); - - gui::HdrCapabilities& hdrCapabilities = outInfo->hdrCapabilities; - hdrCapabilities.supportedHdrTypes.clear(); - hdrCapabilities.supportedHdrTypes.reserve( - info.hdrCapabilities.getSupportedHdrTypes().size()); - for (const auto& hdr : info.hdrCapabilities.getSupportedHdrTypes()) { - hdrCapabilities.supportedHdrTypes.push_back(static_cast<int32_t>(hdr)); - } - hdrCapabilities.maxLuminance = info.hdrCapabilities.getDesiredMaxLuminance(); - hdrCapabilities.maxAverageLuminance = info.hdrCapabilities.getDesiredMaxAverageLuminance(); - hdrCapabilities.minLuminance = info.hdrCapabilities.getDesiredMinLuminance(); - - outInfo->autoLowLatencyModeSupported = info.autoLowLatencyModeSupported; - outInfo->gameContentTypeSupported = info.gameContentTypeSupported; - outInfo->preferredBootDisplayMode = info.preferredBootDisplayMode; + getDynamicDisplayInfoInternal(info, outInfo); } return binderStatusFromStatusT(status); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 6ddcfbccf8..e265939e70 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -512,10 +512,13 @@ private: status_t getDisplayStats(const sp<IBinder>& displayToken, DisplayStatInfo* stats); status_t getDisplayState(const sp<IBinder>& displayToken, ui::DisplayState*) EXCLUDES(mStateLock); - status_t getStaticDisplayInfo(const sp<IBinder>& displayToken, ui::StaticDisplayInfo*) - EXCLUDES(mStateLock); - status_t getDynamicDisplayInfo(const sp<IBinder>& displayToken, ui::DynamicDisplayInfo*) + status_t getStaticDisplayInfo(int64_t displayId, ui::StaticDisplayInfo*) EXCLUDES(mStateLock); + status_t getDynamicDisplayInfoFromId(int64_t displayId, ui::DynamicDisplayInfo*) EXCLUDES(mStateLock); + status_t getDynamicDisplayInfoFromToken(const sp<IBinder>& displayToken, + ui::DynamicDisplayInfo*) EXCLUDES(mStateLock); + void getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo*&, const sp<DisplayDevice>&, + const display::DisplaySnapshot&); status_t getDisplayNativePrimaries(const sp<IBinder>& displayToken, ui::DisplayPrimaries&); status_t setActiveColorMode(const sp<IBinder>& displayToken, ui::ColorMode colorMode); status_t getBootDisplayModeSupport(bool* outSupport) const; @@ -702,7 +705,7 @@ private: */ bool applyTransactionState(const FrameTimelineInfo& info, std::vector<ResolvedComposerState>& state, - const Vector<DisplayState>& displays, uint32_t flags, + Vector<DisplayState>& displays, uint32_t flags, const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime, bool isAutoTimestamp, const client_cache_t& uncacheBuffer, const int64_t postTime, @@ -1401,10 +1404,12 @@ public: gui::DisplayStatInfo* outStatInfo) override; binder::Status getDisplayState(const sp<IBinder>& display, gui::DisplayState* outState) override; - binder::Status getStaticDisplayInfo(const sp<IBinder>& display, + binder::Status getStaticDisplayInfo(int64_t displayId, gui::StaticDisplayInfo* outInfo) override; - binder::Status getDynamicDisplayInfo(const sp<IBinder>& display, - gui::DynamicDisplayInfo* outInfo) override; + binder::Status getDynamicDisplayInfoFromId(int64_t displayId, + gui::DynamicDisplayInfo* outInfo) override; + binder::Status getDynamicDisplayInfoFromToken(const sp<IBinder>& display, + gui::DynamicDisplayInfo* outInfo) override; binder::Status getDisplayNativePrimaries(const sp<IBinder>& display, gui::DisplayPrimaries* outPrimaries) override; binder::Status setActiveColorMode(const sp<IBinder>& display, int colorMode) override; @@ -1489,6 +1494,8 @@ private: status_t checkAccessPermission(bool usePermissionCache = kUsePermissionCache); status_t checkControlDisplayBrightnessPermission(); status_t checkReadFrameBufferPermission(); + static void getDynamicDisplayInfoInternal(ui::DynamicDisplayInfo& info, + gui::DynamicDisplayInfo*& outInfo); private: sp<SurfaceFlinger> mFlinger; diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h index c0a6bdbe27..96844d2b18 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h @@ -491,14 +491,14 @@ public: mFlinger->getDisplayState(display, &displayState); } - void getStaticDisplayInfo(sp<IBinder> &display) { + void getStaticDisplayInfo(int64_t displayId) { ui::StaticDisplayInfo staticDisplayInfo; - mFlinger->getStaticDisplayInfo(display, &staticDisplayInfo); + mFlinger->getStaticDisplayInfo(displayId, &staticDisplayInfo); } - void getDynamicDisplayInfo(sp<IBinder> &display) { + void getDynamicDisplayInfo(int64_t displayId) { android::ui::DynamicDisplayInfo dynamicDisplayInfo; - mFlinger->getDynamicDisplayInfo(display, &dynamicDisplayInfo); + mFlinger->getDynamicDisplayInfoFromId(displayId, &dynamicDisplayInfo); } void getDisplayNativePrimaries(sp<IBinder> &display) { android::ui::DisplayPrimaries displayPrimaries; @@ -522,7 +522,7 @@ public: return ids.front(); } - sp<IBinder> fuzzBoot(FuzzedDataProvider *fdp) { + std::pair<sp<IBinder>, int64_t> fuzzBoot(FuzzedDataProvider *fdp) { mFlinger->callingThreadHasUnscopedSurfaceFlingerAccess(fdp->ConsumeBool()); const sp<Client> client = sp<Client>::make(mFlinger); @@ -549,13 +549,13 @@ public: mFlinger->bootFinished(); - return display; + return {display, physicalDisplayId.value}; } void fuzzSurfaceFlinger(const uint8_t *data, size_t size) { FuzzedDataProvider mFdp(data, size); - sp<IBinder> display = fuzzBoot(&mFdp); + auto [display, displayId] = fuzzBoot(&mFdp); sp<IGraphicBufferProducer> bufferProducer = sp<mock::GraphicBufferProducer>::make(); @@ -563,8 +563,8 @@ public: getDisplayStats(display); getDisplayState(display); - getStaticDisplayInfo(display); - getDynamicDisplayInfo(display); + getStaticDisplayInfo(displayId); + getDynamicDisplayInfo(displayId); getDisplayNativePrimaries(display); mFlinger->setAutoLowLatencyMode(display, mFdp.ConsumeBool()); diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp index 16768441f0..4a45eb5586 100644 --- a/services/surfaceflinger/tests/Credentials_test.cpp +++ b/services/surfaceflinger/tests/Credentials_test.cpp @@ -83,6 +83,15 @@ protected: return SurfaceComposerClient::getPhysicalDisplayToken(ids.front()); } + static std::optional<uint64_t> getFirstDisplayId() { + const auto ids = SurfaceComposerClient::getPhysicalDisplayIds(); + if (ids.empty()) { + return std::nullopt; + } + + return ids.front().value; + } + void setupBackgroundSurface() { mDisplay = getFirstDisplayToken(); ASSERT_FALSE(mDisplay == nullptr); @@ -169,29 +178,25 @@ TEST_F(CredentialsTest, ClientInitTest) { TEST_F(CredentialsTest, GetBuiltInDisplayAccessTest) { std::function<bool()> condition = [] { return getFirstDisplayToken() != nullptr; }; // Anyone can access display information. - ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, true)); + ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false)); } TEST_F(CredentialsTest, AllowedGetterMethodsTest) { // The following methods are tested with a UID that is not root, graphics, // or system, to show that anyone can access them. UIDFaker f(AID_BIN); - const auto display = getFirstDisplayToken(); - ASSERT_TRUE(display != nullptr); - - ui::DisplayMode mode; - ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveDisplayMode(display, &mode)); - - Vector<ui::DisplayMode> modes; + const auto id = getFirstDisplayId(); + ASSERT_TRUE(id); ui::DynamicDisplayInfo info; - ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDynamicDisplayInfo(display, &info)); + ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDynamicDisplayInfoFromId(*id, &info)); } TEST_F(CredentialsTest, GetDynamicDisplayInfoTest) { - const auto display = getFirstDisplayToken(); + const auto id = getFirstDisplayId(); + ASSERT_TRUE(id); std::function<status_t()> condition = [=]() { ui::DynamicDisplayInfo info; - return SurfaceComposerClient::getDynamicDisplayInfo(display, &info); + return SurfaceComposerClient::getDynamicDisplayInfoFromId(*id, &info); }; ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, NO_ERROR)); } @@ -335,8 +340,10 @@ TEST_F(CredentialsTest, IsWideColorDisplayBasicCorrectness) { status_t error = SurfaceComposerClient::isWideColorDisplay(display, &result); ASSERT_EQ(NO_ERROR, error); bool hasWideColorMode = false; + const auto id = getFirstDisplayId(); + ASSERT_TRUE(id); ui::DynamicDisplayInfo info; - SurfaceComposerClient::getDynamicDisplayInfo(display, &info); + SurfaceComposerClient::getDynamicDisplayInfoFromId(*id, &info); const auto& colorModes = info.supportedColorModes; for (ColorMode colorMode : colorModes) { switch (colorMode) { @@ -363,10 +370,10 @@ TEST_F(CredentialsTest, IsWideColorDisplayWithPrivileges) { } TEST_F(CredentialsTest, GetActiveColorModeBasicCorrectness) { - const auto display = getFirstDisplayToken(); - ASSERT_FALSE(display == nullptr); + const auto id = getFirstDisplayId(); + ASSERT_TRUE(id); ui::DynamicDisplayInfo info; - SurfaceComposerClient::getDynamicDisplayInfo(display, &info); + SurfaceComposerClient::getDynamicDisplayInfoFromId(*id, &info); ColorMode colorMode = info.activeColorMode; ASSERT_NE(static_cast<ColorMode>(BAD_VALUE), colorMode); } diff --git a/services/surfaceflinger/tests/DisplayConfigs_test.cpp b/services/surfaceflinger/tests/DisplayConfigs_test.cpp index 10dae4636e..4be961bda1 100644 --- a/services/surfaceflinger/tests/DisplayConfigs_test.cpp +++ b/services/surfaceflinger/tests/DisplayConfigs_test.cpp @@ -45,6 +45,7 @@ protected: void SetUp() override { const auto ids = SurfaceComposerClient::getPhysicalDisplayIds(); ASSERT_FALSE(ids.empty()); + mDisplayId = ids.front().value; mDisplayToken = SurfaceComposerClient::getPhysicalDisplayToken(ids.front()); status_t res = SurfaceComposerClient::getDesiredDisplayModeSpecs(mDisplayToken, &mSpecs); ASSERT_EQ(res, NO_ERROR); @@ -58,11 +59,14 @@ protected: void testSetAllowGroupSwitching(bool allowGroupSwitching); sp<IBinder> mDisplayToken; + uint64_t mDisplayId; }; TEST_F(RefreshRateRangeTest, setAllConfigs) { ui::DynamicDisplayInfo info; - status_t res = SurfaceComposerClient::getDynamicDisplayInfo(mDisplayToken, &info); + status_t res = + SurfaceComposerClient::getDynamicDisplayInfoFromId(static_cast<int64_t>(mDisplayId), + &info); const auto& modes = info.supportedDisplayModes; ASSERT_EQ(res, NO_ERROR); ASSERT_GT(modes.size(), 0); diff --git a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ExcludeDolbyVisionTest.cpp b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ExcludeDolbyVisionTest.cpp index 11e734a416..0e149d2bfb 100644 --- a/services/surfaceflinger/tests/unittests/SurfaceFlinger_ExcludeDolbyVisionTest.cpp +++ b/services/surfaceflinger/tests/unittests/SurfaceFlinger_ExcludeDolbyVisionTest.cpp @@ -61,7 +61,7 @@ protected: TEST_F(ExcludeDolbyVisionTest, excludesDolbyVisionOnModesHigherThan4k30) { injectDisplayModes({mode4k60}); ui::DynamicDisplayInfo info; - mFlinger.getDynamicDisplayInfo(mDisplay->getDisplayToken().promote(), &info); + mFlinger.getDynamicDisplayInfoFromToken(mDisplay->getDisplayToken().promote(), &info); std::vector<ui::DisplayMode> displayModes = info.supportedDisplayModes; @@ -75,7 +75,7 @@ TEST_F(ExcludeDolbyVisionTest, excludesDolbyVisionOnModesHigherThan4k30) { TEST_F(ExcludeDolbyVisionTest, includesDolbyVisionOnModesLowerThanOrEqualTo4k30) { injectDisplayModes({mode1080p60, mode4k30, mode4k30NonStandard}); ui::DynamicDisplayInfo info; - mFlinger.getDynamicDisplayInfo(mDisplay->getDisplayToken().promote(), &info); + mFlinger.getDynamicDisplayInfoFromToken(mDisplay->getDisplayToken().promote(), &info); std::vector<ui::DisplayMode> displayModes = info.supportedDisplayModes; @@ -94,7 +94,7 @@ TEST_F(ExcludeDolbyVisionTest, includesDolbyVisionOnModesLowerThanOrEqualTo4k30) TEST_F(ExcludeDolbyVisionTest, 4k30IsNotReportedAsAValidHdrType) { injectDisplayModes({mode4k60}); ui::DynamicDisplayInfo info; - mFlinger.getDynamicDisplayInfo(mDisplay->getDisplayToken().promote(), &info); + mFlinger.getDynamicDisplayInfoFromToken(mDisplay->getDisplayToken().promote(), &info); std::vector<ui::Hdr> displayHdrTypes = info.hdrCapabilities.getSupportedHdrTypes(); diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h index 7d0b340cfa..2117084bbf 100644 --- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h +++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h @@ -487,9 +487,9 @@ public: void updateLayerMetadataSnapshot() { mFlinger->updateLayerMetadataSnapshot(); } - void getDynamicDisplayInfo(const sp<IBinder>& displayToken, - ui::DynamicDisplayInfo* dynamicDisplayInfo) { - mFlinger->getDynamicDisplayInfo(displayToken, dynamicDisplayInfo); + void getDynamicDisplayInfoFromToken(const sp<IBinder>& displayToken, + ui::DynamicDisplayInfo* dynamicDisplayInfo) { + mFlinger->getDynamicDisplayInfoFromToken(displayToken, dynamicDisplayInfo); } /* ------------------------------------------------------------------------ |