From 30524ebfec4742199897f0294478701c8e932348 Mon Sep 17 00:00:00 2001 From: Joshua McCloskey Date: Thu, 9 Jun 2022 22:05:16 +0000 Subject: [PATCH] UserAwareBiometricScheduler lifecycle fix. Fixed an issue where UserAwareBiometricScheduler queue would get stuck indefinitely(causing face/fingerprint to fail until reboot). Testing steps to reproduce Setup. * Have multiple users with multipler biometrics. 1. Add a property in a biometric HAL(Call it biometric A) to hang on auth operation 2. Set property to hang when authenticating(biometric A). 3. Unlock device with alternative biometric(biometric B)/pin/pattern/pass 4. Unset property to stop biometric(biometric A) from hanging 5. Rinse and repeat 1-4 until a hang occurs Test: Manual (See above) Test: atest UserAwareBiometricSchedulerTest Fixes: 234625004 Change-Id: Ia6055ef3314ddaf986d5dfb4b642a5aba183053d --- .../sensors/UserAwareBiometricScheduler.java | 13 +++++ .../UserAwareBiometricSchedulerTest.java | 57 +++++++++++++++++-- 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java index a0999771a1be8..ae75b7dcc1016 100644 --- a/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java +++ b/services/core/java/com/android/server/biometrics/sensors/UserAwareBiometricScheduler.java @@ -67,6 +67,14 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { public void onClientFinished(@NonNull BaseClientMonitor clientMonitor, boolean success) { mHandler.post(() -> { Slog.d(getTag(), "[Client finished] " + clientMonitor + ", success: " + success); + + // Set mStopUserClient to null when StopUserClient fails. Otherwise it's possible + // for that the queue will wait indefinitely until the field is cleared. + if (clientMonitor instanceof StopUserClient && !success) { + Slog.w(getTag(), + "StopUserClient failed(), is the HAL stuck? Clearing mStopUserClient"); + mStopUserClient = null; + } if (mCurrentOperation != null && mCurrentOperation.isFor(mOwner)) { mCurrentOperation = null; } else { @@ -166,4 +174,9 @@ public class UserAwareBiometricScheduler extends BiometricScheduler { mStopUserClient.onUserStopped(); mStopUserClient = null; } + + @VisibleForTesting + @Nullable public StopUserClient getStopUserClient() { + return mStopUserClient; + } } diff --git a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java index 8391914a0bb60..0df3028805d2f 100644 --- a/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java +++ b/services/tests/servicestests/src/com/android/server/biometrics/sensors/UserAwareBiometricSchedulerTest.java @@ -80,6 +80,11 @@ public class UserAwareBiometricSchedulerTest { @Mock private BiometricContext mBiometricContext; + private boolean mShouldFailStopUser = false; + private final StopUserClientShouldFail mStopUserClientShouldFail = + () -> { + return mShouldFailStopUser; + }; private final TestUserStartedCallback mUserStartedCallback = new TestUserStartedCallback(); private final TestUserStoppedCallback mUserStoppedCallback = new TestUserStoppedCallback(); private int mCurrentUserId = UserHandle.USER_NULL; @@ -88,6 +93,7 @@ public class UserAwareBiometricSchedulerTest { @Before public void setUp() { + mShouldFailStopUser = false; mHandler = new Handler(TestableLooper.get(this).getLooper()); mScheduler = new UserAwareBiometricScheduler(TAG, mHandler, @@ -101,7 +107,7 @@ public class UserAwareBiometricSchedulerTest { public StopUserClient getStopUserClient(int userId) { return new TestStopUserClient(mContext, Object::new, mToken, userId, TEST_SENSOR_ID, mBiometricLogger, mBiometricContext, - mUserStoppedCallback); + mUserStoppedCallback, mStopUserClientShouldFail); } @NonNull @@ -240,6 +246,36 @@ public class UserAwareBiometricSchedulerTest { assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(1); } + @Test + public void testStartUser_failsClearsStopUserClient() { + // When a stop user client fails, check that mStopUserClient + // is set to null to prevent the scheduler from getting stuck. + BaseClientMonitor nextClient = mock(BaseClientMonitor.class); + when(nextClient.getTargetUserId()).thenReturn(10); + + mScheduler.scheduleClientMonitor(nextClient); + + waitForIdle(); + verify(nextClient).start(any()); + + // finish first operation + mScheduler.getInternalCallback().onClientFinished(nextClient, true /* success */); + waitForIdle(); + + // schedule second operation but swap out the current operation + // before it runs so that it's not current when it's completion callback runs + nextClient = mock(BaseClientMonitor.class); + when(nextClient.getTargetUserId()).thenReturn(11); + mUserStartedCallback.mAfterStart = () -> mScheduler.mCurrentOperation = null; + mShouldFailStopUser = true; + mScheduler.scheduleClientMonitor(nextClient); + + waitForIdle(); + assertThat(mUserStartedCallback.mStartedUsers).containsExactly(10, 11).inOrder(); + assertThat(mUserStoppedCallback.mNumInvocations).isEqualTo(0); + assertThat(mScheduler.getStopUserClient()).isEqualTo(null); + } + private void waitForIdle() { TestableLooper.get(this).processAllMessages(); } @@ -268,13 +304,19 @@ public class UserAwareBiometricSchedulerTest { } } - private static class TestStopUserClient extends StopUserClient { + private interface StopUserClientShouldFail { + boolean shouldFail(); + } + + private class TestStopUserClient extends StopUserClient { + private StopUserClientShouldFail mShouldFailClient; public TestStopUserClient(@NonNull Context context, @NonNull Supplier lazyDaemon, @Nullable IBinder token, int userId, int sensorId, @NonNull BiometricLogger logger, @NonNull BiometricContext biometricContext, - @NonNull UserStoppedCallback callback) { + @NonNull UserStoppedCallback callback, StopUserClientShouldFail shouldFail) { super(context, lazyDaemon, token, userId, sensorId, logger, biometricContext, callback); + mShouldFailClient = shouldFail; } @Override @@ -285,7 +327,14 @@ public class UserAwareBiometricSchedulerTest { @Override public void start(@NonNull ClientMonitorCallback callback) { super.start(callback); - onUserStopped(); + if (mShouldFailClient.shouldFail()) { + getCallback().onClientFinished(this, false /* success */); + // When the above fails, it means that the HAL has died, in this case we + // need to ensure the UserSwitchCallback correctly returns the NULL user handle. + mCurrentUserId = UserHandle.USER_NULL; + } else { + onUserStopped(); + } } @Override