summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRiddle Hsu <riddlehsu@google.com>2020-03-03 18:12:10 +0800
committerandroid-build-team Robot <android-build-team-robot@google.com>2020-03-19 04:52:02 +0000
commit5deb172bf4d9e2b80cda0b8dd5d2b0573e1c86e9 (patch)
tree20eac22c47f71643097c70f0ccee6cf8c0dba6b8
parent058cafe0c1f61e8edd25ba22e2c8e73a43c5d4ad (diff)
downloadbase-5deb172bf4d9e2b80cda0b8dd5d2b0573e1c86e9.tar.gz
RESTRICT AUTOMERGE Use consistent calling uid and package in navigateUpTo
Originally, if the caller of navigateUpTo is alive, even the calling uid is set to the caller who launched the existing destination activity, the uid from caller process has higher priority to replace the given calling uid. So this change doesn't modify the existing behavior if the caller process is valid. Besides, the case of delivering new intent uses the source record as calling identity too, so the case of starting new activity should be consistent. Also forbid attaching null application thread to avoid unexpected state in process record. Bug: 144285917 Test: atest ActivityStackTests#testNavigateUpTo Change-Id: I60732f430256d37cb926d08d093581f051c4afed (cherry picked from commit 8a2c62c2867d30dd2065096b63512d2861ebae51)
-rw-r--r--services/core/java/com/android/server/am/ActivityManagerService.java5
-rw-r--r--services/core/java/com/android/server/wm/ActivityStack.java15
-rw-r--r--services/core/java/com/android/server/wm/ActivityStarter.java5
-rw-r--r--services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java41
-rw-r--r--services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java1
5 files changed, 58 insertions, 9 deletions
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e8505fcb37a4..7fee0f016750 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -4759,7 +4759,7 @@ public class ActivityManagerService extends IActivityManager.Stub
}
@GuardedBy("this")
- private final boolean attachApplicationLocked(IApplicationThread thread,
+ private boolean attachApplicationLocked(@NonNull IApplicationThread thread,
int pid, int callingUid, long startSeq) {
// Find the application record that is being attached... either via
@@ -5173,6 +5173,9 @@ public class ActivityManagerService extends IActivityManager.Stub
@Override
public final void attachApplication(IApplicationThread thread, long startSeq) {
+ if (thread == null) {
+ throw new SecurityException("Invalid application interface");
+ }
synchronized (this) {
int callingPid = Binder.getCallingPid();
final int callingUid = Binder.getCallingUid();
diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java
index 56be335c2624..1a9b11cd62d5 100644
--- a/services/core/java/com/android/server/wm/ActivityStack.java
+++ b/services/core/java/com/android/server/wm/ActivityStack.java
@@ -4257,6 +4257,11 @@ class ActivityStack extends ConfigurationContainer {
final boolean navigateUpToLocked(ActivityRecord srec, Intent destIntent, int resultCode,
Intent resultData) {
+ if (!srec.attachedToProcess()) {
+ // Nothing to do if the caller is not attached, because this method should be called
+ // from an alive activity.
+ return false;
+ }
final TaskRecord task = srec.getTaskRecord();
final ArrayList<ActivityRecord> activities = task.mActivities;
final int start = activities.indexOf(srec);
@@ -4310,14 +4315,14 @@ class ActivityStack extends ConfigurationContainer {
}
if (parent != null && foundParentInTask) {
+ final int callingUid = srec.info.applicationInfo.uid;
final int parentLaunchMode = parent.info.launchMode;
final int destIntentFlags = destIntent.getFlags();
if (parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_INSTANCE ||
parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TASK ||
parentLaunchMode == ActivityInfo.LAUNCH_SINGLE_TOP ||
(destIntentFlags & Intent.FLAG_ACTIVITY_CLEAR_TOP) != 0) {
- parent.deliverNewIntentLocked(srec.info.applicationInfo.uid, destIntent,
- srec.packageName);
+ parent.deliverNewIntentLocked(callingUid, destIntent, srec.packageName);
} else {
try {
ActivityInfo aInfo = AppGlobals.getPackageManager().getActivityInfo(
@@ -4330,10 +4335,10 @@ class ActivityStack extends ConfigurationContainer {
.setActivityInfo(aInfo)
.setResultTo(parent.appToken)
.setCallingPid(-1)
- .setCallingUid(parent.launchedFromUid)
- .setCallingPackage(parent.launchedFromPackage)
+ .setCallingUid(callingUid)
+ .setCallingPackage(srec.packageName)
.setRealCallingPid(-1)
- .setRealCallingUid(parent.launchedFromUid)
+ .setRealCallingUid(callingUid)
.setComponentSpecified(true)
.execute();
foundParentInTask = res == ActivityManager.START_SUCCESS;
diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java
index 5b697ee89602..f37698de34d5 100644
--- a/services/core/java/com/android/server/wm/ActivityStarter.java
+++ b/services/core/java/com/android/server/wm/ActivityStarter.java
@@ -2763,6 +2763,11 @@ class ActivityStarter {
return mRequest.intent;
}
+ @VisibleForTesting
+ int getCallingUid() {
+ return mRequest.callingUid;
+ }
+
ActivityStarter setReason(String reason) {
mRequest.reason = reason;
return this;
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
index bde0ef6aa39e..ff27b9bb1c9e 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStackTests.java
@@ -28,7 +28,7 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.mock;
-import static com.android.dx.mockito.inline.extended.ExtendedMockito.spy;
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
import static com.android.server.wm.ActivityStack.ActivityState.DESTROYING;
import static com.android.server.wm.ActivityStack.ActivityState.FINISHING;
@@ -54,8 +54,11 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import android.app.ActivityManager;
+import android.app.IApplicationThread;
import android.content.ComponentName;
import android.content.pm.ActivityInfo;
import android.os.UserHandle;
@@ -82,8 +85,9 @@ public class ActivityStackTests extends ActivityTestsBase {
@Before
public void setUp() throws Exception {
mDefaultDisplay = mRootActivityContainer.getDefaultDisplay();
- mStack = spy(mDefaultDisplay.createStack(WINDOWING_MODE_UNDEFINED, ACTIVITY_TYPE_STANDARD,
- true /* onTop */));
+ mStack = mDefaultDisplay.createStack(WINDOWING_MODE_UNDEFINED, ACTIVITY_TYPE_STANDARD,
+ true /* onTop */);
+ spyOn(mStack);
mTask = new TaskBuilder(mSupervisor).setStack(mStack).build();
}
@@ -1078,6 +1082,37 @@ public class ActivityStackTests extends ActivityTestsBase {
assertTrue(listener.mChanged);
}
+ @Test
+ public void testNavigateUpTo() {
+ final ActivityStartController controller = mock(ActivityStartController.class);
+ final ActivityStarter starter = new ActivityStarter(controller,
+ mService, mService.mStackSupervisor, mock(ActivityStartInterceptor.class));
+ doReturn(controller).when(mService).getActivityStartController();
+ spyOn(starter);
+ doReturn(ActivityManager.START_SUCCESS).when(starter).execute();
+
+ final ActivityRecord firstActivity = new ActivityBuilder(mService).setTask(mTask).build();
+ final ActivityRecord secondActivity = new ActivityBuilder(mService).setTask(mTask)
+ .setUid(firstActivity.getUid() + 1).build();
+ doReturn(starter).when(controller).obtainStarter(eq(firstActivity.intent), anyString());
+
+ final IApplicationThread thread = secondActivity.app.getThread();
+ secondActivity.app.setThread(null);
+ // This should do nothing from a non-attached caller.
+ assertFalse(mStack.navigateUpToLocked(secondActivity /* source record */,
+ firstActivity.intent /* destIntent */, 0 /* resultCode */, null /* resultData */));
+
+ secondActivity.app.setThread(thread);
+ assertTrue(mStack.navigateUpToLocked(secondActivity /* source record */,
+ firstActivity.intent /* destIntent */, 0 /* resultCode */, null /* resultData */));
+ // The firstActivity uses default launch mode, so the activities between it and itself will
+ // be finished.
+ assertTrue(secondActivity.finishing);
+ assertTrue(firstActivity.finishing);
+ // The calling uid of the new activity should be the current real caller.
+ assertEquals(secondActivity.getUid(), starter.getCallingUid());
+ }
+
private void verifyShouldSleepActivities(boolean focusedStack,
boolean keyguardGoingAway, boolean displaySleeping, boolean expected) {
final ActivityDisplay display = mock(ActivityDisplay.class);
diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java b/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
index 84bdecb86826..f94f00203521 100644
--- a/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
+++ b/services/tests/wmtests/src/com/android/server/wm/ActivityTestsBase.java
@@ -290,6 +290,7 @@ class ActivityTestsBase {
aInfo.applicationInfo.packageName = mComponent.getPackageName();
aInfo.applicationInfo.uid = mUid;
aInfo.packageName = mComponent.getPackageName();
+ aInfo.name = mComponent.getClassName();
if (mTargetActivity != null) {
aInfo.targetActivity = mTargetActivity;
}