From 372088d7721f4c8cb7260d8e9fdaf498c5a30464 Mon Sep 17 00:00:00 2001 From: Jorim Jaggi Date: Thu, 20 May 2021 18:35:30 +0200 Subject: [PATCH 01/10] Fix race condition between lockNow() and updateLockscreenTimeout If updateLockscreenTimeout gets called before the Runnable queued from lockNow gets executed, lockNow request will be ignored. Fix this by not clearing out the runnable if it's pending lock request. Test: Switch user, ensure lockscreen comes up Bug: 161149543 Change-Id: Ie486396fd7328edf8ca0912df92524bb82a1fb7f (cherry picked from commit 875fa991aac0f3bbd5c66327408ceae60a24a6b3) Merged-In: Ie486396fd7328edf8ca0912df92524bb82a1fb7f (cherry picked from commit 9c8b1512a532478dea055d82ad6a49d53a9f31b1) --- .../android/server/policy/PhoneWindowManager.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index f0439a3db4fc0..b65a310c55ba6 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -608,6 +608,8 @@ public class PhoneWindowManager implements WindowManagerPolicy { private int mPowerButtonSuppressionDelayMillis = POWER_BUTTON_SUPPRESSION_DELAY_DEFAULT_MILLIS; + private boolean mLockNowPending = false; + private static final int MSG_DISPATCH_MEDIA_KEY_WITH_WAKE_LOCK = 3; private static final int MSG_DISPATCH_MEDIA_KEY_REPEAT_WITH_WAKE_LOCK = 4; private static final int MSG_KEYGUARD_DRAWN_COMPLETE = 5; @@ -4941,6 +4943,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { mKeyguardDelegate.doKeyguardTimeout(options); } mLockScreenTimerActive = false; + mLockNowPending = false; options = null; } } @@ -4950,7 +4953,7 @@ public class PhoneWindowManager implements WindowManagerPolicy { } } - ScreenLockTimeout mScreenLockTimeout = new ScreenLockTimeout(); + final ScreenLockTimeout mScreenLockTimeout = new ScreenLockTimeout(); @Override public void lockNow(Bundle options) { @@ -4962,6 +4965,9 @@ public class PhoneWindowManager implements WindowManagerPolicy { mScreenLockTimeout.setLockOptions(options); } mHandler.post(mScreenLockTimeout); + synchronized (mScreenLockTimeout) { + mLockNowPending = true; + } } // TODO (b/113840485): Move this logic to DisplayPolicy when lockscreen supports multi-display. @@ -4977,6 +4983,10 @@ public class PhoneWindowManager implements WindowManagerPolicy { private void updateLockScreenTimeout() { synchronized (mScreenLockTimeout) { + if (mLockNowPending) { + Log.w(TAG, "lockNow pending, ignore updating lockscreen timeout"); + return; + } final boolean enable = !mAllowLockscreenWhenOnDisplays.isEmpty() && mDefaultDisplayPolicy.isAwake() && mKeyguardDelegate != null && mKeyguardDelegate.isSecure(mCurrentUserId); From 49cb41f9d7d2ec741bc2ec27717283dc560a9060 Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 22 Jun 2021 12:38:56 -0700 Subject: [PATCH 02/10] Remove ParsedIntentInfo CREATOR Its existence allows implicit readParcelable calls to invoke a Parcel operation with mismatched read/write data sizes, allowing someone to swap out the data on a reparcel. Internal classes will use writeIntentInfoToParcel, so this is safe to remove. Bug: 191055353 Test: atest com.android.server.pm.test.parsing.parcelling Change-Id: I44faa635faf8a77894a3dda8adf89c10064e53f1 (cherry picked from commit 75214cc510c62f936a713c2da3d0a54db9405957) --- .../pm/parsing/component/ParsedIntentInfo.java | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/core/java/android/content/pm/parsing/component/ParsedIntentInfo.java b/core/java/android/content/pm/parsing/component/ParsedIntentInfo.java index 0ba92cc4fef78..504a7bd5a320c 100644 --- a/core/java/android/content/pm/parsing/component/ParsedIntentInfo.java +++ b/core/java/android/content/pm/parsing/component/ParsedIntentInfo.java @@ -19,7 +19,6 @@ package android.content.pm.parsing.component; import android.annotation.Nullable; import android.content.IntentFilter; import android.os.Parcel; -import android.os.Parcelable; import android.util.Pair; import com.android.internal.util.DataClass; @@ -168,19 +167,6 @@ public final class ParsedIntentInfo extends IntentFilter { + '}'; } - public static final Parcelable.Creator CREATOR = - new Parcelable.Creator() { - @Override - public ParsedIntentInfo createFromParcel(Parcel source) { - return new ParsedIntentInfo(source); - } - - @Override - public ParsedIntentInfo[] newArray(int size) { - return new ParsedIntentInfo[size]; - } - }; - public boolean isHasDefault() { return hasDefault; } From 7f39ba09b8ccad2ae50874d3643cdc93746391ea Mon Sep 17 00:00:00 2001 From: Hai Zhang Date: Wed, 16 Jun 2021 00:20:52 +0000 Subject: [PATCH 03/10] DO NOT MERGE Add cross-user check for getDefaultSmsPackage(). Bug: 177927831 Test: atest RoleSecurityTest Change-Id: I1254804fb72a299e782d45f938acdf979a82f904 (cherry picked from commit cf1bd25a37123624688f9965608a48d362ab4eb0) --- .../java/com/android/server/role/RoleManagerService.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/services/core/java/com/android/server/role/RoleManagerService.java b/services/core/java/com/android/server/role/RoleManagerService.java index 392792dbae69c..b75bce833e04d 100644 --- a/services/core/java/com/android/server/role/RoleManagerService.java +++ b/services/core/java/com/android/server/role/RoleManagerService.java @@ -662,6 +662,12 @@ public class RoleManagerService extends SystemService implements RoleUserState.C @Override public String getDefaultSmsPackage(int userId) { + userId = handleIncomingUser(userId, false, "getDefaultSmsPackage"); + if (!mUserManagerInternal.exists(userId)) { + Slog.e(LOG_TAG, "user " + userId + " does not exist"); + return null; + } + long identity = Binder.clearCallingIdentity(); try { return CollectionUtils.firstOrNull( From 4241ab5ee435ee3c5e6496c001b2cf5bc827cfc4 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Mon, 7 Jun 2021 15:02:45 -0700 Subject: [PATCH 04/10] Fix side effects of trace-ipc and dumpheap commands These shell commands were implicitly deleting any client-named file for which the system uid had deletion capability. They no longer do this, instead using only the client's own capabilities and file manipulation modes. Bug: 185398942 Test: manual "adb shell cmd activity dumpheap system_server /data/system/last-fstrim" Test: atest CtsPermissionTestCases:ShellCommandPermissionTest Change-Id: Ie61ab2c3f4bfbd04de09ca99c1116d1129461e8f (cherry picked from commit 76e8e04703cb49a4984145a18f4552c4bcf72172) --- .../server/am/ActivityManagerShellCommand.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java index 149e3baa90e7a..122af56bd575c 100644 --- a/services/core/java/com/android/server/am/ActivityManagerShellCommand.java +++ b/services/core/java/com/android/server/am/ActivityManagerShellCommand.java @@ -94,7 +94,6 @@ import com.android.internal.util.MemInfoReader; import com.android.server.compat.PlatformCompat; import java.io.BufferedReader; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -787,8 +786,7 @@ final class ActivityManagerShellCommand extends ShellCommand { return -1; } - File file = new File(filename); - file.delete(); + // Writes an error message to stderr on failure ParcelFileDescriptor fd = openFileForSystem(filename, "w"); if (fd == null) { return -1; @@ -942,16 +940,16 @@ final class ActivityManagerShellCommand extends ShellCommand { String logNameTimeString = LOG_NAME_TIME_FORMATTER.format(localDateTime); heapFile = "/data/local/tmp/heapdump-" + logNameTimeString + ".prof"; } - pw.println("File: " + heapFile); - pw.flush(); - File file = new File(heapFile); - file.delete(); + // Writes an error message to stderr on failure ParcelFileDescriptor fd = openFileForSystem(heapFile, "w"); if (fd == null) { return -1; } + pw.println("File: " + heapFile); + pw.flush(); + final CountDownLatch latch = new CountDownLatch(1); final RemoteCallback finishCallback = new RemoteCallback(new OnResultListener() { From ed0c637a311d98426ff29cecd96aa429b53544dc Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Fri, 4 Jun 2021 16:07:23 -0700 Subject: [PATCH 05/10] Improve ellipsize performance Instead of iterate all ellipsized characters, only iterate the necessary ranges for copying. Bug: 188913943 Test: atest CtsTextTestCases CtsGraphicsTestCases CtsWidgetTestCases Change-Id: I3d03b1e3897e427c23fbe51315f412c57a4ce9e9 (cherry picked from commit 2c6121f3e3c52965ae33317e4fe7a273fd1742c6) (cherry picked from commit 40d7a4535aba93073264bf8998daf588f0923986) --- core/java/android/text/Layout.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core/java/android/text/Layout.java b/core/java/android/text/Layout.java index 8a4497a0f0ce6..6baea1aea4718 100644 --- a/core/java/android/text/Layout.java +++ b/core/java/android/text/Layout.java @@ -2350,7 +2350,10 @@ public abstract class Layout { final int ellipsisStringLen = ellipsisString.length(); // Use the ellipsis string only if there are that at least as many characters to replace. final boolean useEllipsisString = ellipsisCount >= ellipsisStringLen; - for (int i = 0; i < ellipsisCount; i++) { + final int min = Math.max(0, start - ellipsisStart - lineStart); + final int max = Math.min(ellipsisCount, end - ellipsisStart - lineStart); + + for (int i = min; i < max; i++) { final char c; if (useEllipsisString && i < ellipsisStringLen) { c = ellipsisString.charAt(i); @@ -2359,9 +2362,7 @@ public abstract class Layout { } final int a = i + ellipsisStart + lineStart; - if (start <= a && a < end) { - dest[destoff + a - start] = c; - } + dest[destoff + a - start] = c; } } From ba984d1c0c0270b6c704189abf4faa808c2038bf Mon Sep 17 00:00:00 2001 From: Louis Chang Date: Mon, 21 Jun 2021 00:33:10 +0800 Subject: [PATCH 06/10] Avoid locking profile task when it is already lock WorkLockActivity was started repeatedly on top of the task that contains work apps when turning screen on and off over and over. So, lots of the WorkLockActivity instances were created and added in the task, which caused system sluggish. Bug: 177457096 Test: manually test work challenges Test: RootWindowContainerTests Change-Id: Iac345471ef3badad6b9e5c0cc2873c60938663eb Merged-In: Iac345471ef3badad6b9e5c0cc2873c60938663eb (cherry picked from commit 805585ed1baa0ddeeab07aa1f77819333a73c93d) (cherry picked from commit db7d06c52903b8dccb7e8d9976a23c366430a42e) --- .../server/wm/RootWindowContainer.java | 10 ++++++++ .../server/wm/RootWindowContainerTests.java | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index ddad1dbd9b3d5..1c1360f6f3897 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -17,6 +17,7 @@ package com.android.server.wm; import static android.app.ActivityTaskManager.INVALID_TASK_ID; +import static android.app.KeyguardManager.ACTION_CONFIRM_DEVICE_CREDENTIAL_WITH_USER; import static android.app.WindowConfiguration.ACTIVITY_TYPE_ASSISTANT; import static android.app.WindowConfiguration.ACTIVITY_TYPE_DREAM; import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME; @@ -3381,6 +3382,15 @@ class RootWindowContainer extends WindowContainer mService.deferWindowLayout(); try { forAllLeafTasks(task -> { + final ActivityRecord top = task.topRunningActivity(); + if (top != null && !top.finishing + && ACTION_CONFIRM_DEVICE_CREDENTIAL_WITH_USER.equals(top.intent.getAction()) + && top.packageName.equals( + mService.getSysUiServiceComponentLocked().getPackageName())) { + // Do nothing since the task is already secure by sysui. + return; + } + if (task.getActivity(activity -> !activity.finishing && activity.mUserId == userId) != null) { mService.getTaskChangeNotificationController().notifyTaskProfileLocked( diff --git a/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java index 1aff8a7b53823..e5c5bf54b0e67 100644 --- a/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java @@ -16,6 +16,7 @@ package com.android.server.wm; +import static android.app.KeyguardManager.ACTION_CONFIRM_DEVICE_CREDENTIAL_WITH_USER; import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD; import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.view.Display.DEFAULT_DISPLAY; @@ -25,6 +26,7 @@ import static android.view.WindowManager.LayoutParams.TYPE_NOTIFICATION_SHADE; import static android.view.WindowManager.LayoutParams.TYPE_STATUS_BAR; import static android.view.WindowManager.LayoutParams.TYPE_TOAST; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.server.wm.ActivityStack.ActivityState.FINISHING; import static com.android.server.wm.ActivityStack.ActivityState.PAUSED; @@ -37,11 +39,15 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import android.app.WindowConfiguration; import android.content.ComponentName; +import android.content.Intent; import android.content.pm.ActivityInfo; import android.os.UserHandle; import android.platform.test.annotations.Presubmit; @@ -194,6 +200,8 @@ public class RootWindowContainerTests extends WindowTestsBase { .setUid(UserHandle.PER_USER_RANGE + 1) .setTask(task) .build(); + doReturn(true).when(topActivity).okToShowLocked(); + topActivity.intent.setAction(Intent.ACTION_MAIN); // Make sure the listeners will be notified for putting the task to locked state TaskChangeNotificationController controller = @@ -201,6 +209,23 @@ public class RootWindowContainerTests extends WindowTestsBase { spyOn(controller); mWm.mRoot.lockAllProfileTasks(0); verify(controller).notifyTaskProfileLocked(eq(task.mTaskId), eq(0)); + + // Create the work lock activity on top of the task + final ActivityRecord workLockActivity = + new ActivityTestsBase.ActivityBuilder(mWm.mAtmService) + .setStack(stack) + .setUid(UserHandle.PER_USER_RANGE + 1) + .setTask(task) + .build(); + doReturn(true).when(workLockActivity).okToShowLocked(); + workLockActivity.intent.setAction(ACTION_CONFIRM_DEVICE_CREDENTIAL_WITH_USER); + doReturn(workLockActivity.mActivityComponent).when( + mWm.mAtmService).getSysUiServiceComponentLocked(); + + // Make sure the listener won't be notified again. + clearInvocations(controller); + mWm.mRoot.lockAllProfileTasks(0); + verify(controller, never()).notifyTaskProfileLocked(eq(task.mTaskId), anyInt()); } } From d5b1cf4978be922dd87255312eb345453aa56ebf Mon Sep 17 00:00:00 2001 From: Hui Yu Date: Fri, 25 Jun 2021 17:32:20 +0000 Subject: [PATCH 07/10] Merge "BG-FGS-start while-in-use permission restriction improvement." into rvc-dev am: e51f884f6a Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/15081994 Bug: 183147114 Bug: 183204439 Test: atest cts/tests/app/src/android/app/cts/ActivityManagerFgsBgStartTest.java#testStartForegroundTimeout Test: atest cts/tests/app/src/android/app/cts/ActivityManagerFgsBgStartTest.java#testSecondStartForeground Change-Id: Ieaaa70884cd11cfa460ec2be6db5e40856ffccef Merged-In: Idc88f274c7a323d175d65bb47eca041772ae9bb7 (cherry picked from commit fd02546e52841dcd0ec194f70baa58cc377505ec) --- .../com/android/server/am/ActiveServices.java | 103 ++++++++++++++---- .../server/am/ActivityManagerConstants.java | 20 ++++ .../com/android/server/am/ServiceRecord.java | 6 + 3 files changed, 107 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 5abb87cedc717..1cd7115f8edcd 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -734,11 +734,8 @@ public final class ActiveServices { } ComponentName cmp = startServiceInnerLocked(smap, service, r, callerFg, addToStarting); - if (!r.mAllowWhileInUsePermissionInFgs) { - r.mAllowWhileInUsePermissionInFgs = - shouldAllowWhileInUsePermissionInFgsLocked(callingPackage, callingPid, - callingUid, service, r, allowBackgroundActivityStarts); - } + setFgsRestrictionLocked(callingPackage, callingPid, callingUid, r, + allowBackgroundActivityStarts); return cmp; } @@ -1411,14 +1408,6 @@ public final class ActiveServices { + String.format("0x%08X", manifestType) + " in service element of manifest file"); } - // If the foreground service is not started from TOP process, do not allow it to - // have while-in-use location/camera/microphone access. - if (!r.mAllowWhileInUsePermissionInFgs) { - Slog.w(TAG, - "Foreground service started from background can not have " - + "location/camera/microphone access: service " - + r.shortInstanceName); - } } boolean alreadyStartedOp = false; boolean stopProcStatsOp = false; @@ -1466,6 +1455,57 @@ public final class ActiveServices { ignoreForeground = true; } + if (!ignoreForeground) { + if (r.mStartForegroundCount == 0) { + /* + If the service was started with startService(), not + startForegroundService(), and if startForeground() isn't called within + mFgsStartForegroundTimeoutMs, then we check the state of the app + (who owns the service, which is the app that called startForeground()) + again. If the app is in the foreground, or in any other cases where + FGS-starts are allowed, then we still allow the FGS to be started. + Otherwise, startForeground() would fail. + + If the service was started with startForegroundService(), then the service + must call startForeground() within a timeout anyway, so we don't need this + check. + */ + if (!r.fgRequired) { + final long delayMs = SystemClock.elapsedRealtime() - r.createRealTime; + if (delayMs > mAm.mConstants.mFgsStartForegroundTimeoutMs) { + resetFgsRestrictionLocked(r); + setFgsRestrictionLocked(r.serviceInfo.packageName, r.app.pid, + r.appInfo.uid, r, false); + EventLog.writeEvent(0x534e4554, "183147114", + r.appInfo.uid, + "call setFgsRestrictionLocked again due to " + + "startForegroundTimeout"); + } + } + } else if (r.mStartForegroundCount >= 1) { + // The second or later time startForeground() is called after service is + // started. Check for app state again. + final long delayMs = SystemClock.elapsedRealtime() - + r.mLastSetFgsRestrictionTime; + if (delayMs > mAm.mConstants.mFgsStartForegroundTimeoutMs) { + resetFgsRestrictionLocked(r); + setFgsRestrictionLocked(r.serviceInfo.packageName, r.app.pid, + r.appInfo.uid, r, false); + EventLog.writeEvent(0x534e4554, "183147114", r.appInfo.uid, + "call setFgsRestrictionLocked for " + + (r.mStartForegroundCount + 1) + "th startForeground"); + } + } + // If the foreground service is not started from TOP process, do not allow it to + // have while-in-use location/camera/microphone access. + if (!r.mAllowWhileInUsePermissionInFgs) { + Slog.w(TAG, + "Foreground service started from background can not have " + + "location/camera/microphone access: service " + + r.shortInstanceName); + } + } + // Apps under strict background restrictions simply don't get to have foreground // services, so now that we've enforced the startForegroundService() contract // we only do the machinery of making the service foreground when the app @@ -1501,6 +1541,7 @@ public final class ActiveServices { active.mNumActive++; } r.isForeground = true; + r.mStartForegroundCount++; if (!stopProcStatsOp) { ServiceState stracker = r.getTracker(); if (stracker != null) { @@ -1559,6 +1600,7 @@ public final class ActiveServices { decActiveForegroundAppLocked(smap, r); } r.isForeground = false; + resetFgsRestrictionLocked(r); ServiceState stracker = r.getTracker(); if (stracker != null) { stracker.setForeground(false, mAm.mProcessStats.getMemFactorLocked(), @@ -2118,12 +2160,7 @@ public final class ActiveServices { } } - if (!s.mAllowWhileInUsePermissionInFgs) { - s.mAllowWhileInUsePermissionInFgs = - shouldAllowWhileInUsePermissionInFgsLocked(callingPackage, - callingPid, callingUid, - service, s, false); - } + setFgsRestrictionLocked(callingPackage, callingPid, callingUid, s, false); if (s.app != null) { if ((flags&Context.BIND_TREAT_LIKE_ACTIVITY) != 0) { @@ -3419,7 +3456,7 @@ public final class ActiveServices { r.isForeground = false; r.foregroundId = 0; r.foregroundNoti = null; - r.mAllowWhileInUsePermissionInFgs = false; + resetFgsRestrictionLocked(r); // Clear start entries. r.clearDeliveredStartsLocked(); @@ -4900,7 +4937,7 @@ public final class ActiveServices { * @return true if allow, false otherwise. */ private boolean shouldAllowWhileInUsePermissionInFgsLocked(String callingPackage, - int callingPid, int callingUid, Intent intent, ServiceRecord r, + int callingPid, int callingUid, ServiceRecord r, boolean allowBackgroundActivityStarts) { // Is the background FGS start restriction turned on? if (!mAm.mConstants.mFlagBackgroundFgsStartRestrictionEnabled) { @@ -4986,6 +5023,28 @@ public final class ActiveServices { boolean canAllowWhileInUsePermissionInFgsLocked(int callingPid, int callingUid, String callingPackage) { return shouldAllowWhileInUsePermissionInFgsLocked( - callingPackage, callingPid, callingUid, null, null, false); + callingPackage, callingPid, callingUid, null, false); + } + + /** + * In R, mAllowWhileInUsePermissionInFgs is to allow while-in-use permissions in foreground + * service or not. while-in-use permissions in FGS started from background might be restricted. + * @param callingPackage caller app's package name. + * @param callingUid caller app's uid. + * @param r the service to start. + * @return true if allow, false otherwise. + */ + private void setFgsRestrictionLocked(String callingPackage, + int callingPid, int callingUid, ServiceRecord r, + boolean allowBackgroundActivityStarts) { + r.mLastSetFgsRestrictionTime = SystemClock.elapsedRealtime(); + if (!r.mAllowWhileInUsePermissionInFgs) { + r.mAllowWhileInUsePermissionInFgs = shouldAllowWhileInUsePermissionInFgsLocked( + callingPackage, callingPid, callingUid, r, allowBackgroundActivityStarts); + } + } + + private void resetFgsRestrictionLocked(ServiceRecord r) { + r.mAllowWhileInUsePermissionInFgs = false; } } diff --git a/services/core/java/com/android/server/am/ActivityManagerConstants.java b/services/core/java/com/android/server/am/ActivityManagerConstants.java index 7be843f178634..00d8208ea1180 100644 --- a/services/core/java/com/android/server/am/ActivityManagerConstants.java +++ b/services/core/java/com/android/server/am/ActivityManagerConstants.java @@ -88,6 +88,7 @@ final class ActivityManagerConstants extends ContentObserver { static final String KEY_PROCESS_START_ASYNC = "process_start_async"; static final String KEY_MEMORY_INFO_THROTTLE_TIME = "memory_info_throttle_time"; static final String KEY_TOP_TO_FGS_GRACE_DURATION = "top_to_fgs_grace_duration"; + static final String KEY_FGS_START_FOREGROUND_TIMEOUT = "fgs_start_foreground_timeout"; static final String KEY_PENDINGINTENT_WARNING_THRESHOLD = "pendingintent_warning_threshold"; private static final int DEFAULT_MAX_CACHED_PROCESSES = 32; @@ -121,6 +122,7 @@ final class ActivityManagerConstants extends ContentObserver { private static final boolean DEFAULT_PROCESS_START_ASYNC = true; private static final long DEFAULT_MEMORY_INFO_THROTTLE_TIME = 5*60*1000; private static final long DEFAULT_TOP_TO_FGS_GRACE_DURATION = 15 * 1000; + private static final int DEFAULT_FGS_START_FOREGROUND_TIMEOUT_MS = 10 * 1000; private static final int DEFAULT_PENDINGINTENT_WARNING_THRESHOLD = 2000; // Flag stored in the DeviceConfig API. @@ -273,6 +275,12 @@ final class ActivityManagerConstants extends ContentObserver { // this long. public long TOP_TO_FGS_GRACE_DURATION = DEFAULT_TOP_TO_FGS_GRACE_DURATION; + /** + * When service started from background, before the timeout it can be promoted to FGS by calling + * Service.startForeground(). + */ + volatile long mFgsStartForegroundTimeoutMs = DEFAULT_FGS_START_FOREGROUND_TIMEOUT_MS; + // Indicates whether the activity starts logging is enabled. // Controlled by Settings.Global.ACTIVITY_STARTS_LOGGING_ENABLED volatile boolean mFlagActivityStartsLoggingEnabled; @@ -421,6 +429,9 @@ final class ActivityManagerConstants extends ContentObserver { case KEY_MIN_ASSOC_LOG_DURATION: updateMinAssocLogDuration(); break; + case KEY_FGS_START_FOREGROUND_TIMEOUT: + updateFgsStartForegroundTimeout(); + break; default: break; } @@ -697,6 +708,13 @@ final class ActivityManagerConstants extends ContentObserver { /* defaultValue */ DEFAULT_MIN_ASSOC_LOG_DURATION); } + private void updateFgsStartForegroundTimeout() { + mFgsStartForegroundTimeoutMs = DeviceConfig.getLong( + DeviceConfig.NAMESPACE_ACTIVITY_MANAGER, + KEY_FGS_START_FOREGROUND_TIMEOUT, + DEFAULT_FGS_START_FOREGROUND_TIMEOUT_MS); + } + void dump(PrintWriter pw) { pw.println("ACTIVITY MANAGER SETTINGS (dumpsys activity settings) " + Settings.Global.ACTIVITY_MANAGER_CONSTANTS + ":"); @@ -769,6 +787,8 @@ final class ActivityManagerConstants extends ContentObserver { pw.println(Arrays.toString(IMPERCEPTIBLE_KILL_EXEMPT_PACKAGES.toArray())); pw.print(" "); pw.print(KEY_MIN_ASSOC_LOG_DURATION); pw.print("="); pw.println(MIN_ASSOC_LOG_DURATION); + pw.print(" "); pw.print(KEY_FGS_START_FOREGROUND_TIMEOUT); pw.print("="); + pw.println(mFgsStartForegroundTimeoutMs); pw.println(); if (mOverrideMaxCachedProcesses >= 0) { diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 1b65dbac2294f..0e628289a09f2 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -142,6 +142,10 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN // allow while-in-use permissions in foreground service or not. // while-in-use permissions in FGS started from background might be restricted. boolean mAllowWhileInUsePermissionInFgs; + // The number of times Service.startForeground() is called; + int mStartForegroundCount; + // Last time mAllowWhileInUsePermissionInFgs is set. + long mLastSetFgsRestrictionTime; // the most recent package that start/bind this service. String mRecentCallingPackage; @@ -406,6 +410,8 @@ final class ServiceRecord extends Binder implements ComponentName.WithComponentN } pw.print(prefix); pw.print("allowWhileInUsePermissionInFgs="); pw.println(mAllowWhileInUsePermissionInFgs); + pw.print(prefix); pw.print("startForegroundCount="); + pw.println(mStartForegroundCount); pw.print(prefix); pw.print("recentCallingPackage="); pw.println(mRecentCallingPackage); if (delayed) { From 5f2616cce92192fdf529117a63c7946087ce9979 Mon Sep 17 00:00:00 2001 From: Steve Elliott Date: Tue, 22 Jun 2021 13:58:48 -0400 Subject: [PATCH 08/10] Don't attach private Notification to A11yEvent when user locked Fixes: 159624555 Test: manual, atest Change-Id: Ib44f1d3695d2b31bee4f8ccae3f948c83f3b40b6 Merged-In: Ib44f1d3695d2b31bee4f8ccae3f948c83f3b40b6 (cherry picked from commit 54fbccc2934eae844550d851480d5448c2542f1d) (cherry picked from commit 40574b32076bc1de5ef1a29daf693c3673e96ba1) --- .../NotificationManagerService.java | 35 +++++-- .../notification/BuzzBeepBlinkTest.java | 93 +++++++++++++++++++ 2 files changed, 121 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 97dc3b2b89fda..dc551e14b9701 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -123,6 +123,7 @@ import android.app.INotificationManager; import android.app.ITransientNotification; import android.app.ITransientNotificationCallback; import android.app.IUriGrantsManager; +import android.app.KeyguardManager; import android.app.Notification; import android.app.NotificationChannel; import android.app.NotificationChannelGroup; @@ -479,6 +480,8 @@ public class NotificationManagerService extends SystemService { final ArrayMap> mDelayedCancelations = new ArrayMap<>(); + private KeyguardManager mKeyguardManager; + // The last key in this list owns the hardware. ArrayList mLights = new ArrayList<>(); @@ -1725,6 +1728,11 @@ public class NotificationManagerService extends SystemService { mAudioManager = audioMananger; } + @VisibleForTesting + void setKeyguardManager(KeyguardManager keyguardManager) { + mKeyguardManager = keyguardManager; + } + @VisibleForTesting ShortcutHelper getShortcutHelper() { return mShortcutHelper; @@ -2330,6 +2338,7 @@ public class NotificationManagerService extends SystemService { mAudioManager = (AudioManager) getContext().getSystemService(Context.AUDIO_SERVICE); mAudioManagerInternal = getLocalService(AudioManagerInternal.class); mWindowManagerInternal = LocalServices.getService(WindowManagerInternal.class); + mKeyguardManager = getContext().getSystemService(KeyguardManager.class); mZenModeHelper.onSystemReady(); mRoleObserver = new RoleObserver(getContext().getSystemService(RoleManager.class), mPackageManager, getContext().getMainExecutor()); @@ -6902,7 +6911,6 @@ public class NotificationManagerService extends SystemService { boolean beep = false; boolean blink = false; - final Notification notification = record.getSbn().getNotification(); final String key = record.getKey(); // Should this notification make noise, vibe, or use the LED? @@ -6924,7 +6932,7 @@ public class NotificationManagerService extends SystemService { if (!record.isUpdate && record.getImportance() > IMPORTANCE_MIN && !suppressedByDnd) { - sendAccessibilityEvent(notification, record.getSbn().getPackageName()); + sendAccessibilityEvent(record); sentAccessibilityEvent = true; } @@ -6946,7 +6954,7 @@ public class NotificationManagerService extends SystemService { boolean hasAudibleAlert = hasValidSound || hasValidVibrate; if (hasAudibleAlert && !shouldMuteNotificationLocked(record)) { if (!sentAccessibilityEvent) { - sendAccessibilityEvent(notification, record.getSbn().getPackageName()); + sendAccessibilityEvent(record); sentAccessibilityEvent = true; } if (DBG) Slog.v(TAG, "Interrupting!"); @@ -7659,17 +7667,30 @@ public class NotificationManagerService extends SystemService { return (x < low) ? low : ((x > high) ? high : x); } - void sendAccessibilityEvent(Notification notification, CharSequence packageName) { + void sendAccessibilityEvent(NotificationRecord record) { if (!mAccessibilityManager.isEnabled()) { return; } - AccessibilityEvent event = + final Notification notification = record.getNotification(); + final CharSequence packageName = record.getSbn().getPackageName(); + final AccessibilityEvent event = AccessibilityEvent.obtain(AccessibilityEvent.TYPE_NOTIFICATION_STATE_CHANGED); event.setPackageName(packageName); event.setClassName(Notification.class.getName()); - event.setParcelableData(notification); - CharSequence tickerText = notification.tickerText; + final int visibilityOverride = record.getPackageVisibilityOverride(); + final int notifVisibility = visibilityOverride == NotificationManager.VISIBILITY_NO_OVERRIDE + ? notification.visibility : visibilityOverride; + final int userId = record.getUser().getIdentifier(); + final boolean needPublic = userId >= 0 && mKeyguardManager.isDeviceLocked(userId); + if (needPublic && notifVisibility != Notification.VISIBILITY_PUBLIC) { + // Emit the public version if we're on the lockscreen and this notification isn't + // publicly visible. + event.setParcelableData(notification.publicVersion); + } else { + event.setParcelableData(notification); + } + final CharSequence tickerText = notification.tickerText; if (!TextUtils.isEmpty(tickerText)) { event.getText().add(tickerText); } diff --git a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java index cfda07c552084..a21bfda585290 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/BuzzBeepBlinkTest.java @@ -46,6 +46,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.ActivityManager; +import android.app.KeyguardManager; import android.app.Notification; import android.app.Notification.Builder; import android.app.NotificationChannel; @@ -103,6 +104,8 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { NotificationUsageStats mUsageStats; @Mock IAccessibilityManager mAccessibilityService; + @Mock + KeyguardManager mKeyguardManager; NotificationRecordLoggerFake mNotificationRecordLogger = new NotificationRecordLoggerFake(); private InstanceIdSequence mNotificationInstanceIdSequence = new InstanceIdSequenceFake( 1 << 30); @@ -147,6 +150,7 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { when(mAudioManager.getStreamVolume(anyInt())).thenReturn(10); when(mAudioManager.getRingerModeInternal()).thenReturn(AudioManager.RINGER_MODE_NORMAL); when(mUsageStats.isAlertRateLimited(any())).thenReturn(false); + when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(false); long serviceReturnValue = IntPair.of( AccessibilityManager.STATE_FLAG_ACCESSIBILITY_ENABLED, @@ -168,6 +172,7 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { mService.setFallbackVibrationPattern(FALLBACK_VIBRATION_PATTERN); mService.setUsageStats(mUsageStats); mService.setAccessibilityManager(accessibilityManager); + mService.setKeyguardManager(mKeyguardManager); mService.mScreenOn = false; mService.mInCallStateOffHook = false; mService.mNotificationPulseEnabled = true; @@ -483,6 +488,94 @@ public class BuzzBeepBlinkTest extends UiServiceTestCase { assertNotEquals(-1, r.getLastAudiblyAlertedMs()); } + @Test + public void testLockedPrivateA11yRedaction() throws Exception { + NotificationRecord r = getBeepyNotification(); + r.setPackageVisibilityOverride(NotificationManager.VISIBILITY_NO_OVERRIDE); + r.getNotification().visibility = Notification.VISIBILITY_PRIVATE; + when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(true); + AccessibilityManager accessibilityManager = Mockito.mock(AccessibilityManager.class); + when(accessibilityManager.isEnabled()).thenReturn(true); + mService.setAccessibilityManager(accessibilityManager); + + mService.buzzBeepBlinkLocked(r); + + ArgumentCaptor eventCaptor = + ArgumentCaptor.forClass(AccessibilityEvent.class); + + verify(accessibilityManager, times(1)) + .sendAccessibilityEvent(eventCaptor.capture()); + + AccessibilityEvent event = eventCaptor.getValue(); + assertEquals(r.getNotification().publicVersion, event.getParcelableData()); + } + + @Test + public void testLockedOverridePrivateA11yRedaction() throws Exception { + NotificationRecord r = getBeepyNotification(); + r.setPackageVisibilityOverride(Notification.VISIBILITY_PRIVATE); + r.getNotification().visibility = Notification.VISIBILITY_PUBLIC; + when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(true); + AccessibilityManager accessibilityManager = Mockito.mock(AccessibilityManager.class); + when(accessibilityManager.isEnabled()).thenReturn(true); + mService.setAccessibilityManager(accessibilityManager); + + mService.buzzBeepBlinkLocked(r); + + ArgumentCaptor eventCaptor = + ArgumentCaptor.forClass(AccessibilityEvent.class); + + verify(accessibilityManager, times(1)) + .sendAccessibilityEvent(eventCaptor.capture()); + + AccessibilityEvent event = eventCaptor.getValue(); + assertEquals(r.getNotification().publicVersion, event.getParcelableData()); + } + + @Test + public void testLockedPublicA11yNoRedaction() throws Exception { + NotificationRecord r = getBeepyNotification(); + r.setPackageVisibilityOverride(NotificationManager.VISIBILITY_NO_OVERRIDE); + r.getNotification().visibility = Notification.VISIBILITY_PUBLIC; + when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(true); + AccessibilityManager accessibilityManager = Mockito.mock(AccessibilityManager.class); + when(accessibilityManager.isEnabled()).thenReturn(true); + mService.setAccessibilityManager(accessibilityManager); + + mService.buzzBeepBlinkLocked(r); + + ArgumentCaptor eventCaptor = + ArgumentCaptor.forClass(AccessibilityEvent.class); + + verify(accessibilityManager, times(1)) + .sendAccessibilityEvent(eventCaptor.capture()); + + AccessibilityEvent event = eventCaptor.getValue(); + assertEquals(r.getNotification(), event.getParcelableData()); + } + + @Test + public void testUnlockedPrivateA11yNoRedaction() throws Exception { + NotificationRecord r = getBeepyNotification(); + r.setPackageVisibilityOverride(NotificationManager.VISIBILITY_NO_OVERRIDE); + r.getNotification().visibility = Notification.VISIBILITY_PRIVATE; + when(mKeyguardManager.isDeviceLocked(anyInt())).thenReturn(false); + AccessibilityManager accessibilityManager = Mockito.mock(AccessibilityManager.class); + when(accessibilityManager.isEnabled()).thenReturn(true); + mService.setAccessibilityManager(accessibilityManager); + + mService.buzzBeepBlinkLocked(r); + + ArgumentCaptor eventCaptor = + ArgumentCaptor.forClass(AccessibilityEvent.class); + + verify(accessibilityManager, times(1)) + .sendAccessibilityEvent(eventCaptor.capture()); + + AccessibilityEvent event = eventCaptor.getValue(); + assertEquals(r.getNotification(), event.getParcelableData()); + } + @Test public void testBeepInsistently() throws Exception { NotificationRecord r = getInsistentBeepyNotification(); From e9a6ebf59258a4ca14f83b74f10113aaddaf2b33 Mon Sep 17 00:00:00 2001 From: Kweku Adams Date: Thu, 3 Jun 2021 16:07:56 -0700 Subject: [PATCH 09/10] Don't export HeapDumpProvider. Stop exporting HeapDumpProvider so apps can only access generated dumps when the user explicitly shares them. Bug: 184046948 Test: capture system heap dump in developer options and confirm test app get SecurityException if it tries to access the dump directly, but gets access when the dump is shared through the notification flow Change-Id: Ibdca7cde4f563baa39163869289da5b79fc3a6db (cherry picked from commit a60c62bcb74e0146820f75f1da49581d1709b63c) (cherry picked from commit 260d0a85ddf986d41340980ef5abf0a878d65d9c) --- packages/Shell/AndroidManifest.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/Shell/AndroidManifest.xml b/packages/Shell/AndroidManifest.xml index 570c2ab3efd1d..3e820c39665cc 100644 --- a/packages/Shell/AndroidManifest.xml +++ b/packages/Shell/AndroidManifest.xml @@ -346,7 +346,7 @@ + android:exported="false" /> Date: Fri, 25 Jun 2021 09:59:32 -0700 Subject: [PATCH 10/10] Use IntentFilter CREATOR directly for serializing ParsedIntentInfo ParsedIntentInfo's CRFEATOR was removed because it exposes a reparcelling vulnerability. This adjusts a system API that relied on the implicit parcelling read to instead use IntentFilter directly, ignoring the fields contained in the subclass. Bug: 192050390 Bug: 191055353 Test: manual, cannot repro crash after patch Merged-In: Ib12e0a959eb5a5d73d5832ff2eee26a30eed5ded Change-Id: Ib12e0a959eb5a5d73d5832ff2eee26a30eed5ded (cherry picked from commit 7ac9b1da731bdf6ed2f34e22d5da7030bc0f7d21) --- .../com/android/server/pm/PackageManagerService.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index c643307c5f51e..cde249fe2a724 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -14252,9 +14252,15 @@ public class PackageManagerService extends IPackageManager.Stub return new ParceledListSlice(result) { @Override protected void writeElement(IntentFilter parcelable, Parcel dest, int callFlags) { - // IntentFilter has final Parcelable methods, so redirect to the subclass - ((ParsedIntentInfo) parcelable).writeIntentInfoToParcel(dest, - callFlags); + parcelable.writeToParcel(dest, callFlags); + } + + @Override + protected void writeParcelableCreator(IntentFilter parcelable, Parcel dest) { + // All Parcel#writeParcelableCreator does is serialize the class name to + // access via reflection to grab its CREATOR. This does that manually, pointing + // to the parent IntentFilter so that all of the subclass fields are ignored. + dest.writeString(IntentFilter.class.getName()); } }; }