summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSally Qi <sallyqi@google.com>2022-10-05 11:42:30 -0700
committerSally Qi <sallyqi@google.com>2023-01-23 10:57:42 -0800
commit03d4458ea0cb00c28f695d99aae5e4c6b15fc237 (patch)
treec0fb21071d596a24c0103c3b198e0f36e83e59e5
parentc7df484a6f4265d20418fc64c46ead350f794a0c (diff)
downloadnative-03d4458ea0cb00c28f695d99aae5e4c6b15fc237.tar.gz
Mitigate the security vulnerability by sanitizing the transaction flags.
- This is part of fix of commit Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df for backporting. - Part of commit Id9d9012d4ede9c8330f0ce1096bcb78e51b7c5df which sanitizes the transaction flags from DisplayState instead. - In rvc, we only have ACCESS_SURFACE_FLINGER permission check passed as `privileged` argument in SF::applyTransactionState. We can directly utilize it for sanitization in DiaplyState. - In rvc code base, SF::setTransactionState pass a const array of displayState objects and then call SF::applyTransactionState. To successfully sanitize the flags for each displayState object, we convert this const array into non-const one before calling SF::applyTransactionState. 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
-rw-r--r--libs/gui/LayerState.cpp21
-rw-r--r--libs/gui/include/gui/LayerState.h1
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp14
-rw-r--r--services/surfaceflinger/SurfaceFlinger.h5
4 files changed, 34 insertions, 7 deletions
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index dfcef8fe7a..a76a21c5c4 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -276,6 +276,27 @@ void DisplayState::merge(const DisplayState& other) {
}
}
+void DisplayState::sanitize(bool privileged) {
+ if (what & DisplayState::eLayerStackChanged) {
+ if (!privileged) {
+ what &= ~DisplayState::eLayerStackChanged;
+ ALOGE("Stripped attempt to set eLayerStackChanged in sanitize");
+ }
+ }
+ if (what & DisplayState::eDisplayProjectionChanged) {
+ if (!privileged) {
+ what &= ~DisplayState::eDisplayProjectionChanged;
+ ALOGE("Stripped attempt to set eDisplayProjectionChanged in sanitize");
+ }
+ }
+ if (what & DisplayState::eSurfaceChanged) {
+ if (!privileged) {
+ what &= ~DisplayState::eSurfaceChanged;
+ ALOGE("Stripped attempt to set eSurfaceChanged in sanitize");
+ }
+ }
+}
+
void layer_state_t::merge(const layer_state_t& other) {
if (other.what & ePositionChanged) {
what |= ePositionChanged;
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 39dbe9e035..b73bb89791 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -267,6 +267,7 @@ struct DisplayState {
DisplayState();
void merge(const DisplayState& other);
+ void sanitize(bool privileged);
uint32_t what;
sp<IBinder> token;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4165df9ae5..14da0c5daa 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3273,7 +3273,7 @@ bool SurfaceFlinger::flushTransactionQueues() {
auto& [applyToken, transactionQueue] = *it;
while (!transactionQueue.empty()) {
- const auto& transaction = transactionQueue.front();
+ auto& transaction = transactionQueue.front();
if (!transactionIsReadyToBeApplied(transaction.desiredPresentTime,
transaction.states)) {
setTransactionFlags(eTransactionFlushNeeded);
@@ -3372,13 +3372,18 @@ void SurfaceFlinger::setTransactionState(
return;
}
- applyTransactionState(states, displays, flags, inputWindowCommands, desiredPresentTime,
+ Vector<DisplayState> displaysList;
+ for (auto& d : displays) {
+ displaysList.add(d);
+ }
+
+ applyTransactionState(states, displaysList, flags, inputWindowCommands, desiredPresentTime,
uncacheBuffer, postTime, privileged, hasListenerCallbacks,
listenerCallbacks);
}
void SurfaceFlinger::applyTransactionState(
- const Vector<ComposerState>& states, const Vector<DisplayState>& displays, uint32_t flags,
+ const Vector<ComposerState>& states, Vector<DisplayState>& displays, uint32_t flags,
const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime,
const client_cache_t& uncacheBuffer, const int64_t postTime, bool privileged,
bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
@@ -3401,7 +3406,8 @@ void SurfaceFlinger::applyTransactionState(
}
}
- for (const DisplayState& display : displays) {
+ for (DisplayState& display : displays) {
+ display.sanitize(privileged);
transactionFlags |= setDisplayStateLocked(display);
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index ccaeb2d858..913158f386 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -618,9 +618,8 @@ private:
/* ------------------------------------------------------------------------
* Transactions
*/
- void applyTransactionState(const Vector<ComposerState>& state,
- const Vector<DisplayState>& displays, uint32_t flags,
- const InputWindowCommands& inputWindowCommands,
+ void applyTransactionState(const Vector<ComposerState>& state, Vector<DisplayState>& displays,
+ uint32_t flags, const InputWindowCommands& inputWindowCommands,
const int64_t desiredPresentTime,
const client_cache_t& uncacheBuffer, const int64_t postTime,
bool privileged, bool hasListenerCallbacks,