DO NOT MERGE: SurfaceFlinger: Add Transaction#sanitize am: 1ff38ab351 (b71b9032) · Commits · e / os / platform_frameworks_native

libs/gui/LayerState.cpp

+65 −0

Original line number	Diff line number	Diff line
		@@ -381,6 +381,71 @@ void DisplayState::merge(const DisplayState& other) {
		}
		}

		void layer_state_t::sanitize(int32_t permissions) {
		// TODO: b/109894387
		//
		// SurfaceFlinger's renderer is not prepared to handle cropping in the face of arbitrary
		// rotation. To see the problem observe that if we have a square parent, and a child
		// of the same size, then we rotate the child 45 degrees around its center, the child
		// must now be cropped to a non rectangular 8 sided region.
		//
		// Of course we can fix this in the future. For now, we are lucky, SurfaceControl is
		// private API, and arbitrary rotation is used in limited use cases, for instance:
		// - WindowManager only uses rotation in one case, which is on a top level layer in which
		// cropping is not an issue.
		// - Launcher, as a privileged app, uses this to transition an application to PiP
		// (picture-in-picture) mode.
		//
		// However given that abuse of rotation matrices could lead to surfaces extending outside
		// of cropped areas, we need to prevent non-root clients without permission
		// ACCESS_SURFACE_FLINGER nor ROTATE_SURFACE_FLINGER
		// (a.k.a. everyone except WindowManager / tests / Launcher) from setting non rectangle
		// preserving transformations.
		if (what & eMatrixChanged) {
		if (!(permissions & Permission::ROTATE_SURFACE_FLINGER)) {
		ui::Transform t;
		t.set(matrix.dsdx, matrix.dtdy, matrix.dtdx, matrix.dsdy);
		if (!t.preserveRects()) {
		what &= ~eMatrixChanged;
		ALOGE("Stripped non rect preserving matrix in sanitize");
		}
		}
		}

		if (what & layer_state_t::eInputInfoChanged) {
		if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
		what &= ~eInputInfoChanged;
		ALOGE("Stripped attempt to set eInputInfoChanged in sanitize");
		}
		}
		if (what & layer_state_t::eTrustedOverlayChanged) {
		if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
		what &= ~eTrustedOverlayChanged;
		ALOGE("Stripped attempt to set eTrustedOverlay in sanitize");
		}
		}
		if (what & layer_state_t::eDropInputModeChanged) {
		if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
		what &= ~eDropInputModeChanged;
		ALOGE("Stripped attempt to set eDropInputModeChanged in sanitize");
		}
		}
		if (what & layer_state_t::eFrameRateSelectionPriority) {
		if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
		what &= ~eFrameRateSelectionPriority;
		ALOGE("Stripped attempt to set eFrameRateSelectionPriority in sanitize");
		}
		}
		if (what & layer_state_t::eFrameRateChanged) {
		if (!ValidateFrameRate(frameRate, frameRateCompatibility,
		changeFrameRateStrategy,
		"layer_state_t::sanitize",
		permissions & Permission::ACCESS_SURFACE_FLINGER)) {
		what &= ~eFrameRateChanged; // logged in ValidateFrameRate
		}
		}
		}

		void layer_state_t::merge(const layer_state_t& other) {
		if (other.what & ePositionChanged) {
		what \|= ePositionChanged;

libs/gui/SurfaceComposerClient.cpp

+7 −1

Original line number	Diff line number	Diff line
		@@ -527,6 +527,13 @@ SurfaceComposerClient::Transaction::Transaction(const Transaction& other)
		mListenerCallbacks = other.mListenerCallbacks;
		}

		void SurfaceComposerClient::Transaction::sanitize() {
		for (auto & [handle, composerState] : mComposerStates) {
		composerState.state.sanitize(0 /* permissionMask */);
		}
		mInputWindowCommands.clear();
		}

		std::unique_ptr<SurfaceComposerClient::Transaction>
		SurfaceComposerClient::Transaction::createFromParcel(const Parcel* parcel) {
		auto transaction = std::make_unique<Transaction>();
		@@ -611,7 +618,6 @@ status_t SurfaceComposerClient::Transaction::readFromParcel(const Parcel* parcel
		if (composerState.read(*parcel) == BAD_VALUE) {
		return BAD_VALUE;
		}

		composerStates[surfaceControlHandle] = composerState;
		}

libs/gui/include/gui/LayerState.h

+7 −0

Original line number	Diff line number	Diff line
		@@ -63,6 +63,12 @@ struct client_cache_t {
		* Used to communicate layer information between SurfaceFlinger and its clients.
		*/
		struct layer_state_t {
		enum Permission {
		ACCESS_SURFACE_FLINGER = 0x1,
		ROTATE_SURFACE_FLINGER = 0x2,
		INTERNAL_SYSTEM_WINDOW = 0x4,
		};

		enum {
		eLayerHidden = 0x01, // SURFACE_HIDDEN in SurfaceControl.java
		eLayerOpaque = 0x02, // SURFACE_OPAQUE
		@@ -130,6 +136,7 @@ struct layer_state_t {
		status_t read(const Parcel& input);
		bool hasBufferChanges() const;
		bool hasValidBuffer() const;
		void sanitize(int32_t permissions);

		struct matrix22_t {
		float dsdx{0};

libs/gui/include/gui/SurfaceComposerClient.h

+8 −0

Original line number	Diff line number	Diff line
		@@ -587,6 +587,14 @@ public:
		void setAnimationTransaction();
		void setEarlyWakeupStart();
		void setEarlyWakeupEnd();

		/**
		* Strip the transaction of all permissioned requests, required when
		* accepting transactions across process boundaries.
		*
		* TODO (b/213644870): Remove all permissioned things from Transaction
		*/
		void sanitize();
		};

		status_t clearLayerFrameStats(const sp<IBinder>& token) const;