Skip to content

Commit f185447

Browse files
patrickdwilliamspatrickdwilliams
committed
Fix DisplayState sanitization.
Bug: 347307756 Change-Id: Ia86633bac196a90aacd0e0aba04b7335a3bb81df Flag: EXEMPT bugfix Test: CredentialsTest Merged-In: I7d227dedaa13a1a31ebf9ace073c792287f35305
1 parent f460b8b commit f185447

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
@@ -62,7 +62,7 @@ class BpSurfaceComposer : public BpInterface<ISurfaceComposer>
6262

6363
status_t setTransactionState(
6464
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
65-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
65+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
6666
InputWindowCommands commands, int64_t desiredPresentTime, bool isAutoTimestamp,
6767
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
6868
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
@@ -701,7 +701,7 @@ class FakeSurfaceComposer : public ISurfaceComposer {
701701

702702
status_t setTransactionState(
703703
const FrameTimelineInfo& /*frameTimelineInfo*/, Vector<ComposerState>& /*state*/,
704-
const Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
704+
Vector<DisplayState>& /*displays*/, uint32_t /*flags*/,
705705
const sp<IBinder>& /*applyToken*/, InputWindowCommands /*inputWindowCommands*/,
706706
int64_t /*desiredPresentTime*/, bool /*isAutoTimestamp*/,
707707
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
@@ -4498,7 +4498,7 @@ bool SurfaceFlinger::shouldLatchUnsignaled(const sp<Layer>& layer, const layer_s
44984498

44994499
status_t SurfaceFlinger::setTransactionState(
45004500
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
4501-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
4501+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
45024502
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime, bool isAutoTimestamp,
45034503
const std::vector<client_cache_t>& uncacheBuffers, bool hasListenerCallbacks,
45044504
const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId,
@@ -4513,7 +4513,7 @@ status_t SurfaceFlinger::setTransactionState(
45134513
composerState.state.sanitize(permissions);
45144514
}
45154515

4516-
for (DisplayState display : displays) {
4516+
for (DisplayState& display : displays) {
45174517
display.sanitize(permissions);
45184518
}
45194519

‎services/surfaceflinger/SurfaceFlinger.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ class SurfaceFlinger : public BnSurfaceComposer,
523523
sp<IBinder> getPhysicalDisplayToken(PhysicalDisplayId displayId) const;
524524
status_t setTransactionState(
525525
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& state,
526-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
526+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
527527
InputWindowCommands inputWindowCommands, int64_t desiredPresentTime,
528528
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
529529
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
@@ -743,7 +743,7 @@ class TestableSurfaceFlinger final : private scheduler::ISchedulerCallback {
743743

744744
auto setTransactionState(
745745
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
746-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
746+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
747747
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
748748
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
749749
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
@@ -503,7 +503,7 @@ class TestableSurfaceFlinger {
503503

504504
auto setTransactionState(
505505
const FrameTimelineInfo& frameTimelineInfo, Vector<ComposerState>& states,
506-
const Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
506+
Vector<DisplayState>& displays, uint32_t flags, const sp<IBinder>& applyToken,
507507
const InputWindowCommands& inputWindowCommands, int64_t desiredPresentTime,
508508
bool isAutoTimestamp, const std::vector<client_cache_t>& uncacheBuffers,
509509
bool hasListenerCallbacks, std::vector<ListenerCallbacks>& listenerCallbacks,

0 commit comments

Comments
 (0)