Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 46a37bec authored by Felipe Leme's avatar Felipe Leme
Browse files

Cache device owner user id on UserManagerService 

UMS needs to check if a user is the DO before removing it, but
calling DPMI at that point would break the LockGuard order.

Test: adb logcat LockGuard *:s # while removing a user
Test: atest UserManagerServiceMockedTest#testGetUserRemovabilityLocked_otherUsers OwnersTest
Test: adb shell dumpsys user | grep "Device owner"
Fixes: 440898996
Bug: 435271558
Flag: android.app.admin.flags.device_owner_for_all

Change-Id: I72e23911f62291a9a72f45f729ff1873493a9115
parent 880e5c0c

services/core/java/com/android/server/pm/UserManagerInternal.java

+8 −0
Original line number Diff line number Diff line
@@ -630,6 +630,14 @@ public abstract class UserManagerInternal { 
    /** Logs an activity launched in the headless system user */

    public abstract void logLaunchedHsuActivity(ComponentName activity);



    /**

     * Sets the id of the {@code DeviceOwner}, if any.

     *

     * <p>{@code DeviceOwner} is a {@code DPM} (Device Policy Management) concept and hence should

     * only be called by the {@code DPM} infra.

     */

    public abstract void setDeviceOwnerUserId(@CanBeNULL @UserIdInt int userId);



    /**

     * Checks whether to show a notification for sounds (e.g., alarms, timers, etc.) from background

     * users.

services/core/java/com/android/server/pm/UserManagerService.java

+33 −16
Original line number Diff line number Diff line
@@ -595,6 +595,17 @@ public class UserManagerService extends IUserManager.Stub { 
    @GuardedBy("mUsersLock")

    private int[] mUserIdsIncludingPreCreated;



    /**

     * Caches the id of the {@link android.app.admin.DevicePolicyManager#getDeviceOwner() device

     * owner} (or {@code UserHandle.USER_NULL} when there isn't one).

     *

     * <p>It must be cached because the device owner cannot be removed, and calling

     * {@link #mDevicePolicyManagerInternal} to retrieve it when removing a user would break the

     * lock guard order.

     */

    @GuardedBy("mUsersLock")

    private @CanBeNULL @UserIdInt int mDeviceOwnerUserId = UserHandle.USER_NULL;



    @GuardedBy("mPackagesLock")

    private int mNextSerialNumber;

    private int mUserVersion = 0;
@@ -1419,14 +1430,6 @@ public class UserManagerService extends IUserManager.Stub { 
        return UserHandle.USER_NULL;

    }



    private @UserIdInt int getDeviceOwnerUserId() {

        DevicePolicyManagerInternal dpmi = getDevicePolicyManagerInternal();

        if (dpmi == null) {

            return UserHandle.USER_NULL;

        }

        return dpmi.getDeviceOwnerUserId();

    }



    @Override

    public void setBootUser(@UserIdInt int userId) {

        checkCreateUsersPermission("Set boot user");
@@ -7225,7 +7228,7 @@ public class UserManagerService extends IUserManager.Stub { 
        if (mRemovingUserIds.get(userId)) {

            return UserManager.REMOVE_RESULT_ALREADY_BEING_REMOVED;

        }

        if (userId == getDeviceOwnerUserId()) {

        if (userId == mDeviceOwnerUserId) {

            return UserManager.REMOVE_RESULT_DEVICE_OWNER;

        }

        if (isNonRemovableLastAdminUserLU(userData.info)) {
@@ -8139,12 +8142,7 @@ public class UserManagerService extends IUserManager.Stub { 
        }



        final int currentUserId = getCurrentUserId();

        pw.print("Current user: ");

        if (currentUserId != UserHandle.USER_NULL) {

            pw.println(currentUserId);

        } else {

            pw.println("N/A");

        }

        printNullableUser(pw, "Current user", currentUserId);



        pw.println();

        synchronized (mPackagesLock) {
@@ -8251,7 +8249,8 @@ public class UserManagerService extends IUserManager.Stub { 
            pw.println("  System user allocations: " + mUser0Allocations.get());

        }

        synchronized (mUsersLock) {

            pw.println("  Boot user: " + mBootUser);

            printNullableUser(pw, "Boot user", mBootUser);

            printNullableUser(pw, "Device owner user", mDeviceOwnerUserId);

        }

        // TODO(b/413464199): This confusing line is, regrettably, currently required by Tradefed.

        pw.println("Can add private profile: "+ canAddPrivateProfile(currentUserId));
@@ -8293,6 +8292,16 @@ public class UserManagerService extends IUserManager.Stub { 


    }



    private static void printNullableUser(PrintWriter pw, String what,

            @CanBeNULL @UserIdInt int userId) {

        pw.print(what); pw.print(": ");

        if (userId == UserHandle.USER_NULL) {

            pw.println("N/A");

        } else {

            pw.println(userId);

        }

    }



    private void dumpUser(PrintWriter pw, @CanBeCURRENT @UserIdInt int userId, StringBuilder sb,

            long now, long nowRealtime) {

        if (userId == UserHandle.USER_CURRENT) {
@@ -8968,6 +8977,14 @@ public class UserManagerService extends IUserManager.Stub { 
            mDeprecationReporter.logLaunchedHsuActivity(activity);

        }



        @Override

        public void setDeviceOwnerUserId(int userId) {

            Slogf.i(LOG_TAG, "Setting device owner user as %d", userId);

            synchronized (mUsersLock) {

                mDeviceOwnerUserId = userId;

            }

        }



    } // class LocalService



    /**

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java

+5 −4
Original line number Diff line number Diff line
@@ -2313,7 +2313,9 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { 
                injector.getUserManager(),

                injector.getPackageManagerInternal(),

                injector.getActivityTaskManagerInternal(),

                injector.getActivityManagerInternal(), mStateCache, pathProvider);

                injector.getActivityManagerInternal(),

                injector.getUserManagerInternal(),

                mStateCache, pathProvider);

    }















    /**










@@ -10009,13 +10011,12 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {













                            UserHandle.of(u));















                }



























                // TODO Send to system too?















                sendOwnerChangedBroadcast(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED, userId);















            });















            mDeviceAdminServiceController.startServiceForAdmin(















                    admin.getPackageName(), userId, "set-device-owner");



























            Slogf.i(LOG_TAG, "Device owner set: " + admin + " on user " + userId);













            Slogf.i(LOG_TAG, "Device owner set: %s on user %d", admin, userId);















        }





























        if (setProfileOwnerOnCurrentUserIfNecessary










@@ -10405,7 +10406,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {













                sendOwnerChangedBroadcast(DevicePolicyManager.ACTION_DEVICE_OWNER_CHANGED,















                        deviceOwnerUserId);















            });













            Slogf.i(LOG_TAG, "Device owner removed: " + deviceOwnerComponent);













            Slogf.i(LOG_TAG, "Device owner removed: %s", deviceOwnerComponent);















        }















    }







































services/devicepolicy/java/com/android/server/devicepolicy/Owners.java



+13
−0




























Original line number
Diff line number
Diff line








@@ -45,6 +45,8 @@ import com.android.internal.annotations.GuardedBy;













import com.android.internal.annotations.VisibleForTesting;















import com.android.server.LocalServices;















import com.android.server.devicepolicy.OwnersData.OwnerInfo;













import com.android.server.pm.UserManagerInternal;













import com.android.server.utils.Slogf;















import com.android.server.wm.ActivityTaskManagerInternal;































import java.io.File;









@@ -66,6 +68,7 @@ class Owners {













    private final PackageManagerInternal mPackageManagerInternal;















    private final ActivityTaskManagerInternal mActivityTaskManagerInternal;















    private final ActivityManagerInternal mActivityManagerInternal;













    private final UserManagerInternal mUserManagerInternal;















    private final DeviceStateCacheImpl mDeviceStateCache;































    @GuardedBy("mData")









@@ -78,12 +81,14 @@ class Owners {













            PackageManagerInternal packageManagerInternal,















            ActivityTaskManagerInternal activityTaskManagerInternal,















            ActivityManagerInternal activityManagerInternal,













            UserManagerInternal userManagerInternal,















            DeviceStateCacheImpl deviceStateCache,















            PolicyPathProvider pathProvider) {















        mUserManager = userManager;















        mPackageManagerInternal = packageManagerInternal;















        mActivityTaskManagerInternal = activityTaskManagerInternal;















        mActivityManagerInternal = activityManagerInternal;













        mUserManagerInternal = userManagerInternal;















        mDeviceStateCache = deviceStateCache;















        mData = new OwnersData(pathProvider);















    }










@@ -119,9 +124,11 @@ class Owners {













    // ActivityTaskManager.















    @GuardedBy("mData")















    private void notifyChangeLocked() {













        Slogf.d(TAG, "notifyChangeLocked()");















        pushToDevicePolicyManager();















        pushToPackageManagerLocked();















        pushToActivityManagerLocked();













        pushToUserManagerLocked();















        pushToAppOpsLocked();















    }


























@@ -170,6 +177,11 @@ class Owners {













        mActivityManagerInternal.setProfileOwnerUid(profileOwners);















    }





























    @GuardedBy("mData")













    private void pushToUserManagerLocked() {













        mUserManagerInternal.setDeviceOwnerUserId(getDeviceOwnerUserId());













    }





























    @GuardedBy("mData")















    int getDeviceOwnerUidLocked() {















        if (mData.mDeviceOwner != null) {










@@ -719,6 +731,7 @@ class Owners {













        synchronized (mData) {















            mSystemReady = true;















            pushToActivityManagerLocked();













            pushToUserManagerLocked();















            pushToAppOpsLocked();















        }















    }

































services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceMockedTest.java



+1
−7




























Original line number
Diff line number
Diff line








@@ -2059,7 +2059,7 @@ public final class UserManagerServiceMockedTest {













        var dyingUser = addDyingUser(new UserInfo(USER_ID4, A_USER_HAS_NO_NAME, FLAG_FULL));















        var deviceOwnerUser = addUser(















                new UserInfo(USER_ID5, A_USER_HAS_NO_NAME, FLAG_FULL | FLAG_ADMIN));













        mockGetDeviceOwnerUserId(deviceOwnerUser.id);













        mUmi.setDeviceOwnerUserId(deviceOwnerUser.id);































        // Failure cases first















        expectGetUserRemovability("system user", USER_SYSTEM, REMOVE_RESULT_ERROR_SYSTEM_USER);










@@ -2186,12 +2186,6 @@ public final class UserManagerServiceMockedTest {













                .thenReturn(new Pair<>(currentUserId, targetUserId));















    }





























    private void mockGetDeviceOwnerUserId(@UserIdInt int deviceOwnerUserId) {













        Log.d(TAG, "mockGetDeviceOwnerUserId(): " + deviceOwnerUserId);













        mockGetLocalService(DevicePolicyManagerInternal.class, mDevicePolicyManagerInternal);













        when(mDevicePolicyManagerInternal.getDeviceOwnerUserId()).thenReturn(deviceOwnerUserId);













    }





























    private <T> void mockGetLocalService(Class<T> serviceClass, T service) {















        doReturn(service).when(() -> LocalServices.getService(serviceClass));















    }