From f49416b2abe24a53a4593da4c099634abeee80da Mon Sep 17 00:00:00 2001 From: joshmccloskey Date: Tue, 5 Jan 2021 10:24:54 -0800 Subject: [PATCH 1/6] Added nullptr check to pullFaceSettingsLocked Fixes: 175852187 Test: It builds. Change-Id: Id1eb98f071a96178de42285bb21ef7009517f748 (cherry picked from commit dab89bdde3893e53eb9a7df7f54fff3b2f4c5029) --- .../com/android/server/stats/pull/StatsPullAtomService.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java index 486de39deeb8f..68ff0229d5c56 100644 --- a/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java +++ b/services/core/java/com/android/server/stats/pull/StatsPullAtomService.java @@ -3362,7 +3362,11 @@ public class StatsPullAtomService extends SystemService { int pullFaceSettingsLocked(int atomTag, List pulledData) { final long callingToken = Binder.clearCallingIdentity(); try { - List users = mContext.getSystemService(UserManager.class).getUsers(); + UserManager manager = mContext.getSystemService(UserManager.class); + if (manager == null) { + return StatsManager.PULL_SKIP; + } + List users = manager.getUsers(); int numUsers = users.size(); FaceManager faceManager = mContext.getSystemService(FaceManager.class); From bb2279de3ca08408433dc82496b60ecf4e2b9520 Mon Sep 17 00:00:00 2001 From: SongFerngWang Date: Wed, 5 May 2021 21:33:00 +0800 Subject: [PATCH 2/6] [security] SubscriptionGroup is exposed to unprivileged callers SubscriptionInfo.mGroupUUID is not cleared in conditionallyRemoveIdentifiers if the caller only has READ_PHONE_STATE (based on a check to checkReadPhoneState) and not READ_DEVICE_IDENTIFIERS. Bug: 181053462 Test: atest SubscriptionManagerTest Change-Id: Ic2b62523330dc6e2169ad851715c4ab3da3b29cf Merged-In: Ic2b62523330dc6e2169ad851715c4ab3da3b29cf (cherry picked from commit 219d284a68f56093aa9ca6610a4999b35c4cf5a9) --- telephony/java/android/telephony/SubscriptionInfo.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/telephony/java/android/telephony/SubscriptionInfo.java b/telephony/java/android/telephony/SubscriptionInfo.java index 0ee6568b64307..90d7a161767cd 100644 --- a/telephony/java/android/telephony/SubscriptionInfo.java +++ b/telephony/java/android/telephony/SubscriptionInfo.java @@ -566,6 +566,13 @@ public class SubscriptionInfo implements Parcelable { return mGroupUUID; } + /** + * @hide + */ + public void clearGroupUuid() { + this.mGroupUUID = null; + } + /** * @hide */ From fbe5177bd5d704dabf434458649fd93a07d8d654 Mon Sep 17 00:00:00 2001 From: Tej Singh Date: Wed, 19 May 2021 20:12:46 -0700 Subject: [PATCH 3/6] [RESTRICT AUTOMERGE] Fix OOB write in noteAtomLogged It's possible for bad atoms to have negative atom ids. This results in an OOB write when we note that the atom was logged. This adds a validation check on the logging. Also added safetynet logging for negative atoms Bug: 187957589 Test: POC in bug no longer led to the OOB write & crash Test: checked event log for safetynet logging Change-Id: I8a6b094c94309d7b02430fb860891ef814efb426 (cherry picked from commit cc0bba36c7c326e2fb75f1531547d2ed861d392c) --- cmds/statsd/src/guardrail/StatsdStats.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmds/statsd/src/guardrail/StatsdStats.cpp b/cmds/statsd/src/guardrail/StatsdStats.cpp index 6e89038f41520..14b967a11830d 100644 --- a/cmds/statsd/src/guardrail/StatsdStats.cpp +++ b/cmds/statsd/src/guardrail/StatsdStats.cpp @@ -459,9 +459,12 @@ void StatsdStats::notePullExceedMaxDelay(int pullAtomId) { void StatsdStats::noteAtomLogged(int atomId, int32_t timeSec) { lock_guard lock(mLock); - if (atomId <= kMaxPushedAtomId) { + if (atomId >= 0 && atomId <= kMaxPushedAtomId) { mPushedAtomStats[atomId]++; } else { + if (atomId < 0) { + android_errorWriteLog(0x534e4554, "187957589"); + } if (mNonPlatformPushedAtomStats.size() < kMaxNonPlatformPushedAtoms) { mNonPlatformPushedAtomStats[atomId]++; } From 7b82cbbe3411396b187b68548f2c325b42e964a6 Mon Sep 17 00:00:00 2001 From: Zim Date: Fri, 4 Dec 2020 11:20:02 +0000 Subject: [PATCH 4/6] Block SAF directory access to /sdcard/Android This works for target R+ apps, but need to come up with a better story for target Date: Tue, 15 Jun 2021 11:44:23 -0700 Subject: [PATCH 5/6] Validate the ServiceRecord state while handling misbehaving FGS Fix a race condition where the previous misbehaving FGS's notifcation is being posted, but that FGS's being stopped, and meanwhile a new FGS is coming up, the system would get confused and results in IllegalStateException. Bug: 182160371 Test: atest CtsAppTestCases:ServiceTest Change-Id: If18e1d7ba88aef693349b82dc6e70f7d98c68665 Merged-In: If18e1d7ba88aef693349b82dc6e70f7d98c68665 (cherry picked from commit ae5a219b955d63cbfcc15465d145a9303aafb807) --- .../java/com/android/server/am/ActiveServices.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 1062e141060f2..5abb87cedc717 100644 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -935,7 +935,18 @@ public final class ActiveServices { void killMisbehavingService(ServiceRecord r, int appUid, int appPid, String localPackageName) { synchronized (mAm) { - stopServiceLocked(r); + if (!r.destroying) { + // This service is still alive, stop it. + stopServiceLocked(r); + } else { + // Check if there is another instance of it being started in parallel, + // if so, stop that too to avoid spamming the system. + final ServiceMap smap = getServiceMapLocked(r.userId); + final ServiceRecord found = smap.mServicesByInstanceName.remove(r.instanceName); + if (found != null) { + stopServiceLocked(found); + } + } mAm.crashApplication(appUid, appPid, localPackageName, -1, "Bad notification for startForeground", true /*force*/); } From 2992a1dac025074726364d522cf9eac586f3d2c3 Mon Sep 17 00:00:00 2001 From: Paul Scovanner Date: Sat, 19 Jun 2021 04:56:21 +0000 Subject: [PATCH 6/6] Revert "Detects all activities for whether showing work challenge" This reverts commit 87fa64ebe46f1b3273e1e42c99ef2a09f19145a8. Reason for revert: b/191385314 Change-Id: I87ac32c9aa6a8d2a43533f7512d5b6ea85fdeb36 (cherry picked from commit bbdd0f043c714c62b45abd39d9078a049db70ef9) --- .../server/wm/RootWindowContainer.java | 40 +++++++++++++++---- .../server/wm/RootWindowContainerTests.java | 33 --------------- 2 files changed, 32 insertions(+), 41 deletions(-) diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index ddad1dbd9b3d5..eaf76938e2e83 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -3372,7 +3372,7 @@ class RootWindowContainer extends WindowContainer } /** - * Find all task stacks containing {@param userId} and intercept them with an activity + * Find all visible task stacks containing {@param userId} and intercept them with an activity * to block out the contents and possibly start a credential-confirming intent. * * @param userId user handle for the locked managed profile. @@ -3380,18 +3380,42 @@ class RootWindowContainer extends WindowContainer void lockAllProfileTasks(@UserIdInt int userId) { mService.deferWindowLayout(); try { - forAllLeafTasks(task -> { - if (task.getActivity(activity -> !activity.finishing && activity.mUserId == userId) - != null) { - mService.getTaskChangeNotificationController().notifyTaskProfileLocked( - task.mTaskId, userId); - } - }, true /* traverseTopToBottom */); + final PooledConsumer c = PooledLambda.obtainConsumer( + RootWindowContainer::taskTopActivityIsUser, this, PooledLambda.__(Task.class), + userId); + forAllLeafTasks(c, true /* traverseTopToBottom */); + c.recycle(); } finally { mService.continueWindowLayout(); } } + /** + * Detects whether we should show a lock screen in front of this task for a locked user. + *

+ * We'll do this if either of the following holds: + *

    + *
  • The top activity explicitly belongs to {@param userId}.
    • + *
  • The top activity returns a result to an activity belonging to {@param userId}.
    • + *
+ * + * @return {@code true} if the top activity looks like it belongs to {@param userId}. + */ + private void taskTopActivityIsUser(Task task, @UserIdInt int userId) { + // To handle the case that work app is in the task but just is not the top one. + final ActivityRecord activityRecord = task.getTopNonFinishingActivity(); + final ActivityRecord resultTo = (activityRecord != null ? activityRecord.resultTo : null); + + // Check the task for a top activity belonging to userId, or returning a + // result to an activity belonging to userId. Example case: a document + // picker for personal files, opened by a work app, should still get locked. + if ((activityRecord != null && activityRecord.mUserId == userId) + || (resultTo != null && resultTo.mUserId == userId)) { + mService.getTaskChangeNotificationController().notifyTaskProfileLocked( + task.mTaskId, userId); + } + } + void cancelInitializingActivities() { for (int displayNdx = getChildCount() - 1; displayNdx >= 0; --displayNdx) { final DisplayContent display = getChildAt(displayNdx); 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..35d1b17d5822e 100644 --- a/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java @@ -25,7 +25,6 @@ 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.spyOn; import static com.android.server.wm.ActivityStack.ActivityState.FINISHING; import static com.android.server.wm.ActivityStack.ActivityState.PAUSED; import static com.android.server.wm.ActivityStack.ActivityState.PAUSING; @@ -37,13 +36,10 @@ 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.eq; -import static org.mockito.Mockito.verify; import android.app.WindowConfiguration; import android.content.ComponentName; import android.content.pm.ActivityInfo; -import android.os.UserHandle; import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; @@ -173,34 +169,5 @@ public class RootWindowContainerTests extends WindowTestsBase { activity.setState(FINISHING, "test FINISHING"); assertThat(mWm.mRoot.allPausedActivitiesComplete()).isTrue(); } - - @Test - public void testLockAllProfileTasks() { - // Make an activity visible with the user id set to 0 - DisplayContent displayContent = mWm.mRoot.getDisplayContent(DEFAULT_DISPLAY); - TaskDisplayArea taskDisplayArea = displayContent.getTaskDisplayAreaAt(0); - final ActivityStack stack = createTaskStackOnDisplay(WINDOWING_MODE_FULLSCREEN, - ACTIVITY_TYPE_STANDARD, displayContent); - final ActivityRecord activity = new ActivityTestsBase.ActivityBuilder(stack.mAtmService) - .setStack(stack) - .setUid(0) - .setCreateTask(true) - .build(); - - // Create another activity on top and the user id is 1 - Task task = activity.getTask(); - final ActivityRecord topActivity = new ActivityTestsBase.ActivityBuilder(mWm.mAtmService) - .setStack(stack) - .setUid(UserHandle.PER_USER_RANGE + 1) - .setTask(task) - .build(); - - // Make sure the listeners will be notified for putting the task to locked state - TaskChangeNotificationController controller = - mWm.mAtmService.getTaskChangeNotificationController(); - spyOn(controller); - mWm.mRoot.lockAllProfileTasks(0); - verify(controller).notifyTaskProfileLocked(eq(task.mTaskId), eq(0)); - } }