Revert "Do not call into ActivityManager from DPMS within DPMS lock"
Bug 25567963
This reverts commit 53de36f9c40c9a4ac1eb9cca8f458aa6c998c1fd.
Change-Id: I4faaa0b4c50d75e208f37b99bc1d6e2f0fff8127
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index b1b18d7..e27dba6 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -55,8 +55,6 @@
import android.app.IAppTask;
import android.app.ITaskStackListener;
import android.app.ProfilerInfo;
-import android.app.admin.DevicePolicyManagerInternal;
-import android.app.admin.IDevicePolicyManager;
import android.app.assist.AssistContent;
import android.app.assist.AssistStructure;
import android.app.usage.UsageEvents;
@@ -502,6 +500,11 @@
*/
SparseArray<String[]> mLockTaskPackages = new SparseArray<>();
+ /**
+ * The package name of the DeviceOwner. This package is not permitted to have its data cleared.
+ */
+ String mDeviceOwnerName;
+
final UserController mUserController;
public class PendingAssistExtras extends Binder implements Runnable {
@@ -5132,12 +5135,8 @@
public boolean clearApplicationUserData(final String packageName,
final IPackageDataObserver observer, int userId) {
enforceNotIsolatedCaller("clearApplicationUserData");
-
- final DevicePolicyManagerInternal dpmi =
- LocalServices.getService(DevicePolicyManagerInternal.class);
- if (dpmi != null && dpmi.isDeviceAdminPackage(userId, packageName)) {
- throw new SecurityException(
- "Clearing DeviceAdmin/DeviceOwner/ProfileOwner data is forbidden.");
+ if (packageName != null && packageName.equals(mDeviceOwnerName)) {
+ throw new SecurityException("Clearing DeviceOwner data is forbidden.");
}
int uid = Binder.getCallingUid();
int pid = Binder.getCallingPid();
@@ -9217,6 +9216,17 @@
}
@Override
+ public void updateDeviceOwner(String packageName) {
+ final int callingUid = Binder.getCallingUid();
+ if (callingUid != 0 && callingUid != Process.SYSTEM_UID) {
+ throw new SecurityException("updateDeviceOwner called from non-system process");
+ }
+ synchronized (this) {
+ mDeviceOwnerName = packageName;
+ }
+ }
+
+ @Override
public void updateLockTaskPackages(int userId, String[] packages) {
final int callingUid = Binder.getCallingUid();
if (callingUid != 0 && callingUid != Process.SYSTEM_UID) {
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index 0625793..37e46fb 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -156,11 +156,6 @@
/**
* Implementation of the device policy APIs.
- *
- * Locking policies:
- * - {@link DevicePolicyManagerService} must not call into {@link IActivityManager} within {@code
- * this} lock to avoid lock inversion.
- * - Methods that call into {@link IActivityManager} must have the "AM" suffix.
*/
public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
@@ -1110,7 +1105,7 @@
.asInterface(ServiceManager.getService(Context.WINDOW_SERVICE));
}
- IActivityManager getIActivityManagerInner() {
+ IActivityManager getIActivityManager() {
return ActivityManagerNative.getDefault();
}
@@ -1237,16 +1232,6 @@
}
/**
- * Caller must not hold {@code this} lock. See also the class javadoc.
- */
- final IActivityManager getIActivityManager() {
- if (Thread.holdsLock(this)) {
- Slog.wtfStack(LOG_TAG, "Call to ActivityManager detected within DPMS lock");
- }
- return mInjector.getIActivityManagerInner();
- }
-
- /**
* Instantiates the service.
*/
public DevicePolicyManagerService(Context context) {
@@ -1360,6 +1345,8 @@
// TODO PO may not have a class name either due to b/17652534. Address that too.
+ updateDeviceOwnerLocked();
+
// TODO Notify UM to update restrictions (?)
}
}
@@ -2046,23 +2033,36 @@
validatePasswordOwnerLocked(policy);
updateMaximumTimeToLockLocked(policy);
- updateLockTaskPackages(policy.mLockTaskPackages, userHandle);
+ updateLockTaskPackagesLocked(policy.mLockTaskPackages, userHandle);
if (policy.mStatusBarDisabled) {
setStatusBarDisabledInternal(policy.mStatusBarDisabled, userHandle);
}
}
- private void updateLockTaskPackages(List<String> packages, final int userId) {
- final String[] copy = packages.toArray(new String[packages.size()]);
- mHandler.post(new Runnable() {
- @Override
- public void run() {
- try {
- getIActivityManager().updateLockTaskPackages(userId, copy);
- } catch (RemoteException willNotHappen) {
- }
+ private void updateLockTaskPackagesLocked(List<String> packages, int userId) {
+ long ident = mInjector.binderClearCallingIdentity();
+ try {
+ mInjector.getIActivityManager()
+ .updateLockTaskPackages(userId, packages.toArray(new String[packages.size()]));
+ } catch (RemoteException e) {
+ // Not gonna happen.
+ } finally {
+ mInjector.binderRestoreCallingIdentity(ident);
+ }
+ }
+
+ private void updateDeviceOwnerLocked() {
+ long ident = mInjector.binderClearCallingIdentity();
+ try {
+ if (getDeviceOwner() != null) {
+ mInjector.getIActivityManager()
+ .updateDeviceOwner(getDeviceOwner().getPackageName());
}
- });
+ } catch (RemoteException e) {
+ // Not gonna happen.
+ } finally {
+ mInjector.binderRestoreCallingIdentity(ident);
+ }
}
static void validateQualityConstant(int quality) {
@@ -2133,21 +2133,14 @@
}
private void ensureDeviceOwnerUserStarted() {
- if (!mOwners.hasDeviceOwner()) {
- return;
- }
- final int userId = mOwners.getDeviceOwnerUserId();
- if (userId == UserHandle.USER_SYSTEM) {
- return;
- }
- if (VERBOSE_LOG) {
- Log.v(LOG_TAG, "Starting non-system DO user: " + userId);
- }
- mHandler.post(new Runnable() {
- @Override
- public void run() {
+ if (mOwners.hasDeviceOwner()) {
+ final int userId = mOwners.getDeviceOwnerUserId();
+ if (VERBOSE_LOG) {
+ Log.v(LOG_TAG, "Starting non-system DO user: " + userId);
+ }
+ if (userId != UserHandle.USER_SYSTEM) {
try {
- getIActivityManager().startUserInBackground(userId);
+ mInjector.getIActivityManager().startUserInBackground(userId);
// STOPSHIP Prevent the DO user from being killed.
@@ -2155,7 +2148,7 @@
Slog.w(LOG_TAG, "Exception starting user", e);
}
}
- });
+ }
}
private void cleanUpOldUsers() {
@@ -2421,19 +2414,15 @@
}
enforceCrossUserPermission(userHandle);
synchronized (this) {
- return packageHasActiveAdminsLocked(packageName, userHandle);
- }
- }
-
- boolean packageHasActiveAdminsLocked(String packageName, int userHandle) {
- DevicePolicyData policy = getUserData(userHandle);
- final int N = policy.mAdminList.size();
- for (int i = 0; i < N; i++) {
- if (policy.mAdminList.get(i).info.getPackageName().equals(packageName)) {
- return true;
+ DevicePolicyData policy = getUserData(userHandle);
+ final int N = policy.mAdminList.size();
+ for (int i=0; i<N; i++) {
+ if (policy.mAdminList.get(i).info.getPackageName().equals(packageName)) {
+ return true;
+ }
}
+ return false;
}
- return false;
}
@Override
@@ -3736,7 +3725,7 @@
@Override
public void run() {
try {
- IActivityManager am = getIActivityManager();
+ IActivityManager am = mInjector.getIActivityManager();
if (am.getCurrentUser().id == userHandle) {
am.switchUser(UserHandle.USER_SYSTEM);
}
@@ -4492,6 +4481,7 @@
mOwners.setDeviceOwner(admin, ownerName, userId);
mOwners.writeDeviceOwner();
+ updateDeviceOwnerLocked();
Intent intent = new Intent(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED);
ident = mInjector.binderClearCallingIdentity();
@@ -4593,6 +4583,7 @@
mOwners.clearDeviceOwner();
mOwners.writeDeviceOwner();
+ updateDeviceOwnerLocked();
// Reactivate backup service.
long ident = mInjector.binderClearCallingIdentity();
try {
@@ -5353,14 +5344,14 @@
}
}
- private boolean checkCallerIsCurrentUserOrProfileAM() {
+ private boolean checkCallerIsCurrentUserOrProfile() {
int callingUserId = UserHandle.getCallingUserId();
long token = mInjector.binderClearCallingIdentity();
try {
UserInfo currentUser;
UserInfo callingUser = mUserManager.getUserInfo(callingUserId);
try {
- currentUser = getIActivityManager().getCurrentUser();
+ currentUser = mInjector.getIActivityManager().getCurrentUser();
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Failed to talk to activity managed.", e);
return false;
@@ -5391,7 +5382,7 @@
// TODO When InputMethodManager supports per user calls remove
// this restriction.
- if (!checkCallerIsCurrentUserOrProfileAM()) {
+ if (!checkCallerIsCurrentUserOrProfile()) {
return false;
}
@@ -5443,7 +5434,7 @@
public List getPermittedInputMethodsForCurrentUser() {
UserInfo currentUser;
try {
- currentUser = getIActivityManager().getCurrentUser();
+ currentUser = mInjector.getIActivityManager().getCurrentUser();
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Failed to make remote calls to get current user", e);
// Activity managed is dead, just allow all IMEs
@@ -5538,7 +5529,7 @@
}
// Start user in background.
- getIActivityManager().startUserInBackground(userHandle);
+ mInjector.getIActivityManager().startUserInBackground(userHandle);
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Failed to make remote calls for configureUser", e);
}
@@ -5571,20 +5562,20 @@
Preconditions.checkNotNull(who, "ComponentName is null");
synchronized (this) {
getActiveAdminForCallerLocked(who, DeviceAdminInfo.USES_POLICY_DEVICE_OWNER);
- }
- long id = mInjector.binderClearCallingIdentity();
- try {
- int userId = UserHandle.USER_SYSTEM;
- if (userHandle != null) {
- userId = userHandle.getIdentifier();
+ long id = mInjector.binderClearCallingIdentity();
+ try {
+ int userId = UserHandle.USER_SYSTEM;
+ if (userHandle != null) {
+ userId = userHandle.getIdentifier();
+ }
+ return mInjector.getIActivityManager().switchUser(userId);
+ } catch (RemoteException e) {
+ Log.e(LOG_TAG, "Couldn't switch user", e);
+ return false;
+ } finally {
+ mInjector.binderRestoreCallingIdentity(id);
}
- return getIActivityManager().switchUser(userId);
- } catch (RemoteException e) {
- Log.e(LOG_TAG, "Couldn't switch user", e);
- return false;
- } finally {
- mInjector.binderRestoreCallingIdentity(id);
}
}
@@ -6056,7 +6047,7 @@
// Store the settings persistently.
saveSettingsLocked(userHandle);
- updateLockTaskPackages(packages, userHandle);
+ updateLockTaskPackagesLocked(packages, userHandle);
}
/**
@@ -6424,16 +6415,6 @@
}
}
- @Override
- public boolean isDeviceAdminPackage(int userId, String packageName) {
- if (packageName == null) {
- return false;
- }
- synchronized (DevicePolicyManagerService.this) {
- return packageHasActiveAdminsLocked(packageName, userId);
- }
- }
-
private void notifyCrossProfileProvidersChanged(int userId, List<String> packages) {
final List<OnCrossProfileWidgetProvidersChangeListener> listeners;
synchronized (DevicePolicyManagerService.this) {
diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
index 6419338..2c01b8a 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java
@@ -128,7 +128,7 @@
}
@Override
- IActivityManager getIActivityManagerInner() {
+ IActivityManager getIActivityManager() {
return context.iactivityManager;
}
diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
index ca3d950..36980e3 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -476,6 +476,10 @@
// Fire!
assertTrue(dpm.setDeviceOwner(admin1, "owner-name"));
+ // Verify internal calls.
+ verify(mContext.iactivityManager, times(1)).updateDeviceOwner(
+ eq(admin1.getPackageName()));
+
// TODO We should check if the caller has called clearCallerIdentity().
verify(mContext.ibackupManager, times(1)).setBackupServiceActive(
eq(UserHandle.USER_SYSTEM), eq(false));
@@ -542,6 +546,10 @@
dpm.setActiveAdmin(admin1, /* replace =*/ false);
assertTrue(dpm.setDeviceOwner(admin1, "owner-name"));
+ // Verify internal calls.
+ verify(mContext.iactivityManager, times(1)).updateDeviceOwner(
+ eq(admin1.getPackageName()));
+
assertEquals(admin1.getPackageName(), dpm.getDeviceOwner());
// Set up other mocks.
@@ -576,6 +584,10 @@
dpm.setActiveAdmin(admin1, /* replace =*/ false);
assertTrue(dpm.setDeviceOwner(admin1, "owner-name"));
+ // Verify internal calls.
+ verify(mContext.iactivityManager, times(1)).updateDeviceOwner(
+ eq(admin1.getPackageName()));
+
assertEquals(admin1.getPackageName(), dpm.getDeviceOwner());
// Now call clear from the secondary user, which should throw.