Do not call into ActivityManager from DPMS within DPMS lock
This will allow AMS to call into DPMS within the AMS lock instead,
which will help I1537bd57b34696768ee81a979d53bb396efbc12a.
- AM.clearApplicationUserData() will not be allowed for any DA
apps.
Bug 25567963
Change-Id: I9f0d071c815a011be4f4c85c502c39d0fe0fe5e8
diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index 7c3fcc3..44cb590 100644
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -2607,14 +2607,6 @@
return true;
}
- case UPDATE_DEVICE_OWNER_TRANSACTION: {
- data.enforceInterface(IActivityManager.descriptor);
- String packageName = data.readString();
- updateDeviceOwner(packageName);
- reply.writeNoException();
- return true;
- }
-
case GET_PACKAGE_PROCESS_STATE_TRANSACTION: {
data.enforceInterface(IActivityManager.descriptor);
String pkg = data.readString();
@@ -6154,18 +6146,6 @@
}
@Override
- public void updateDeviceOwner(String packageName) throws RemoteException {
- Parcel data = Parcel.obtain();
- Parcel reply = Parcel.obtain();
- data.writeInterfaceToken(IActivityManager.descriptor);
- data.writeString(packageName);
- mRemote.transact(UPDATE_DEVICE_OWNER_TRANSACTION, data, reply, 0);
- reply.readException();
- data.recycle();
- reply.recycle();
- }
-
- @Override
public int getPackageProcessState(String packageName, String callingPackage)
throws RemoteException {
Parcel data = Parcel.obtain();
diff --git a/core/java/android/app/IActivityManager.java b/core/java/android/app/IActivityManager.java
index fcc040b..4fdcfaa 100644
--- a/core/java/android/app/IActivityManager.java
+++ b/core/java/android/app/IActivityManager.java
@@ -518,7 +518,6 @@
public void setVoiceKeepAwake(IVoiceInteractionSession session, boolean keepAwake)
throws RemoteException;
public void updateLockTaskPackages(int userId, String[] packages) throws RemoteException;
- public void updateDeviceOwner(String packageName) throws RemoteException;
public int getPackageProcessState(String packageName, String callingPackage)
throws RemoteException;
@@ -881,7 +880,6 @@
int NOTE_ALARM_FINISH_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+292;
int GET_PACKAGE_PROCESS_STATE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+293;
int SHOW_LOCK_TASK_ESCAPE_MESSAGE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+294;
- int UPDATE_DEVICE_OWNER_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+295;
int KEYGUARD_GOING_AWAY_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+296;
int REGISTER_UID_OBSERVER_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+297;
int UNREGISTER_UID_OBSERVER_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION+298;
diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java
index d7ffcc4..022b7c3 100644
--- a/core/java/android/app/admin/DevicePolicyManager.java
+++ b/core/java/android/app/admin/DevicePolicyManager.java
@@ -569,7 +569,7 @@
* extra field. This will invoke a UI to bring the user through adding the profile owner admin
* to remotely control restrictions on the user.
*
- * <p>The intent must be invoked via {@link Activity#startActivityForResult()} to receive the
+ * <p>The intent must be invoked via {@link Activity#startActivityForResult} to receive the
* result of whether or not the user approved the action. If approved, the result will
* be {@link Activity#RESULT_OK} and the component will be set as an active admin as well
* as a profile owner.
@@ -2970,7 +2970,7 @@
/**
* @hide
- * @param user The user for whom to fetch the profile owner name, if any.
+ * @param userId The user for whom to fetch the profile owner name, if any.
* @return the human readable name of the organisation associated with this profile owner or
* null if one is not set.
* @throws IllegalArgumentException if the userId is invalid.
diff --git a/core/java/android/app/admin/DevicePolicyManagerInternal.java b/core/java/android/app/admin/DevicePolicyManagerInternal.java
index 4270e16..5a46cd5 100644
--- a/core/java/android/app/admin/DevicePolicyManagerInternal.java
+++ b/core/java/android/app/admin/DevicePolicyManagerInternal.java
@@ -80,4 +80,10 @@
* This method always returns a new {@link Bundle}.
*/
public abstract Bundle getComposedUserRestrictions(int userId, Bundle inBundle);
+
+ /**
+ * @return true if a package is a device admin (possibly DO or PO) running on
+ * user {@code userId}.
+ */
+ public abstract boolean isDeviceAdminPackage(int userId, String packageName);
}
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e27dba6..b1b18d7 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -55,6 +55,8 @@
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;
@@ -500,11 +502,6 @@
*/
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 {
@@ -5135,8 +5132,12 @@
public boolean clearApplicationUserData(final String packageName,
final IPackageDataObserver observer, int userId) {
enforceNotIsolatedCaller("clearApplicationUserData");
- if (packageName != null && packageName.equals(mDeviceOwnerName)) {
- throw new SecurityException("Clearing DeviceOwner data is forbidden.");
+
+ final DevicePolicyManagerInternal dpmi =
+ LocalServices.getService(DevicePolicyManagerInternal.class);
+ if (dpmi != null && dpmi.isDeviceAdminPackage(userId, packageName)) {
+ throw new SecurityException(
+ "Clearing DeviceAdmin/DeviceOwner/ProfileOwner data is forbidden.");
}
int uid = Binder.getCallingUid();
int pid = Binder.getCallingPid();
@@ -9216,17 +9217,6 @@
}
@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 37e46fb..0625793 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -156,6 +156,11 @@
/**
* 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 {
@@ -1105,7 +1110,7 @@
.asInterface(ServiceManager.getService(Context.WINDOW_SERVICE));
}
- IActivityManager getIActivityManager() {
+ IActivityManager getIActivityManagerInner() {
return ActivityManagerNative.getDefault();
}
@@ -1232,6 +1237,16 @@
}
/**
+ * 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) {
@@ -1345,8 +1360,6 @@
// TODO PO may not have a class name either due to b/17652534. Address that too.
- updateDeviceOwnerLocked();
-
// TODO Notify UM to update restrictions (?)
}
}
@@ -2033,36 +2046,23 @@
validatePasswordOwnerLocked(policy);
updateMaximumTimeToLockLocked(policy);
- updateLockTaskPackagesLocked(policy.mLockTaskPackages, userHandle);
+ updateLockTaskPackages(policy.mLockTaskPackages, userHandle);
if (policy.mStatusBarDisabled) {
setStatusBarDisabledInternal(policy.mStatusBarDisabled, userHandle);
}
}
- 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());
+ 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) {
+ }
}
- } catch (RemoteException e) {
- // Not gonna happen.
- } finally {
- mInjector.binderRestoreCallingIdentity(ident);
- }
+ });
}
static void validateQualityConstant(int quality) {
@@ -2133,14 +2133,21 @@
}
private void ensureDeviceOwnerUserStarted() {
- 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) {
+ 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() {
try {
- mInjector.getIActivityManager().startUserInBackground(userId);
+ getIActivityManager().startUserInBackground(userId);
// STOPSHIP Prevent the DO user from being killed.
@@ -2148,7 +2155,7 @@
Slog.w(LOG_TAG, "Exception starting user", e);
}
}
- }
+ });
}
private void cleanUpOldUsers() {
@@ -2414,17 +2421,21 @@
}
enforceCrossUserPermission(userHandle);
synchronized (this) {
- 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 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;
+ }
+ }
+ return false;
+ }
+
@Override
public void removeActiveAdmin(ComponentName adminReceiver, int userHandle) {
if (!mHasFeature) {
@@ -3725,7 +3736,7 @@
@Override
public void run() {
try {
- IActivityManager am = mInjector.getIActivityManager();
+ IActivityManager am = getIActivityManager();
if (am.getCurrentUser().id == userHandle) {
am.switchUser(UserHandle.USER_SYSTEM);
}
@@ -4481,7 +4492,6 @@
mOwners.setDeviceOwner(admin, ownerName, userId);
mOwners.writeDeviceOwner();
- updateDeviceOwnerLocked();
Intent intent = new Intent(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED);
ident = mInjector.binderClearCallingIdentity();
@@ -4583,7 +4593,6 @@
mOwners.clearDeviceOwner();
mOwners.writeDeviceOwner();
- updateDeviceOwnerLocked();
// Reactivate backup service.
long ident = mInjector.binderClearCallingIdentity();
try {
@@ -5344,14 +5353,14 @@
}
}
- private boolean checkCallerIsCurrentUserOrProfile() {
+ private boolean checkCallerIsCurrentUserOrProfileAM() {
int callingUserId = UserHandle.getCallingUserId();
long token = mInjector.binderClearCallingIdentity();
try {
UserInfo currentUser;
UserInfo callingUser = mUserManager.getUserInfo(callingUserId);
try {
- currentUser = mInjector.getIActivityManager().getCurrentUser();
+ currentUser = getIActivityManager().getCurrentUser();
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Failed to talk to activity managed.", e);
return false;
@@ -5382,7 +5391,7 @@
// TODO When InputMethodManager supports per user calls remove
// this restriction.
- if (!checkCallerIsCurrentUserOrProfile()) {
+ if (!checkCallerIsCurrentUserOrProfileAM()) {
return false;
}
@@ -5434,7 +5443,7 @@
public List getPermittedInputMethodsForCurrentUser() {
UserInfo currentUser;
try {
- currentUser = mInjector.getIActivityManager().getCurrentUser();
+ currentUser = 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
@@ -5529,7 +5538,7 @@
}
// Start user in background.
- mInjector.getIActivityManager().startUserInBackground(userHandle);
+ getIActivityManager().startUserInBackground(userHandle);
} catch (RemoteException e) {
Slog.e(LOG_TAG, "Failed to make remote calls for configureUser", e);
}
@@ -5562,20 +5571,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();
- }
- return mInjector.getIActivityManager().switchUser(userId);
- } catch (RemoteException e) {
- Log.e(LOG_TAG, "Couldn't switch user", e);
- return false;
- } finally {
- mInjector.binderRestoreCallingIdentity(id);
+ long id = mInjector.binderClearCallingIdentity();
+ try {
+ int userId = UserHandle.USER_SYSTEM;
+ if (userHandle != null) {
+ userId = userHandle.getIdentifier();
}
+ return getIActivityManager().switchUser(userId);
+ } catch (RemoteException e) {
+ Log.e(LOG_TAG, "Couldn't switch user", e);
+ return false;
+ } finally {
+ mInjector.binderRestoreCallingIdentity(id);
}
}
@@ -6047,7 +6056,7 @@
// Store the settings persistently.
saveSettingsLocked(userHandle);
- updateLockTaskPackagesLocked(packages, userHandle);
+ updateLockTaskPackages(packages, userHandle);
}
/**
@@ -6415,6 +6424,16 @@
}
}
+ @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 2c01b8a..6419338 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 getIActivityManager() {
+ IActivityManager getIActivityManagerInner() {
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 36980e3..ca3d950 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -476,10 +476,6 @@
// 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));
@@ -546,10 +542,6 @@
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.
@@ -584,10 +576,6 @@
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.