From 4f3d52cfd43955844b3a3086bdd96c79f32be59d Mon Sep 17 00:00:00 2001 From: Kholoud Mohamed Date: Fri, 29 May 2020 11:57:35 +0100 Subject: [PATCH] Use killUid instead of killApplication to kill app When the cross profile app op gets revoked, we need to kill the app then send an app op changed broadcast to the app. The problem with using killApplication is that it is async and does not guarantee that the app is killed before the broadcast is sent. It also fixes an unrelated issue when CrossProfileApps#clearInteractAcrossProfilesAppOps is called from managed provisioning which is non system process, this causes a security exception if killApplication is used. Fixes: 156995567 Fixes: 157318765 Test: atest ManagedProfileCrossProfileTest Test: atest ManagedProfileProvisioningTest Change-Id: Iceedd57baeb64daccef072bc787b1e1cb1bfe814 --- .../pm/CrossProfileAppsServiceImpl.java | 23 +++++++++---------- .../CrossProfileAppsServiceImplRoboTest.java | 2 +- .../pm/CrossProfileAppsServiceImplTest.java | 16 +++++-------- 3 files changed, 18 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java index e82ee22c8064d..a6e2232b2ca5d 100644 --- a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java +++ b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java @@ -60,6 +60,7 @@ import com.android.internal.util.ArrayUtils; import com.android.internal.util.FunctionalUtils.ThrowingRunnable; import com.android.internal.util.FunctionalUtils.ThrowingSupplier; import com.android.server.LocalServices; +import com.android.server.pm.permission.PermissionManagerService; import com.android.server.wm.ActivityTaskManagerInternal; import java.util.ArrayList; @@ -475,9 +476,11 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { mInjector.getAppOpsManager() .setMode(OP_INTERACT_ACROSS_PROFILES, uid, packageName, newMode); } + // Kill the UID before sending the broadcast to ensure that apps can be informed when + // their app-op has been revoked. + maybeKillUid(packageName, uid, hadPermission); sendCanInteractAcrossProfilesChangedBroadcast(packageName, uid, UserHandle.of(userId)); maybeLogSetInteractAcrossProfilesAppOp(packageName, newMode, userId, logMetrics, uid); - maybeKillUid(packageName, uid, hadPermission); } /** @@ -492,7 +495,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { if (hasInteractAcrossProfilesPermission(packageName, uid, PermissionChecker.PID_UNKNOWN)) { return; } - mInjector.killUid(packageName, uid); + mInjector.killUid(uid); } private void maybeLogSetInteractAcrossProfilesAppOp( @@ -797,15 +800,11 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } @Override - public void killUid(String packageName, int uid) { - try { - ActivityManager.getService().killApplication( - packageName, - UserHandle.getAppId(uid), - UserHandle.getUserId(uid), - PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED); - } catch (RemoteException ignored) { - } + public void killUid(int uid) { + PermissionManagerService.killUid( + UserHandle.getAppId(uid), + UserHandle.getUserId(uid), + PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED); } } @@ -847,7 +846,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { int checkComponentPermission(String permission, int uid, int owningUid, boolean exported); - void killUid(String packageName, int uid); + void killUid(int uid); } class LocalService extends CrossProfileAppsInternal { diff --git a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java index d78dad55e1814..0aa44d0d8707c 100644 --- a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java +++ b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java @@ -709,7 +709,7 @@ public class CrossProfileAppsServiceImplRoboTest { } @Override - public void killUid(String packageName, int uid) { + public void killUid(int uid) { mKilledUids.add(uid); } } diff --git a/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java b/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java index a2393a80d11e4..2162c0b20eacc 100644 --- a/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java @@ -32,7 +32,6 @@ import android.content.pm.PermissionInfo; import android.content.pm.ResolveInfo; import android.os.Bundle; import android.os.IBinder; -import android.os.RemoteException; import android.os.UserHandle; import android.os.UserManager; import android.permission.PermissionManager; @@ -42,6 +41,7 @@ import android.util.SparseArray; import com.android.internal.util.FunctionalUtils.ThrowingRunnable; import com.android.internal.util.FunctionalUtils.ThrowingSupplier; import com.android.server.LocalServices; +import com.android.server.pm.permission.PermissionManagerService; import com.android.server.wm.ActivityTaskManagerInternal; import org.junit.Before; @@ -696,15 +696,11 @@ public class CrossProfileAppsServiceImplTest { } @Override - public void killUid(String packageName, int uid) { - try { - ActivityManager.getService().killApplication( - packageName, - UserHandle.getAppId(uid), - UserHandle.getUserId(uid), - PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED); - } catch (RemoteException ignored) { - } + public void killUid(int uid) { + PermissionManagerService.killUid( + UserHandle.getAppId(uid), + UserHandle.getUserId(uid), + PermissionManager.KILL_APP_REASON_PERMISSIONS_REVOKED); } } }