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

Commit bf5ccd26 authored by Adam Bookatz's avatar Adam Bookatz
Browse files

Various createUsers don't sometimes return null 

Previously, UserManagerService.createUserInternalUncheckedNoTracing
would usually throw on failure, but could instead return null in certain
circumstances (namely, bad input).

This lead to ambiguity over whether the various createUser methods were
able to return null.

Here, we throw instead in these previously-null situations. This way,
all of the createUser methods are @NonNull, unless they specifically
catch errors and return null instead. The methods are now all
self-consistent.

In particular, this fixes the previously not-always-correct annotation
of @NonNull that is on UserManager.preCreateUser.

Bug: 219942927
Test: atest com.android.server.pm.UserManagerTest
Test: atest com.android.server.pm.UserManagerServiceTest
Test: atest android.multiuser.cts.UserManagerTest
Change-Id: I7a63b4e684d03b4d2ef76046d0cf277ba2d5744d
parent 51a18cab

core/java/android/os/UserManager.java

+22 −26
Original line number Original line Diff line number Diff line
@@ -3612,7 +3612,7 @@ public class UserManager { 
     *

     *

     * <p>Requires {@link android.Manifest.permission#MANAGE_USERS}.

     * <p>Requires {@link android.Manifest.permission#MANAGE_USERS}.

     * {@link android.Manifest.permission#CREATE_USERS} suffices if flags are in

     * {@link android.Manifest.permission#CREATE_USERS} suffices if flags are in

     * com.android.server.pm.UserManagerService#ALLOWED_FLAGS_FOR_CREATE_USERS_PERMISSION}.

     * com.android.server.pm.UserManagerService#ALLOWED_FLAGS_FOR_CREATE_USERS_PERMISSION.

     *

     *

     * @param name     the user's name

     * @param name     the user's name

     * @param userType the type of user, such as {@link UserManager#USER_TYPE_FULL_GUEST}.

     * @param userType the type of user, such as {@link UserManager#USER_TYPE_FULL_GUEST}.
@@ -3686,7 +3686,7 @@ public class UserManager { 
     *

     *

     * <p>Requires {@link android.Manifest.permission#MANAGE_USERS}.

     * <p>Requires {@link android.Manifest.permission#MANAGE_USERS}.

     * {@link android.Manifest.permission#CREATE_USERS} suffices if flags are in

     * {@link android.Manifest.permission#CREATE_USERS} suffices if flags are in

     * com.android.server.pm.UserManagerService#ALLOWED_FLAGS_FOR_CREATE_USERS_PERMISSION}.

     * com.android.server.pm.UserManagerService#ALLOWED_FLAGS_FOR_CREATE_USERS_PERMISSION.

     *

     *

     * @param userType the type of user, such as {@link UserManager#USER_TYPE_FULL_GUEST}.

     * @param userType the type of user, such as {@link UserManager#USER_TYPE_FULL_GUEST}.

     * @return the {@link UserInfo} object for the created user.

     * @return the {@link UserInfo} object for the created user.
@@ -3718,10 +3718,9 @@ public class UserManager { 
     */

     */

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

            Manifest.permission.CREATE_USERS})

            Manifest.permission.CREATE_USERS})

    public UserInfo createGuest(Context context) {

    public @Nullable UserInfo createGuest(Context context) {

        try {

        try {

            final UserInfo guest = mService.createUserWithThrow(null, USER_TYPE_FULL_GUEST, 0);

            final UserInfo guest = mService.createUserWithThrow(null, USER_TYPE_FULL_GUEST, 0);

            if (guest != null) {

            Settings.Secure.putStringForUser(context.getContentResolver(),

            Settings.Secure.putStringForUser(context.getContentResolver(),

                    Settings.Secure.SKIP_FIRST_USE_HINTS, "1", guest.id);

                    Settings.Secure.SKIP_FIRST_USE_HINTS, "1", guest.id);
@@ -3738,7 +3737,6 @@ public class UserManager { 
                    setUserEphemeral(guest.id, true);

                    setUserEphemeral(guest.id, true);

                }

                }

            }

            }

            }

            return guest;

            return guest;

        } catch (ServiceSpecificException e) {

        } catch (ServiceSpecificException e) {

            return null;

            return null;
@@ -3855,7 +3853,7 @@ public class UserManager { 
     */

     */

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

            Manifest.permission.CREATE_USERS})

            Manifest.permission.CREATE_USERS})

    public UserInfo createProfileForUser(String name, @NonNull String userType,

    public @Nullable UserInfo createProfileForUser(String name, @NonNull String userType,

            @UserInfoFlag int flags, @UserIdInt int userId) {

            @UserInfoFlag int flags, @UserIdInt int userId) {

        return createProfileForUser(name, userType, flags, userId, null);

        return createProfileForUser(name, userType, flags, userId, null);

    }

    }
@@ -3900,7 +3898,7 @@ public class UserManager { 
     */

     */

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

    @RequiresPermission(anyOf = {Manifest.permission.MANAGE_USERS,

            Manifest.permission.CREATE_USERS})

            Manifest.permission.CREATE_USERS})

    public UserInfo createProfileForUserEvenWhenDisallowed(String name,

    public @Nullable UserInfo createProfileForUserEvenWhenDisallowed(String name,

            @NonNull String userType, @UserInfoFlag int flags, @UserIdInt int userId,

            @NonNull String userType, @UserInfoFlag int flags, @UserIdInt int userId,

            String[] disallowedPackages) {

            String[] disallowedPackages) {

        try {

        try {
@@ -3931,11 +3929,9 @@ public class UserManager { 
        try {

        try {

            final int parentUserId = mUserId;

            final int parentUserId = mUserId;

            final UserInfo profile = mService.createRestrictedProfileWithThrow(name, parentUserId);

            final UserInfo profile = mService.createRestrictedProfileWithThrow(name, parentUserId);

            if (profile != null) {

            final UserHandle parentUserHandle = UserHandle.of(parentUserId);

            final UserHandle parentUserHandle = UserHandle.of(parentUserId);

            AccountManager.get(mContext).addSharedAccountsFromParentUser(parentUserHandle,

            AccountManager.get(mContext).addSharedAccountsFromParentUser(parentUserHandle,

                    UserHandle.of(profile.id));

                    UserHandle.of(profile.id));

            }

            return profile;

            return profile;

        } catch (ServiceSpecificException e) {

        } catch (ServiceSpecificException e) {

            return null;

            return null;

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

+3 −2
Original line number Original line Diff line number Diff line
@@ -238,8 +238,9 @@ public abstract class UserManagerInternal { 
     * the user is created (as it will be passed back to it through

     * the user is created (as it will be passed back to it through

     * {@link UserLifecycleListener#onUserCreated(UserInfo, Object)});

     * {@link UserLifecycleListener#onUserCreated(UserInfo, Object)});

     */

     */

    public abstract UserInfo createUserEvenWhenDisallowed(String name, String userType,

    public abstract @NonNull UserInfo createUserEvenWhenDisallowed(

            int flags, String[] disallowedPackages, @Nullable Object token)

            @Nullable String name, @NonNull String userType, @UserInfo.UserInfoFlag int flags,

            @Nullable String[] disallowedPackages, @Nullable Object token)

            throws UserManager.CheckedUserOperationException;

            throws UserManager.CheckedUserOperationException;





    /**

    /**

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

+60 −44
Original line number Original line Diff line number Diff line
@@ -4494,9 +4494,11 @@ public class UserManagerService extends IUserManager.Stub { 
     * as well as for {@link UserManager#USER_TYPE_FULL_RESTRICTED}.

     * as well as for {@link UserManager#USER_TYPE_FULL_RESTRICTED}.

     */

     */

    @Override

    @Override

    public UserInfo createProfileForUserWithThrow(@Nullable String name, @NonNull String userType,

    public @NonNull UserInfo createProfileForUserWithThrow(

            @UserInfoFlag int flags, @UserIdInt int userId, @Nullable String[] disallowedPackages)

            @Nullable String name, @NonNull String userType, @UserInfoFlag int flags,

            @UserIdInt int userId, @Nullable String[] disallowedPackages)

            throws ServiceSpecificException {

            throws ServiceSpecificException {



        checkCreateUsersPermission(flags);

        checkCreateUsersPermission(flags);

        try {

        try {

            return createUserInternal(name, userType, flags, userId, disallowedPackages);

            return createUserInternal(name, userType, flags, userId, disallowedPackages);
@@ -4509,10 +4511,11 @@ public class UserManagerService extends IUserManager.Stub { 
     * @see #createProfileForUser

     * @see #createProfileForUser

     */

     */

    @Override

    @Override

    public UserInfo createProfileForUserEvenWhenDisallowedWithThrow(String name,

    public @NonNull UserInfo createProfileForUserEvenWhenDisallowedWithThrow(

            @NonNull String userType,

            @Nullable String name, @NonNull String userType, @UserInfoFlag int flags,

            @UserInfoFlag int flags, @UserIdInt int userId, @Nullable String[] disallowedPackages)

            @UserIdInt int userId, @Nullable String[] disallowedPackages)

            throws ServiceSpecificException {

            throws ServiceSpecificException {



        checkCreateUsersPermission(flags);

        checkCreateUsersPermission(flags);

        try {

        try {

            return createUserInternalUnchecked(name, userType, flags, userId,

            return createUserInternalUnchecked(name, userType, flags, userId,
@@ -4523,9 +4526,10 @@ public class UserManagerService extends IUserManager.Stub { 
    }

    }





    @Override

    @Override

    public UserInfo createUserWithThrow(String name, @NonNull String userType,

    public @NonNull UserInfo createUserWithThrow(

            @UserInfoFlag int flags)

            @Nullable String name, @NonNull String userType, @UserInfoFlag int flags)

            throws ServiceSpecificException {

            throws ServiceSpecificException {



        checkCreateUsersPermission(flags);

        checkCreateUsersPermission(flags);

        try {

        try {

            return createUserInternal(name, userType, flags, UserHandle.USER_NULL,

            return createUserInternal(name, userType, flags, UserHandle.USER_NULL,
@@ -4536,7 +4540,10 @@ public class UserManagerService extends IUserManager.Stub { 
    }

    }





    @Override

    @Override

    public UserInfo preCreateUserWithThrow(String userType) throws ServiceSpecificException {

    public @NonNull UserInfo preCreateUserWithThrow(

            @NonNull String userType)

            throws ServiceSpecificException {



        final UserTypeDetails userTypeDetails = mUserTypes.get(userType);

        final UserTypeDetails userTypeDetails = mUserTypes.get(userType);

        final int flags = userTypeDetails != null ? userTypeDetails.getDefaultUserInfoFlags() : 0;

        final int flags = userTypeDetails != null ? userTypeDetails.getDefaultUserInfoFlags() : 0;
@@ -4556,10 +4563,12 @@ public class UserManagerService extends IUserManager.Stub { 
    }

    }





    @Override

    @Override

    public UserHandle createUserWithAttributes(

    public @NonNull UserHandle createUserWithAttributes(

            String userName, String userType, @UserInfoFlag int flags,

            @Nullable String userName, @NonNull String userType, @UserInfoFlag int flags,

            Bitmap userIcon,

            @Nullable Bitmap userIcon, @Nullable String accountName, @Nullable String accountType,

            String accountName, String accountType, PersistableBundle accountOptions) {

            @Nullable PersistableBundle accountOptions)

            throws ServiceSpecificException {



        checkCreateUsersPermission(flags);

        checkCreateUsersPermission(flags);





        if (someUserHasAccountNoChecks(accountName, accountType)) {

        if (someUserHasAccountNoChecks(accountName, accountType)) {
@@ -4569,12 +4578,7 @@ public class UserManagerService extends IUserManager.Stub { 




        UserInfo userInfo;

        UserInfo userInfo;

        try {

        try {

            userInfo = createUserInternal(userName, userType, flags,

            userInfo = createUserInternal(userName, userType, flags, UserHandle.USER_NULL, null);

                    UserHandle.USER_NULL, null);



            if (userInfo == null) {

                throw new ServiceSpecificException(USER_OPERATION_ERROR_UNKNOWN);

            }

        } catch (UserManager.CheckedUserOperationException e) {

        } catch (UserManager.CheckedUserOperationException e) {

            throw e.toServiceSpecificException();

            throw e.toServiceSpecificException();

        }

        }
@@ -4588,7 +4592,8 @@ public class UserManagerService extends IUserManager.Stub { 
        return userInfo.getUserHandle();

        return userInfo.getUserHandle();

    }

    }





    private UserInfo createUserInternal(@Nullable String name, @NonNull String userType,

    private @NonNull UserInfo createUserInternal(

            @Nullable String name, @NonNull String userType,

            @UserInfoFlag int flags, @UserIdInt int parentId,

            @UserInfoFlag int flags, @UserIdInt int parentId,

            @Nullable String[] disallowedPackages)

            @Nullable String[] disallowedPackages)

            throws UserManager.CheckedUserOperationException {

            throws UserManager.CheckedUserOperationException {
@@ -4610,11 +4615,12 @@ public class UserManagerService extends IUserManager.Stub { 
                /* preCreate= */ false, disallowedPackages, /* token= */ null);

                /* preCreate= */ false, disallowedPackages, /* token= */ null);

    }

    }





    private UserInfo createUserInternalUnchecked(@Nullable String name,

    private @NonNull UserInfo createUserInternalUnchecked(

            @NonNull String userType, @UserInfoFlag int flags, @UserIdInt int parentId,

            @Nullable String name, @NonNull String userType, @UserInfoFlag int flags,

            boolean preCreate, @Nullable String[] disallowedPackages,

            @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages,

            @Nullable Object token)

            @Nullable Object token)

            throws UserManager.CheckedUserOperationException {

            throws UserManager.CheckedUserOperationException {



        final int noneUserId = -1;

        final int noneUserId = -1;

        final TimingsTraceAndSlog t = new TimingsTraceAndSlog();

        final TimingsTraceAndSlog t = new TimingsTraceAndSlog();

        t.traceBegin("createUser-" + flags);

        t.traceBegin("createUser-" + flags);
@@ -4632,27 +4638,31 @@ public class UserManagerService extends IUserManager.Stub { 
        }

        }

    }

    }





    private UserInfo createUserInternalUncheckedNoTracing(@Nullable String name,

    private @NonNull UserInfo createUserInternalUncheckedNoTracing(

            @NonNull String userType, @UserInfoFlag int flags, @UserIdInt int parentId,

            @Nullable String name, @NonNull String userType, @UserInfoFlag int flags,

            boolean preCreate, @Nullable String[] disallowedPackages,

            @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages,

            @NonNull TimingsTraceAndSlog t, @Nullable Object token)

            @NonNull TimingsTraceAndSlog t, @Nullable Object token)

            throws UserManager.CheckedUserOperationException {

            throws UserManager.CheckedUserOperationException {



        final UserTypeDetails userTypeDetails = mUserTypes.get(userType);

        final UserTypeDetails userTypeDetails = mUserTypes.get(userType);

        if (userTypeDetails == null) {

        if (userTypeDetails == null) {

            Slog.e(LOG_TAG, "Cannot create user of invalid user type: " + userType);

            throwCheckedUserOperationException(

            return null;

                    "Cannot create user of invalid user type: " + userType,

                    USER_OPERATION_ERROR_UNKNOWN);

        }

        }

        userType = userType.intern(); // Now that we know it's valid, we can intern it.

        userType = userType.intern(); // Now that we know it's valid, we can intern it.

        flags |= userTypeDetails.getDefaultUserInfoFlags();

        flags |= userTypeDetails.getDefaultUserInfoFlags();

        if (!checkUserTypeConsistency(flags)) {

        if (!checkUserTypeConsistency(flags)) {

            Slog.e(LOG_TAG, "Cannot add user. Flags (" + Integer.toHexString(flags)

            throwCheckedUserOperationException(

                    + ") and userTypeDetails (" + userType +  ") are inconsistent.");

                    "Cannot add user. Flags (" + Integer.toHexString(flags)

            return null;

                            + ") and userTypeDetails (" + userType +  ") are inconsistent.",

                    USER_OPERATION_ERROR_UNKNOWN);

        }

        }

        if ((flags & UserInfo.FLAG_SYSTEM) != 0) {

        if ((flags & UserInfo.FLAG_SYSTEM) != 0) {

            Slog.e(LOG_TAG, "Cannot add user. Flags (" + Integer.toHexString(flags)

            throwCheckedUserOperationException(

                    + ") indicated SYSTEM user, which cannot be created.");

                    "Cannot add user. Flags (" + Integer.toHexString(flags)

            return null;

                            + ") indicated SYSTEM user, which cannot be created.",

                    USER_OPERATION_ERROR_UNKNOWN);

        }

        }

        if (!isUserTypeEnabled(userTypeDetails)) {

        if (!isUserTypeEnabled(userTypeDetails)) {

            throwCheckedUserOperationException(

            throwCheckedUserOperationException(
@@ -4678,7 +4688,8 @@ public class UserManagerService extends IUserManager.Stub { 
        DeviceStorageMonitorInternal dsm = LocalServices

        DeviceStorageMonitorInternal dsm = LocalServices

                .getService(DeviceStorageMonitorInternal.class);

                .getService(DeviceStorageMonitorInternal.class);

        if (dsm.isMemoryLow()) {

        if (dsm.isMemoryLow()) {

            throwCheckedUserOperationException("Cannot add user. Not enough space on disk.",

            throwCheckedUserOperationException(

                    "Cannot add user. Not enough space on disk.",

                    UserManager.USER_OPERATION_ERROR_LOW_STORAGE);

                    UserManager.USER_OPERATION_ERROR_LOW_STORAGE);

        }

        }
@@ -4706,7 +4717,8 @@ public class UserManagerService extends IUserManager.Stub { 
                    }

                    }

                }

                }

                if (!preCreate && !canAddMoreUsersOfType(userTypeDetails)) {

                if (!preCreate && !canAddMoreUsersOfType(userTypeDetails)) {

                    throwCheckedUserOperationException("Cannot add more users of type " + userType

                    throwCheckedUserOperationException(

                            "Cannot add more users of type " + userType

                                    + ". Maximum number of that type already exists.",

                                    + ". Maximum number of that type already exists.",

                            UserManager.USER_OPERATION_ERROR_MAX_USERS);

                            UserManager.USER_OPERATION_ERROR_MAX_USERS);

                }

                }
@@ -5331,13 +5343,13 @@ public class UserManagerService extends IUserManager.Stub { 
     * @hide

     * @hide

     */

     */

    @Override

    @Override

    public UserInfo createRestrictedProfileWithThrow(@Nullable String name, int parentUserId) {

    public @NonNull UserInfo createRestrictedProfileWithThrow(

            @Nullable String name, @UserIdInt int parentUserId)

            throws ServiceSpecificException {



        checkCreateUsersPermission("setupRestrictedProfile");

        checkCreateUsersPermission("setupRestrictedProfile");

        final UserInfo user = createProfileForUserWithThrow(

        final UserInfo user = createProfileForUserWithThrow(

                name, UserManager.USER_TYPE_FULL_RESTRICTED, 0, parentUserId, null);

                name, UserManager.USER_TYPE_FULL_RESTRICTED, 0, parentUserId, null);

        if (user == null) {

            return null;

        }

        final long identity = Binder.clearCallingIdentity();

        final long identity = Binder.clearCallingIdentity();

        try {

        try {

            setUserRestriction(UserManager.DISALLOW_MODIFY_ACCOUNTS, true, user.id);

            setUserRestriction(UserManager.DISALLOW_MODIFY_ACCOUNTS, true, user.id);
@@ -6338,8 +6350,10 @@ public class UserManagerService extends IUserManager.Stub { 
        setSeedAccountDataNoChecks(userId, accountName, accountType, accountOptions, persist);

        setSeedAccountDataNoChecks(userId, accountName, accountType, accountOptions, persist);

    }

    }





    private void setSeedAccountDataNoChecks(@UserIdInt int userId, String accountName,

    private void setSeedAccountDataNoChecks(@UserIdInt int userId, @Nullable String accountName,

            String accountType, PersistableBundle accountOptions, boolean persist) {

            @Nullable String accountType, @Nullable PersistableBundle accountOptions,

            boolean persist) {



        synchronized (mPackagesLock) {

        synchronized (mPackagesLock) {

            final UserData userData;

            final UserData userData;

            synchronized (mUsersLock) {

            synchronized (mUsersLock) {
@@ -6908,9 +6922,11 @@ public class UserManagerService extends IUserManager.Stub { 
        }

        }





        @Override

        @Override

        public UserInfo createUserEvenWhenDisallowed(String name, @NonNull String userType,

        public @NonNull UserInfo createUserEvenWhenDisallowed(

                @UserInfoFlag int flags, String[] disallowedPackages, @Nullable Object token)

                @Nullable String name, @NonNull String userType, @UserInfoFlag int flags,

                @Nullable String[] disallowedPackages, @Nullable Object token)

                throws UserManager.CheckedUserOperationException {

                throws UserManager.CheckedUserOperationException {



            return createUserInternalUnchecked(name, userType, flags,

            return createUserInternalUnchecked(name, userType, flags,

                    UserHandle.USER_NULL, /* preCreated= */ false, disallowedPackages, token);

                    UserHandle.USER_NULL, /* preCreated= */ false, disallowedPackages, token);

        }

        }

services/java/com/android/server/HsumBootUserInitializer.java

+1 −5
Original line number Original line Diff line number Diff line
@@ -112,11 +112,7 @@ final class HsumBootUserInitializer { 
                    UserInfo.FLAG_ADMIN | UserInfo.FLAG_MAIN,

                    UserInfo.FLAG_ADMIN | UserInfo.FLAG_MAIN,

                    /* disallowedPackages= */ null,

                    /* disallowedPackages= */ null,

                    /* token= */ null);

                    /* token= */ null);

            if (newInitialUser == null) {

                Slogf.wtf(TAG, "Initial bootable MainUser creation failed: returned null");

            } else {

            Slogf.i(TAG, "Successfully created MainUser, userId=%d", newInitialUser.id);

            Slogf.i(TAG, "Successfully created MainUser, userId=%d", newInitialUser.id);

            }

        } catch (UserManager.CheckedUserOperationException e) {

        } catch (UserManager.CheckedUserOperationException e) {

            Slogf.wtf(TAG, "Initial bootable MainUser creation failed", e);

            Slogf.wtf(TAG, "Initial bootable MainUser creation failed", e);

        }

        }