Merge "Do not call into ActivityManager from DPMS within DPMS lock"
diff --git a/core/java/android/app/ActivityManagerNative.java b/core/java/android/app/ActivityManagerNative.java
index 4449e4f..65c63f6 100644
--- a/core/java/android/app/ActivityManagerNative.java
+++ b/core/java/android/app/ActivityManagerNative.java
@@ -2611,14 +2611,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();
@@ -6163,18 +6155,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 b69a480..cf2452b 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 ba84642..66defa6 100644
--- a/core/java/android/app/admin/DevicePolicyManager.java
+++ b/core/java/android/app/admin/DevicePolicyManager.java
@@ -577,7 +577,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.
@@ -2978,7 +2978,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 0cf58b1..41d4271 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;
@@ -502,11 +504,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 {
@@ -5139,8 +5136,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();
@@ -9241,17 +9242,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 1e32f60..eee7d92 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();
}
@@ -1236,6 +1241,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) {
@@ -1349,8 +1364,6 @@
// TODO PO may not have a class name either due to b/17652534. Address that too.
- updateDeviceOwnerLocked();
-
// TODO Notify UM to update restrictions (?)
}
}
@@ -2037,36 +2050,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) {
@@ -2137,14 +2137,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.
@@ -2152,7 +2159,7 @@
Slog.w(LOG_TAG, "Exception starting user", e);
}
}
- }
+ });
}
private void cleanUpOldUsers() {
@@ -2418,17 +2425,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) {
@@ -3729,7 +3740,7 @@
@Override
public void run() {
try {
- IActivityManager am = mInjector.getIActivityManager();
+ IActivityManager am = getIActivityManager();
if (am.getCurrentUser().id == userHandle) {
am.switchUser(UserHandle.USER_SYSTEM);
}
@@ -4485,7 +4496,6 @@
mOwners.setDeviceOwner(admin, ownerName, userId);
mOwners.writeDeviceOwner();
- updateDeviceOwnerLocked();
Intent intent = new Intent(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED);
ident = mInjector.binderClearCallingIdentity();
@@ -4587,7 +4597,6 @@
mOwners.clearDeviceOwner();
mOwners.writeDeviceOwner();
- updateDeviceOwnerLocked();
// Reactivate backup service.
long ident = mInjector.binderClearCallingIdentity();
try {
@@ -5348,14 +5357,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;
@@ -5386,7 +5395,7 @@
// TODO When InputMethodManager supports per user calls remove
// this restriction.
- if (!checkCallerIsCurrentUserOrProfile()) {
+ if (!checkCallerIsCurrentUserOrProfileAM()) {
return false;
}
@@ -5438,7 +5447,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
@@ -5533,7 +5542,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);
}
@@ -5566,20 +5575,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);
}
}
@@ -6051,7 +6060,7 @@
// Store the settings persistently.
saveSettingsLocked(userHandle);
- updateLockTaskPackagesLocked(packages, userHandle);
+ updateLockTaskPackages(packages, userHandle);
}
/**
@@ -6419,6 +6428,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.