Skip to content

Commit 8e2ca26

Browse files
patrickdwilliamspatrickdwilliams
committed
Fix DisplayState sanitization.
Bug: 347307756 Change-Id: I99eab98164814eb596a40b3a6c3c17fc76940043 Flag: EXEMPT bugfix Test: CredentialsTest Merged-In: Ia86633bac196a90aacd0e0aba04b7335a3bb81df Merged-In: I7d227dedaa13a1a31ebf9ace073c792287f35305
1 parent 37b340f commit 8e2ca26

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
@@ -113,7 +113,7 @@ class ISurfaceComposer: public IInterface {
113113
/* open/close transactions. requires ACCESS_SURFACE_FLINGER permission */
114114
virtual status_t setTransactionState(
115115
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
116-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
116+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
117117
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
118118
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffer,
119119
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
@@ -4502,7 +4502,7 @@ bool SurfaceFlinger::shouldLatchUnsignaled(const sp<Layer>& layer, const layer_s
45024502

45034503
status_t SurfaceFlinger::setTransactionState(
45044504
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
4505-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
4505+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
45064506
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp,
45074507
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
45084508
const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
@@ -4517,7 +4517,7 @@ status_t SurfaceFlinger::setTransactionState(
45174517
composerState.state.sanitize(permissions);
45184518
}
45194519

4520-
for (DisplayState display : displays) {
4520+
for (DisplayState& display : displays) {
45214521
display.sanitize(permissions);
45224522
}
45234523

‎services/surfaceflinger/SurfaceFlinger.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ class SurfaceFlinger : public BnSurfaceComposer,
517517
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
518518
status_t setTransactionState(
519519
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
520-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
520+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
521521
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
522522
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
523523
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)