From 3ea58dbc1d7a248160403f089b9998bf6694aae1 Mon Sep 17 00:00:00 2001 From: Sally Qi Date: Wed, 5 Oct 2022 11:42:30 -0700 Subject: Mitigate the security vulnerability by sanitizing the transaction flags. - This is part of fix of commit Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df for backporting. 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 Merged-In: Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df --- libs/gui/LayerState.cpp | 21 +++++++++++++++++++++ libs/gui/include/gui/LayerState.h | 1 + services/surfaceflinger/SurfaceFlinger.cpp | 9 +++++---- services/surfaceflinger/SurfaceFlinger.h | 2 +- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp index bb4b44684f..849781a8e3 100644 --- a/libs/gui/LayerState.cpp +++ b/libs/gui/LayerState.cpp @@ -381,6 +381,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/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h index 9460319f4b..55677b6afa 100644 --- a/libs/gui/include/gui/LayerState.h +++ b/libs/gui/include/gui/LayerState.h @@ -276,6 +276,7 @@ struct DisplayState { DisplayState(); void merge(const DisplayState& other); + void sanitize(int32_t permissions); uint32_t what; sp token; diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp index c7c3e02604..1895b1c091 100644 --- a/services/surfaceflinger/SurfaceFlinger.cpp +++ b/services/surfaceflinger/SurfaceFlinger.cpp @@ -3460,7 +3460,7 @@ void SurfaceFlinger::flushTransactionQueues() { // to prevent onHandleDestroyed from being called while the lock is held, // we must keep a copy of the transactions (specifically the composer // states) around outside the scope of the lock - std::vector transactions; + std::vector transactions; // Layer handles that have transactions with buffers that are ready to be applied. std::unordered_set, ISurfaceComposer::SpHash> bufferLayersReadyToPresent; { @@ -3524,7 +3524,7 @@ void SurfaceFlinger::flushTransactionQueues() { } // Now apply all transactions. - for (const auto& transaction : transactions) { + for (auto& transaction : transactions) { applyTransactionState(transaction.frameTimelineInfo, transaction.states, transaction.displays, transaction.flags, transaction.inputWindowCommands, transaction.desiredPresentTime, @@ -3744,7 +3744,7 @@ status_t SurfaceFlinger::setTransactionState( void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelineInfo, const Vector& states, - const Vector& displays, uint32_t flags, + Vector& displays, uint32_t flags, const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime, bool isAutoTimestamp, const client_cache_t& uncacheBuffer, @@ -3753,7 +3753,8 @@ void SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin const std::vector& 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); } diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h index 95c251c721..97ff27c150 100644 --- a/services/surfaceflinger/SurfaceFlinger.h +++ b/services/surfaceflinger/SurfaceFlinger.h @@ -829,7 +829,7 @@ private: * Transactions */ void applyTransactionState(const FrameTimelineInfo& info, const Vector& state, - const Vector& displays, uint32_t flags, + Vector& displays, uint32_t flags, const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime, bool isAutoTimestamp, const client_cache_t& uncacheBuffer, const int64_t postTime, -- cgit v1.2.3-59-g8ed1b