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

Fix potential deadlock in applyUserRestrictions methods.

Browse Source 
This fixes a lock inversion issue where the user lock is being taken
out while the restrictions lock is held in UserManagerService.

The user lock is being taken out to retrieve user data in order to
call scheduleWriteUser, but when the write occurs, we don't actually
use that UserData as it is retrieved again. We only use the provided
UserData to get the user id. So, this CL changes scheduleWriteUser to
take the user id instead.

Bug: 273083107
Test: atest UserManagerTest
Change-Id: I11db509235dfc59642871c168bc2f3e1daabe821
This commit is contained in:
Oli Lan
2023-04-05 11:46:18 +01:00
parent 901bee878f
commit a22346a17a
1 changed files with 10 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -2592,7 +2592,7 @@ public class UserManagerService extends IUserManager.Stub {
}
}
if (scheduleWriteUser) {
scheduleWriteUser(userData);
scheduleWriteUser(userId);
}
}
@@ -2902,7 +2902,7 @@ public class UserManagerService extends IUserManager.Stub {
!= newBaseRestrictions);
if (mBaseUserRestrictions.updateRestrictions(userId, newBaseRestrictions)) {
scheduleWriteUser(getUserDataNoChecks(userId));
scheduleWriteUser(userId);
}
}
@@ -2978,7 +2978,7 @@ public class UserManagerService extends IUserManager.Stub {
@GuardedBy("mRestrictionsLock")
private void applyUserRestrictionsLR(@UserIdInt int userId) {
updateUserRestrictionsInternalLR(null, userId);
scheduleWriteUser(getUserDataNoChecks(userId));
scheduleWriteUser(userId);
}
@GuardedBy("mRestrictionsLock")
@@ -4129,14 +4129,14 @@ public class UserManagerService extends IUserManager.Stub {
}
}
private void scheduleWriteUser(UserData userData) {
private void scheduleWriteUser(@UserIdInt int userId) {
if (DBG) {
debug("scheduleWriteUser");
}
// No need to wrap it within a lock -- worst case, we'll just post the same message
// twice.
if (!mHandler.hasMessages(WRITE_USER_MSG, userData)) {
Message msg = mHandler.obtainMessage(WRITE_USER_MSG, userData);
if (!mHandler.hasMessages(WRITE_USER_MSG, userId)) {
Message msg = mHandler.obtainMessage(WRITE_USER_MSG, userId);
mHandler.sendMessageDelayed(msg, WRITE_USER_DELAY);
}
}
@@ -4152,7 +4152,7 @@ public class UserManagerService extends IUserManager.Stub {
// Something went wrong, schedule full rewrite.
UserData userData = getUserDataNoChecks(userId);
if (userData != null) {
scheduleWriteUser(userData);
scheduleWriteUser(userId);
}
});
}
@@ -6363,7 +6363,7 @@ public class UserManagerService extends IUserManager.Stub {
userData.info.lastLoggedInTime = now;
}
userData.info.lastLoggedInFingerprint = PackagePartitions.FINGERPRINT;
scheduleWriteUser(userData);
scheduleWriteUser(userId);
}
/**
@@ -6533,7 +6533,7 @@ public class UserManagerService extends IUserManager.Stub {
private void setLastEnteredForegroundTimeToNow(@NonNull UserData userData) {
userData.mLastEnteredForegroundTimeMillis = System.currentTimeMillis();
scheduleWriteUser(userData);
scheduleWriteUser(userData.info.id);
}
@Override
@@ -6832,7 +6832,7 @@ public class UserManagerService extends IUserManager.Stub {
case WRITE_USER_MSG:
removeMessages(WRITE_USER_MSG, msg.obj);
synchronized (mPackagesLock) {
int userId = ((UserData) msg.obj).info.id;
int userId = (int) msg.obj;
UserData userData = getUserDataNoChecks(userId);
if (userData != null) {
writeUserLP(userData);