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

Added more checks on startUserInBackgroundOnSecondaryDisplay():

Browse Source 
- Profile can only start on same display as user.
- Only one (full) user can be started in a display.
- Cannot be called on DEFAULT_DISPLAY

Test: atest CtsMultiUserTestCases:android.multiuser.cts.UserManagerTest
Bug: 240736142
Bug: 239824814

Change-Id: I8c5540ec34aab6a84e5ab29f89ad8baf658df1d0
This commit is contained in:
Felipe Leme
2022-08-09 12:41:25 -07:00
parent e5b4ecaf97
commit f2e0460ca4
2 changed files with 48 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
services/core/java/com/android/server/am/UserController.java
View File

@@ -1468,7 +1468,16 @@ class UserController implements Handler.Callback {
checkCallingHasOneOfThosePermissions("startUserOnSecondaryDisplay",
MANAGE_USERS, CREATE_USERS);
return startUserNoChecks(userId, displayId, /* foreground= */ false, unlockListener);
// DEFAULT_DISPLAY is used for "regular" start user operations
Preconditions.checkArgument(displayId != Display.DEFAULT_DISPLAY,
"Cannot use DEFAULT_DISPLAY");
try {
return startUserNoChecks(userId, displayId, /* foreground= */ false, unlockListener);
} catch (RuntimeException e) {
Slogf.w(TAG, "startUserOnSecondaryDisplay(%d, %d) failed: %s", userId, displayId, e);
return false;
}
}
private boolean startUserNoChecks(@UserIdInt int userId, int displayId, boolean foreground,
@@ -1492,13 +1501,15 @@ class UserController implements Handler.Callback {
foreground ? " in foreground" : "");
}
// TODO(b/239982558): move logic below to a different class (like DisplayAssignmentManager)
if (displayId != Display.DEFAULT_DISPLAY) {
// This is called by startUserOnSecondaryDisplay()
if (!UserManager.isUsersOnSecondaryDisplaysEnabled()) {
// TODO(b/239824814): add CTS test and/or unit test
// TODO(b/239824814): add CTS test and/or unit test for all exceptional cases
throw new UnsupportedOperationException("Not supported by device");
}
// TODO(b/239982558): add unit test for the exceptional cases below
// TODO(b/239982558): call DisplayManagerInternal to check if display is valid instead
Preconditions.checkArgument(displayId > 0, "Invalid display id (%d)", displayId);
Preconditions.checkArgument(userId != UserHandle.USER_SYSTEM, "Cannot start system user"
+ " on secondary display (%d)", displayId);
@@ -1508,7 +1519,6 @@ class UserController implements Handler.Callback {
// TODO(b/239982558): for now we're just updating the user's visibility, but most likely
// we'll need to remove this call and handle that as part of the user state workflow
// instead.
// TODO(b/239982558): check if display is valid
mInjector.getUserManagerInternal().assignUserToDisplay(userId, displayId);
}

37
services/core/java/com/android/server/pm/UserManagerService.java
View File

@@ -1587,6 +1587,10 @@ public class UserManagerService extends IUserManager.Stub {
public boolean isProfile(@UserIdInt int userId) {
checkQueryOrInteractPermissionIfCallerInOtherProfileGroup(userId, "isProfile");
return isProfileUnchecked(userId);
}
private boolean isProfileUnchecked(@UserIdInt int userId) {
synchronized (mUsersLock) {
UserInfo userInfo = getUserInfoLU(userId);
return userInfo != null && userInfo.isProfile();
@@ -6827,9 +6831,6 @@ public class UserManagerService extends IUserManager.Stub {
return;
}
// TODO(b/239982558) check for invalid cases like:
// - userId already assigned to another display
// - displayId already assigned to another user
synchronized (mUsersLock) {
if (mUsersOnSecondaryDisplays == null) {
if (DBG) {
@@ -6841,6 +6842,36 @@ public class UserManagerService extends IUserManager.Stub {
Slogf.d(LOG_TAG, "Adding %d->%d to mUsersOnSecondaryDisplays",
userId, displayId);
}
if (isProfileUnchecked(userId)) {
// Profile can only start in the same display as parent
int parentUserId = getProfileParentId(userId);
int parentDisplayId = mUsersOnSecondaryDisplays.get(parentUserId);
if (displayId != parentDisplayId) {
throw new IllegalStateException("Cannot assign profile " + userId + " to "
+ "display " + displayId + " as its parent (user " + parentUserId
+ ") is assigned to display " + parentDisplayId);
}
} else {
// Check if display is available
for (int i = 0; i < mUsersOnSecondaryDisplays.size(); i++) {
// Make sure display is not used by other users...
// TODO(b/240736142); currently, if a user was started in a display, it
// would need to be stopped first, so "switching" a user on secondary
// diplay requires 2 non-atomic operations (stop and start). Once this logic
// is refactored, it should be atomic.
if (mUsersOnSecondaryDisplays.valueAt(i) == displayId) {
throw new IllegalStateException("Cannot assign " + userId + " to "
+ "display " + displayId + " as it's already assigned to "
+ "user " + mUsersOnSecondaryDisplays.keyAt(i));
}
// TODO(b/239982558) also check that user is not already assigned to other
// display (including 0). That would be harder to tested under CTS though
// (for example, would need to add a new AM method to start user in bg on
// main display), so it's better to test on unit tests
}
}
mUsersOnSecondaryDisplays.put(userId, displayId);
}
}