From 20303e05bf73796124ab70a279cf849b61b97905 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Sat, 22 Jan 2022 16:49:31 -0800 Subject: [PATCH] DO NOT MERGE: WM: Call Transaction#sanitize Various elements of the Transaction interface require a permission in order to apply. In particular the setTrustedOverlay and setInputWindowInfo fields. These permission checks are implemented by checking the PID and the UID of the process which sent the transaction. Unfortunately widespread use of transaction merging makes this inadequate. At the moment IWindowSession#finishDrawing seems to be the only boundary on which transactions move from client to system processes, and so we expose a sanitize method and use it from there to resolve the situation in an easily backportable way. Moving forward it likely make sense to move security sensitive interfaces off of Transaction. Most of the things behind permissions currently are not truly security sensitive, more of just a request not to use them. It was also considered to sanitize transactions at all process boundaries through writeToParcel, however this could be disruptive as previously permissioned processes (WM and SysUI) could freely exchange transactions. As the change needs to be backportable the lowest risk option was chosen. Bug: 213644870 Test: Existing tests pass Change-Id: I8d4a0ebe0cdfaed7ff1ad862724d49a14ed04478 (cherry picked from commit cdf139841567a625608b94339fa49cb895f6a69f) --- core/java/android/view/SurfaceControl.java | 8 ++++++++ core/jni/android_view_SurfaceControl.cpp | 7 +++++++ .../java/com/android/server/wm/WindowManagerService.java | 4 ++++ 3 files changed, 19 insertions(+) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index 960d23d7afb09..a686cbf257ef6 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -245,6 +245,7 @@ public final class SurfaceControl implements Parcelable { @SurfaceControl.BufferTransform int transformHint); private static native int nativeGetTransformHint(long nativeObject); private static native int nativeGetLayerId(long nativeObject); + private static native void nativeSanitize(long transactionObject); /** * Transforms that can be applied to buffers as they are displayed to a window. @@ -3543,6 +3544,13 @@ public final class SurfaceControl implements Parcelable { return this; } + /** + * @hide + */ + public void sanitize() { + nativeSanitize(mNativeObject); + } + /** * Merge the other transaction into this transaction, clearing the * other transaction as if it had been applied. diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 1452c67ae3c68..33fc92d60d62e 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -877,6 +877,11 @@ static void nativeSetDropInputMode(JNIEnv* env, jclass clazz, jlong transactionO transaction->setDropInputMode(ctrl, static_cast(mode)); } +static void nativeSanitize(JNIEnv* env, jclass clazz, jlong transactionObj) { + auto transaction = reinterpret_cast(transactionObj); + transaction->sanitize(); +} + static jlongArray nativeGetPhysicalDisplayIds(JNIEnv* env, jclass clazz) { const auto displayIds = SurfaceComposerClient::getPhysicalDisplayIds(); jlongArray array = env->NewLongArray(displayIds.size()); @@ -2030,6 +2035,8 @@ static const JNINativeMethod sSurfaceControlMethods[] = { (void*)nativeSetDropInputMode }, {"nativeGetLayerId", "(J)I", (void*)nativeGetLayerId }, + {"nativeSanitize", "(J)V", + (void*) nativeSanitize } // clang-format on }; diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 4258e073429ee..caad57590aba6 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -2656,6 +2656,10 @@ public class WindowManagerService extends IWindowManager.Stub void finishDrawingWindow(Session session, IWindow client, @Nullable SurfaceControl.Transaction postDrawTransaction) { + if (postDrawTransaction != null) { + postDrawTransaction.sanitize(); + } + final long origId = Binder.clearCallingIdentity(); try { synchronized (mGlobalLock) {