From 1371c57f6f597b8d3aec922e2b46437a026c8d3d Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 13 Feb 2020 13:41:53 +0800 Subject: [PATCH] Ensure clearCallingIdentity for getDisplayContentOrCreate If a new display is created, it may acquire wakelock for freezing display when configuring the display. That may raise an exception that the calling uid is not the same as the owner uid. Fix: 147778342 Test: atest DisplayPolicyTests Change-Id: Ib31c0d1f98964874a0ab97725d43f6bbd009062c --- .../com/android/server/wm/DisplayPolicy.java | 29 ++-- .../server/wm/WindowManagerService.java | 145 +++++++++--------- .../server/wm/DisplayPolicyTestsBase.java | 7 +- 3 files changed, 95 insertions(+), 86 deletions(-) diff --git a/services/core/java/com/android/server/wm/DisplayPolicy.java b/services/core/java/com/android/server/wm/DisplayPolicy.java index ca6bd2d1e3b4d..386c10612375a 100644 --- a/services/core/java/com/android/server/wm/DisplayPolicy.java +++ b/services/core/java/com/android/server/wm/DisplayPolicy.java @@ -937,17 +937,17 @@ public class DisplayPolicy { * @return If ok, WindowManagerImpl.ADD_OKAY. If too many singletons, * WindowManagerImpl.ADD_MULTIPLE_SINGLETON */ - int validateAddingWindowLw(WindowManager.LayoutParams attrs) { + int validateAddingWindowLw(WindowManager.LayoutParams attrs, int callingPid, int callingUid) { if ((attrs.privateFlags & PRIVATE_FLAG_IS_SCREEN_DECOR) != 0) { - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, "DisplayPolicy"); } switch (attrs.type) { case TYPE_STATUS_BAR: - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, "DisplayPolicy"); if (mStatusBar != null) { if (mStatusBar.isAlive()) { @@ -956,8 +956,8 @@ public class DisplayPolicy { } break; case TYPE_NOTIFICATION_SHADE: - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, "DisplayPolicy"); if (mNotificationShade != null) { if (mNotificationShade.isAlive()) { @@ -966,8 +966,8 @@ public class DisplayPolicy { } break; case TYPE_NAVIGATION_BAR: - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, "DisplayPolicy"); if (mNavigationBar != null) { if (mNavigationBar.isAlive()) { @@ -977,16 +977,17 @@ public class DisplayPolicy { break; case TYPE_NAVIGATION_BAR_PANEL: // Check for permission if the caller is not the recents component. - if (!mService.mAtmInternal.isCallerRecents(Binder.getCallingUid())) { - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, "DisplayPolicy"); + if (!mService.mAtmInternal.isCallerRecents(callingUid)) { + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, + "DisplayPolicy"); } break; case TYPE_STATUS_BAR_PANEL: case TYPE_STATUS_BAR_SUB_PANEL: case TYPE_VOICE_INTERACTION_STARTING: - mContext.enforceCallingOrSelfPermission( - android.Manifest.permission.STATUS_BAR_SERVICE, + mContext.enforcePermission( + android.Manifest.permission.STATUS_BAR_SERVICE, callingPid, callingUid, "DisplayPolicy"); break; } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 8f9caea265349..c67d840e76ad6 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -1342,8 +1342,9 @@ public class WindowManagerService extends IWindowManager.Stub } WindowState parentWindow = null; - long origId; final int callingUid = Binder.getCallingUid(); + final int callingPid = Binder.getCallingPid(); + final long origId = Binder.clearCallingIdentity(); final int type = attrs.type; synchronized (mGlobalLock) { @@ -1504,10 +1505,9 @@ public class WindowManagerService extends IWindowManager.Stub } final DisplayPolicy displayPolicy = displayContent.getDisplayPolicy(); - displayPolicy.adjustWindowParamsLw(win, win.mAttrs, Binder.getCallingPid(), - Binder.getCallingUid()); + displayPolicy.adjustWindowParamsLw(win, win.mAttrs, callingPid, callingUid); - res = displayPolicy.validateAddingWindowLw(attrs); + res = displayPolicy.validateAddingWindowLw(attrs, callingPid, callingUid); if (res != WindowManagerGlobal.ADD_OKAY) { return res; } @@ -1559,8 +1559,6 @@ public class WindowManagerService extends IWindowManager.Stub displayContent.mTapExcludedWindows.add(win); } - origId = Binder.clearCallingIdentity(); - win.attach(); mWindowMap.put(client.asBinder(), win); win.initAppOpsState(); @@ -2569,34 +2567,41 @@ public class WindowManagerService extends IWindowManager.Stub } } - synchronized (mGlobalLock) { - if (!callerCanManageAppTokens) { - if (!unprivilegedAppCanCreateTokenWith(null, Binder.getCallingUid(), type, type, - null, packageName) || packageName == null) { - throw new SecurityException("Requires MANAGE_APP_TOKENS permission"); + final int callingUid = Binder.getCallingUid(); + final long origId = Binder.clearCallingIdentity(); + try { + synchronized (mGlobalLock) { + if (!callerCanManageAppTokens) { + if (packageName == null || !unprivilegedAppCanCreateTokenWith( + null /* parentWindow */, callingUid, type, type, null /* tokenForLog */, + packageName)) { + throw new SecurityException("Requires MANAGE_APP_TOKENS permission"); + } + } + + final DisplayContent dc = getDisplayContentOrCreate(displayId, null /* token */); + if (dc == null) { + ProtoLog.w(WM_ERROR, "addWindowToken: Attempted to add token: %s" + + " for non-exiting displayId=%d", binder, displayId); + return WindowManagerGlobal.ADD_INVALID_DISPLAY; + } + + WindowToken token = dc.getWindowToken(binder); + if (token != null) { + ProtoLog.w(WM_ERROR, "addWindowToken: Attempted to add binder token: %s" + + " for already created window token: %s" + + " displayId=%d", binder, token, displayId); + return WindowManagerGlobal.ADD_DUPLICATE_ADD; + } + // TODO(window-container): Clean up dead tokens + if (type == TYPE_WALLPAPER) { + new WallpaperWindowToken(this, binder, true, dc, callerCanManageAppTokens); + } else { + new WindowToken(this, binder, type, true, dc, callerCanManageAppTokens); } } - - final DisplayContent dc = getDisplayContentOrCreate(displayId, null /* token */); - if (dc == null) { - ProtoLog.w(WM_ERROR, "addWindowToken: Attempted to add token: %s" - + " for non-exiting displayId=%d", binder, displayId); - return WindowManagerGlobal.ADD_INVALID_DISPLAY; - } - - WindowToken token = dc.getWindowToken(binder); - if (token != null) { - ProtoLog.w(WM_ERROR, "addWindowToken: Attempted to add binder token: %s" - + " for already created window token: %s" - + " displayId=%d", binder, token, displayId); - return WindowManagerGlobal.ADD_DUPLICATE_ADD; - } - // TODO(window-container): Clean up dead tokens - if (type == TYPE_WALLPAPER) { - new WallpaperWindowToken(this, binder, true, dc, callerCanManageAppTokens); - } else { - new WindowToken(this, binder, type, true, dc, callerCanManageAppTokens); - } + } finally { + Binder.restoreCallingIdentity(origId); } return WindowManagerGlobal.ADD_OKAY; } @@ -6843,34 +6848,32 @@ public class WindowManagerService extends IWindowManager.Stub throw new SecurityException("Requires INTERNAL_SYSTEM_WINDOW permission"); } - synchronized (mGlobalLock) { - final DisplayContent displayContent = getDisplayContentOrCreate(displayId, null); - if (displayContent == null) { - ProtoLog.w(WM_ERROR, - "Attempted to set windowing mode to a display that does not exist: %d", - displayId); - return; - } - - int lastWindowingMode = displayContent.getWindowingMode(); - mDisplayWindowSettings.setWindowingModeLocked(displayContent, mode); - - displayContent.reconfigureDisplayLocked(); - - if (lastWindowingMode != displayContent.getWindowingMode()) { - // reconfigure won't detect this change in isolation because the windowing mode is - // already set on the display, so fire off a new config now. - - final long origId = Binder.clearCallingIdentity(); - try { - // direct call since lock is shared. - displayContent.sendNewConfiguration(); - } finally { - Binder.restoreCallingIdentity(origId); + final long origId = Binder.clearCallingIdentity(); + try { + synchronized (mGlobalLock) { + final DisplayContent displayContent = getDisplayContentOrCreate(displayId, null); + if (displayContent == null) { + ProtoLog.w(WM_ERROR, + "Attempted to set windowing mode to a display that does not exist: %d", + displayId); + return; + } + + int lastWindowingMode = displayContent.getWindowingMode(); + mDisplayWindowSettings.setWindowingModeLocked(displayContent, mode); + + displayContent.reconfigureDisplayLocked(); + + if (lastWindowingMode != displayContent.getWindowingMode()) { + // reconfigure won't detect this change in isolation because the windowing mode + // is already set on the display, so fire off a new config now. + displayContent.sendNewConfiguration(); + // Now that all configurations are updated, execute pending transitions. + displayContent.executeAppTransition(); } - // Now that all configurations are updated, execute pending transitions - displayContent.executeAppTransition(); } + } finally { + Binder.restoreCallingIdentity(origId); } } @@ -6898,18 +6901,22 @@ public class WindowManagerService extends IWindowManager.Stub throw new SecurityException("Requires INTERNAL_SYSTEM_WINDOW permission"); } - synchronized (mGlobalLock) { - final DisplayContent displayContent = getDisplayContentOrCreate(displayId, null); - if (displayContent == null) { - ProtoLog.w(WM_ERROR, - "Attempted to set remove mode to a display that does not exist: %d", - displayId); - return; + final long origId = Binder.clearCallingIdentity(); + try { + synchronized (mGlobalLock) { + final DisplayContent displayContent = getDisplayContentOrCreate(displayId, null); + if (displayContent == null) { + ProtoLog.w(WM_ERROR, + "Attempted to set remove mode to a display that does not exist: %d", + displayId); + return; + } + + mDisplayWindowSettings.setRemoveContentModeLocked(displayContent, mode); + displayContent.reconfigureDisplayLocked(); } - - mDisplayWindowSettings.setRemoveContentModeLocked(displayContent, mode); - - displayContent.reconfigureDisplayLocked(); + } finally { + Binder.restoreCallingIdentity(origId); } } diff --git a/services/tests/wmtests/src/com/android/server/wm/DisplayPolicyTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/DisplayPolicyTestsBase.java index 2a20c2e959ccb..7ba3fd815b2d0 100644 --- a/services/tests/wmtests/src/com/android/server/wm/DisplayPolicyTestsBase.java +++ b/services/tests/wmtests/src/com/android/server/wm/DisplayPolicyTestsBase.java @@ -102,10 +102,11 @@ public class DisplayPolicyTestsBase extends WindowTestsBase { } void addWindow(WindowState win) { - mDisplayPolicy.adjustWindowParamsLw(win, win.mAttrs, Binder.getCallingPid(), - Binder.getCallingUid()); + final int callingPid = Binder.getCallingPid(); + final int callingUid = Binder.getCallingUid(); + mDisplayPolicy.adjustWindowParamsLw(win, win.mAttrs, callingPid, callingUid); assertEquals(WindowManagerGlobal.ADD_OKAY, - mDisplayPolicy.validateAddingWindowLw(win.mAttrs)); + mDisplayPolicy.validateAddingWindowLw(win.mAttrs, callingPid, callingUid)); mDisplayPolicy.addWindowLw(win, win.mAttrs); win.mHasSurface = true; }