Skip to content

Commit fc9812c

Browse files
patrickdwilliamsandroid-build-merge-worker-robot
patrickdwilliams
authored and
android-build-merge-worker-robot
committed
Fix DisplayState sanitization. am: 8e2ca26
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/28467246 Change-Id: Ic9b91b5b928b99ddd61f5cb553bfdc12f667ce4f Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
2 parents b4bf96a + 8e2ca26 commit fc9812c

9 files changed

Lines changed: 61 additions & 9 deletions

File tree

‎libs/gui/ISurfaceComposer.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class BpSurfaceComposer : public BpInterface<ISurfaceComposer>
6161

6262
status_t setTransactionState(
6363
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
64-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
64+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
6565
InputWindowCommands commands, int64_t desiredPresentTime, bool isAutoTimestamp,
6666
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
6767
const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,

‎libs/gui/SurfaceComposerClient.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,8 @@ void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
10471047
uncacheBuffer.token = BufferCache::getInstance().getToken();
10481048
uncacheBuffer.id = cacheId;
10491049
Vector<ComposerState> composerStates;
1050-
status_t status = sf->setTransactionState(FrameTimelineInfo{}, composerStates, {},
1050+
Vector<DisplayState> displayStates;
1051+
status_t status = sf->setTransactionState(FrameTimelineInfo{}, composerStates, displayStates,
10511052
ISurfaceComposer::eOneWay,
10521053
Transaction::getDefaultApplyToken(), {}, systemTime(),
10531054
true, {uncacheBuffer}, false, {}, generateId(), {});

‎libs/gui/include/gui/ISurfaceComposer.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class ISurfaceComposer: public IInterface {
112112
/* open/close transactions. requires ACCESS_SURFACE_FLINGER permission */
113113
virtual status_t setTransactionState(
114114
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
115-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
115+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
116116
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
117117
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffer,
118118
bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,

‎libs/gui/tests/Surface_test.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,7 @@ class FakeSurfaceComposer : public ISurfaceComposer {
697697

698698
status_t setTransactionState(
699699
const FrameTimelineInfo& /*frameTimelineInfo*/, Vector<ComposerState>& /*state*/,
700-
const Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
700+
Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
701701
const sp<IBinder>& /*applyToken*/, InputWindowCommands /*inputWindowCommands*/,
702702
int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,
703703
const std::vector<client_cache_t>& /*cachedBuffer*/, bool /*hasListenerCallbacks*/,

‎services/surfaceflinger/SurfaceFlinger.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4507,7 +4507,7 @@ bool SurfaceFlinger::shouldLatchUnsignaled(const sp<Layer>& layer, const layer_s
45074507

45084508
status_t SurfaceFlinger::setTransactionState(
45094509
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
4510-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
4510+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
45114511
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp,
45124512
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
45134513
const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
@@ -4522,7 +4522,7 @@ status_t SurfaceFlinger::setTransactionState(
45224522
composerState.state.sanitize(permissions);
45234523
}
45244524

4525-
for (DisplayState display : displays) {
4525+
for (DisplayState& display : displays) {
45264526
display.sanitize(permissions);
45274527
}
45284528

‎services/surfaceflinger/SurfaceFlinger.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ class SurfaceFlinger : public BnSurfaceComposer,
518518
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
519519
status_t setTransactionState(
520520
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
521-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
521+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
522522
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
523523
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
524524
bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,

‎services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ class TestableSurfaceFlinger final : private scheduler::ISchedulerCallback {
739739

740740
auto setTransactionState(
741741
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
742-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
742+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
743743
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
744744
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
745745
bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,

‎services/surfaceflinger/tests/Credentials_test.cpp‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <private/android_filesystem_config.h>
2828
#include <private/gui/ComposerServiceAIDL.h>
2929
#include <ui/DisplayMode.h>
30+
#include <ui/DisplayState.h>
3031
#include <ui/DynamicDisplayInfo.h>
3132
#include <utils/String8.h>
3233
#include <functional>
@@ -439,6 +440,56 @@ TEST_F(CredentialsTest, TransactionPermissionTest) {
439440
}
440441
}
441442

443+
TEST_F(CredentialsTest, DisplayTransactionPermissionTest) {
444+
const auto display = getFirstDisplayToken();
445+
446+
ui::DisplayState displayState;
447+
ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayState(display, &displayState));
448+
const ui::Rotation initialOrientation = displayState.orientation;
449+
450+
// Set display orientation from an untrusted process. This should fail silently.
451+
{
452+
UIDFaker f{AID_BIN};
453+
Transaction transaction;
454+
Rect layerStackRect;
455+
Rect displayRect;
456+
transaction.setDisplayProjection(display, initialOrientation + ui::ROTATION_90,
457+
layerStackRect, displayRect);
458+
transaction.apply(/*synchronous=*/true);
459+
}
460+
461+
// Verify that the display orientation did not change.
462+
ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayState(display, &displayState));
463+
ASSERT_EQ(initialOrientation, displayState.orientation);
464+
465+
// Set display orientation from a trusted process.
466+
{
467+
UIDFaker f{AID_SYSTEM};
468+
Transaction transaction;
469+
Rect layerStackRect;
470+
Rect displayRect;
471+
transaction.setDisplayProjection(display, initialOrientation + ui::ROTATION_90,
472+
layerStackRect, displayRect);
473+
transaction.apply(/*synchronous=*/true);
474+
}
475+
476+
// Verify that the display orientation did change.
477+
ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayState(display, &displayState));
478+
ASSERT_EQ(initialOrientation + ui::ROTATION_90, displayState.orientation);
479+
480+
// Reset orientation
481+
{
482+
UIDFaker f{AID_SYSTEM};
483+
Transaction transaction;
484+
Rect layerStackRect;
485+
Rect displayRect;
486+
transaction.setDisplayProjection(display, initialOrientation, layerStackRect, displayRect);
487+
transaction.apply(/*synchronous=*/true);
488+
}
489+
ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayState(display, &displayState));
490+
ASSERT_EQ(initialOrientation, displayState.orientation);
491+
}
492+
442493
} // namespace android
443494

444495
// TODO(b/129481165): remove the #pragma below and fix conversion issues

‎services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,7 @@ class TestableSurfaceFlinger {
468468

469469
auto setTransactionState(
470470
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
471-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
471+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
472472
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
473473
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
474474
bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,

0 commit comments

Comments
 (0)