Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Drive-by fix for wtfIfInLock + two violations

Browse Source 
wtfIfInLock() method was checking 'this' lock while the code is
actually using mLockDoNoUseDirectly returned by getLockObject(),
thus violation werent reported. This CL changes this and fixes
violations when calling hasIncompatibleAccountsOrNonAdbNoLock.
Also move it to other lock-related methods.

Bug: 162815601
Test: atest com.android.cts.devicepolicy.DeviceOwnerTest#testDeviceOwnerSetup
Test: atest com.android.cts.devicepolicy.ProfileOwnerTest#testManagement
Test: manual: add account, attempt setting a DO
Change-Id: Id64930552f5a9b25464ba942b0b1ddbe9378928e
This commit is contained in:
Pavel Grafov
2021-04-27 15:53:18 +01:00
parent 1b2c66567e
commit 4602e87f41
1 changed files with 26 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
View File

@@ -745,6 +745,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
Slogf.wtfStack(LOG_TAG, "Not holding DPMS lock.");
}
/**
* Calls wtfStack() if called with the DPMS lock held.
*/
private void wtfIfInLock() {
if (Thread.holdsLock(mLockDoNoUseDirectly)) {
Slogf.wtfStack(LOG_TAG, "Shouldn't be called with DPMS lock held");
}
}
@VisibleForTesting
final TransferOwnershipMetadataManager mTransferOwnershipMetadataManager;
@@ -8141,8 +8150,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
Preconditions.checkArgument(admin != null);
final CallerIdentity caller = getCallerIdentity();
// Cannot be called while holding the lock:
final boolean hasIncompatibleAccountsOrNonAdb =
hasIncompatibleAccountsOrNonAdbNoLock(caller, userId, admin);
synchronized (getLockObject()) {
enforceCanSetDeviceOwnerLocked(caller, admin, userId);
enforceCanSetDeviceOwnerLocked(caller, admin, userId, hasIncompatibleAccountsOrNonAdb);
Preconditions.checkArgument(isPackageInstalledForUser(admin.getPackageName(), userId),
"Invalid component " + admin + " for device owner");
final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId);
@@ -8537,8 +8549,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
Preconditions.checkArgument(who != null);
final CallerIdentity caller = getCallerIdentity();
// Cannot be called while holding the lock:
final boolean hasIncompatibleAccountsOrNonAdb =
hasIncompatibleAccountsOrNonAdbNoLock(caller, userHandle, who);
synchronized (getLockObject()) {
enforceCanSetProfileOwnerLocked(caller, who, userHandle);
enforceCanSetProfileOwnerLocked(
caller, who, userHandle, hasIncompatibleAccountsOrNonAdb);
Preconditions.checkArgument(isPackageInstalledForUser(who.getPackageName(), userHandle),
"Component " + who + " not installed for userId:" + userHandle);
final ActiveAdmin admin = getActiveAdminUncheckedLocked(who, userHandle);
@@ -9116,15 +9132,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
});
}
/**
* Calls wtfStack() if called with the DPMS lock held.
*/
private void wtfIfInLock() {
if (Thread.holdsLock(this)) {
Slogf.wtfStack(LOG_TAG, "Shouldn't be called with DPMS lock held");
}
}
/**
* The profile owner can only be set by adb or an app with the MANAGE_PROFILE_AND_DEVICE_OWNERS
* permission.
@@ -9133,8 +9140,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
* - SYSTEM_UID
* - adb unless hasIncompatibleAccountsOrNonAdb is true.
*/
private void enforceCanSetProfileOwnerLocked(CallerIdentity caller,
@Nullable ComponentName owner, int userHandle) {
private void enforceCanSetProfileOwnerLocked(
CallerIdentity caller, @Nullable ComponentName owner, int userHandle,
boolean hasIncompatibleAccountsOrNonAdb) {
UserInfo info = getUserInfo(userHandle);
if (info == null) {
// User doesn't exist.
@@ -9154,7 +9162,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
}
if (isAdb(caller)) {
if ((mIsWatch || hasUserSetupCompleted(userHandle))
&& hasIncompatibleAccountsOrNonAdbNoLock(caller, userHandle, owner)) {
&& hasIncompatibleAccountsOrNonAdb) {
throw new IllegalStateException("Not allowed to set the profile owner because "
+ "there are already some accounts on the profile");
}
@@ -9191,8 +9199,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
* The Device owner can only be set by adb or an app with the MANAGE_PROFILE_AND_DEVICE_OWNERS
* permission.
*/
private void enforceCanSetDeviceOwnerLocked(CallerIdentity caller,
@Nullable ComponentName owner, @UserIdInt int deviceOwnerUserId) {
private void enforceCanSetDeviceOwnerLocked(
CallerIdentity caller, @Nullable ComponentName owner, @UserIdInt int deviceOwnerUserId,
boolean hasIncompatibleAccountsOrNonAdb) {
if (!isAdb(caller)) {
Preconditions.checkCallAuthorization(
hasCallingOrSelfPermission(permission.MANAGE_PROFILE_AND_DEVICE_OWNERS));
@@ -9200,8 +9209,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
final int code = checkDeviceOwnerProvisioningPreConditionLocked(owner,
/* deviceOwnerUserId= */ deviceOwnerUserId, /* callingUserId*/ caller.getUserId(),
isAdb(caller),
hasIncompatibleAccountsOrNonAdbNoLock(caller, deviceOwnerUserId, owner));
isAdb(caller), hasIncompatibleAccountsOrNonAdb);
if (code != CODE_OK) {
throw new IllegalStateException(
computeProvisioningErrorString(code, deviceOwnerUserId));