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

Fix deadlock in RebootEscrowManager

Browse Source 
Problem: RebootEscrowManager tries to take ActivityManager lock while
Synthetic Password Manager lock is already held.

Solution: RebootEscrowManager offload call to ActivityManager to a
handler thread.

Test: atest com.android.server.locksettings and manual testing of RoR
flow on physical device

Bug: 270006672

Change-Id: Icbc7127369cfe2eb0a6e0e554134f19b801b0274
This commit is contained in:
Veena Arvind
2023-03-03 20:58:56 +00:00
parent 0ca2959dc7
commit f9f0a8757b
3 changed files with 68 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
services/core/java/com/android/server/locksettings/LockSettingsService.java
View File

@@ -416,6 +416,8 @@ public class LockSettingsService extends ILockSettings.Stub {
static class Injector {
protected Context mContext;
private ServiceThread mHandlerThread;
private Handler mHandler;
public Injector(Context context) {
mContext = context;
@@ -426,14 +428,20 @@ public class LockSettingsService extends ILockSettings.Stub {
}
public ServiceThread getServiceThread() {
ServiceThread handlerThread = new ServiceThread(TAG, Process.THREAD_PRIORITY_BACKGROUND,
true /*allowIo*/);
handlerThread.start();
return handlerThread;
if (mHandlerThread == null) {
mHandlerThread = new ServiceThread(TAG,
Process.THREAD_PRIORITY_BACKGROUND,
true /*allowIo*/);
mHandlerThread.start();
}
return mHandlerThread;
}
public Handler getHandler(ServiceThread handlerThread) {
return new Handler(handlerThread.getLooper());
if (mHandler == null) {
mHandler = new Handler(handlerThread.getLooper());
}
return mHandler;
}
public LockSettingsStorage getStorage() {
@@ -514,7 +522,8 @@ public class LockSettingsService extends ILockSettings.Stub {
public RebootEscrowManager getRebootEscrowManager(RebootEscrowManager.Callbacks callbacks,
LockSettingsStorage storage) {
return new RebootEscrowManager(mContext, callbacks, storage);
return new RebootEscrowManager(mContext, callbacks, storage,
getHandler(getServiceThread()));
}
public boolean hasEnrolledBiometrics(int userId) {

12
services/core/java/com/android/server/locksettings/RebootEscrowManager.java
View File

@@ -205,6 +205,8 @@ class RebootEscrowManager {
private final RebootEscrowKeyStoreManager mKeyStoreManager;
private final Handler mHandler;
PowerManager.WakeLock mWakeLock;
private ConnectivityManager.NetworkCallback mNetworkCallback;
@@ -399,19 +401,21 @@ class RebootEscrowManager {
}
}
RebootEscrowManager(Context context, Callbacks callbacks, LockSettingsStorage storage) {
this(new Injector(context, storage), callbacks, storage);
RebootEscrowManager(Context context, Callbacks callbacks, LockSettingsStorage storage,
Handler handler) {
this(new Injector(context, storage), callbacks, storage, handler);
}
@VisibleForTesting
RebootEscrowManager(Injector injector, Callbacks callbacks,
LockSettingsStorage storage) {
LockSettingsStorage storage, Handler handler) {
mInjector = injector;
mCallbacks = callbacks;
mStorage = storage;
mUserManager = injector.getUserManager();
mEventLog = injector.getEventLog();
mKeyStoreManager = injector.getKeyStoreManager();
mHandler = handler;
}
/** Wrapper function to set error code serialized through handler, */
@@ -937,7 +941,7 @@ class RebootEscrowManager {
private void setRebootEscrowReady(boolean ready) {
if (mRebootEscrowReady != ready) {
mRebootEscrowListener.onPreparedForReboot(ready);
mHandler.post(() -> mRebootEscrowListener.onPreparedForReboot(ready));
}
mRebootEscrowReady = ready;
}

76
services/tests/servicestests/src/com/android/server/locksettings/RebootEscrowManagerTests.java
View File

@@ -74,6 +74,8 @@ import org.mockito.ArgumentCaptor;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import javax.crypto.SecretKey;
@@ -327,16 +329,30 @@ public class RebootEscrowManagerTests {
mInjected = mock(MockableRebootEscrowInjected.class);
mMockInjector = new MockInjector(mContext, mUserManager, mRebootEscrow,
mKeyStoreManager, mStorage, mInjected);
mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
HandlerThread thread = new HandlerThread("RebootEscrowManagerTest");
thread.start();
mHandler = new Handler(thread.getLooper());
mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage, mHandler);
}
private void setServerBasedRebootEscrowProvider() throws Exception {
mMockInjector = new MockInjector(mContext, mUserManager, mServiceConnection,
mKeyStoreManager, mStorage, mInjected);
mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage);
mService = new RebootEscrowManager(mMockInjector, mCallbacks, mStorage, mHandler);
}
private void waitForHandler() throws InterruptedException {
// Wait for handler to complete processing.
CountDownLatch latch = new CountDownLatch(1);
mHandler.post(latch::countDown);
assertTrue(latch.await(5, TimeUnit.SECONDS));
}
private void callToRebootEscrowIfNeededAndWait(int userId) throws InterruptedException {
mService.callToRebootEscrowIfNeeded(userId, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
waitForHandler();
}
@Test
@@ -346,7 +362,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
}
@@ -358,8 +374,7 @@ public class RebootEscrowManagerTests {
mService.setRebootEscrowListener(mockListener);
mService.prepareRebootEscrow();
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
verify(mockListener).onPreparedForReboot(eq(true));
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
assertFalse(mStorage.hasRebootEscrowServerBlob());
}
@@ -369,7 +384,7 @@ public class RebootEscrowManagerTests {
RebootEscrowListener mockListener = mock(RebootEscrowListener.class);
mService.setRebootEscrowListener(mockListener);
mService.prepareRebootEscrow();
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
clearInvocations(mRebootEscrow);
@@ -393,7 +408,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -417,7 +432,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -438,7 +453,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -456,10 +471,9 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
mService.callToRebootEscrowIfNeeded(SECURE_SECONDARY_USER_ID, FAKE_SP_VERSION,
FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(SECURE_SECONDARY_USER_ID);
verify(mRebootEscrow, never()).storeKey(any());
assertTrue(mStorage.hasRebootEscrow(PRIMARY_USER_ID));
@@ -491,7 +505,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -514,7 +528,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -557,7 +571,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -601,7 +615,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -646,7 +660,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -692,7 +706,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -741,7 +755,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -794,7 +808,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -849,7 +863,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -896,7 +910,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -952,7 +966,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -1011,7 +1025,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -1071,7 +1085,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -1127,7 +1141,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mServiceConnection);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mServiceConnection, never()).wrapBlob(any(), anyLong(), anyLong());
@@ -1179,7 +1193,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -1210,7 +1224,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -1238,7 +1252,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -1277,7 +1291,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
verify(mRebootEscrow, never()).storeKey(any());
@@ -1312,7 +1326,7 @@ public class RebootEscrowManagerTests {
mService.prepareRebootEscrow();
clearInvocations(mRebootEscrow);
mService.callToRebootEscrowIfNeeded(PRIMARY_USER_ID, FAKE_SP_VERSION, FAKE_AUTH_TOKEN);
callToRebootEscrowIfNeededAndWait(PRIMARY_USER_ID);
verify(mockListener).onPreparedForReboot(eq(true));
assertTrue(mStorage.hasRebootEscrow(PRIMARY_USER_ID));
verify(mRebootEscrow, never()).storeKey(any());