From 77df3dffa51f768ef4aea5374933d6f0bd3730fa Mon Sep 17 00:00:00 2001 From: Kevin Chyn Date: Mon, 8 Feb 2021 12:42:37 -0800 Subject: [PATCH] Clean up biometric AIDL binderDied handling 1) We should only register one death recipient - in the top-level provider class. The provider can notify all sensors to clean up when the HAL dies 2) Always set mCurrentSession to null. Otherwise, if current client is null, or non-interruptable, subsequent attempts to access the HAL will result in a DeadObjectException Fixes: 179097844 Test: atest com.android.server.biometrics Change-Id: I8604dc1988b8b7f3572596b1431da4d6e5c393c2 --- .../sensors/face/aidl/FaceProvider.java | 3 +- .../biometrics/sensors/face/aidl/Sensor.java | 35 ++++++++----------- .../fingerprint/aidl/FingerprintProvider.java | 3 +- .../sensors/fingerprint/aidl/Sensor.java | 35 ++++++++----------- 4 files changed, 32 insertions(+), 44 deletions(-) diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java index e685ee2899afa..1b6b9d70d5ac8 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/FaceProvider.java @@ -643,8 +643,7 @@ public class FaceProvider implements IBinder.DeathRecipient, ServiceProvider { final Sensor sensor = mSensors.valueAt(i); final int sensorId = mSensors.keyAt(i); PerformanceTracker.getInstanceForSensorId(sensorId).incrementHALDeathCount(); - sensor.getScheduler().recordCrashState(); - sensor.getScheduler().reset(); + sensor.onBinderDied(); } }); } diff --git a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java index 640838c6ee049..baeb3fdda67dc 100644 --- a/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/face/aidl/Sensor.java @@ -33,7 +33,6 @@ import android.hardware.face.FaceManager; import android.hardware.face.FaceSensorPropertiesInternal; import android.hardware.keymaster.HardwareAuthToken; import android.os.Handler; -import android.os.IBinder; import android.os.RemoteException; import android.os.UserManager; import android.util.Slog; @@ -63,7 +62,7 @@ import java.util.Map; /** * Maintains the state of a single sensor within an instance of the {@link IFace} HAL. */ -public class Sensor implements IBinder.DeathRecipient { +public class Sensor { private boolean mTestHalEnabled; @@ -481,7 +480,6 @@ public class Sensor implements IBinder.DeathRecipient { mTag, mScheduler, sensorId, userId, callback); final ISession newSession = daemon.createSession(sensorId, userId, resultController); - newSession.asBinder().linkToDeath(this, 0 /* flags */); mCurrentSession = new Session(mTag, newSession, userId, resultController); } @@ -523,24 +521,21 @@ public class Sensor implements IBinder.DeathRecipient { proto.end(sensorToken); } - @Override - public void binderDied() { - Slog.e(mTag, "Binder died"); - mHandler.post(() -> { - final BaseClientMonitor client = mScheduler.getCurrentClient(); - if (client instanceof Interruptable) { - Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(FaceManager.FACE_ERROR_HW_UNAVAILABLE, - 0 /* vendorCode */); + public void onBinderDied() { + final BaseClientMonitor client = mScheduler.getCurrentClient(); + if (client instanceof Interruptable) { + Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); + final Interruptable interruptable = (Interruptable) client; + interruptable.onError(FaceManager.FACE_ERROR_HW_UNAVAILABLE, + 0 /* vendorCode */); - mScheduler.recordCrashState(); + FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, + BiometricsProtoEnums.MODALITY_FACE, + BiometricsProtoEnums.ISSUE_HAL_DEATH); + } - FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, - BiometricsProtoEnums.MODALITY_FACE, - BiometricsProtoEnums.ISSUE_HAL_DEATH); - mCurrentSession = null; - } - }); + mScheduler.recordCrashState(); + mScheduler.reset(); + mCurrentSession = null; } } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java index ced46e140c0a8..0bd2f241ed8d2 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/FingerprintProvider.java @@ -697,8 +697,7 @@ public class FingerprintProvider implements IBinder.DeathRecipient, ServiceProvi final Sensor sensor = mSensors.valueAt(i); final int sensorId = mSensors.keyAt(i); PerformanceTracker.getInstanceForSensorId(sensorId).incrementHALDeathCount(); - sensor.getScheduler().recordCrashState(); - sensor.getScheduler().reset(); + sensor.onBinderDied(); } }); } diff --git a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java index f0e7e1cf5d254..7e4ee9e77ab22 100644 --- a/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java +++ b/services/core/java/com/android/server/biometrics/sensors/fingerprint/aidl/Sensor.java @@ -31,7 +31,6 @@ import android.hardware.fingerprint.FingerprintManager; import android.hardware.fingerprint.FingerprintSensorPropertiesInternal; import android.hardware.keymaster.HardwareAuthToken; import android.os.Handler; -import android.os.IBinder; import android.os.RemoteException; import android.os.UserManager; import android.util.Slog; @@ -65,7 +64,7 @@ import java.util.Map; * {@link android.hardware.biometrics.fingerprint.IFingerprint} HAL. */ @SuppressWarnings("deprecation") -class Sensor implements IBinder.DeathRecipient { +class Sensor { private boolean mTestHalEnabled; @@ -461,7 +460,6 @@ class Sensor implements IBinder.DeathRecipient { mTag, mScheduler, sensorId, userId, callback); final ISession newSession = daemon.createSession(sensorId, userId, resultController); - newSession.asBinder().linkToDeath(this, 0 /* flags */); mCurrentSession = new Session(mTag, newSession, userId, resultController); } @@ -503,24 +501,21 @@ class Sensor implements IBinder.DeathRecipient { proto.end(sensorToken); } - @Override - public void binderDied() { - Slog.e(mTag, "Binder died"); - mHandler.post(() -> { - final BaseClientMonitor client = mScheduler.getCurrentClient(); - if (client instanceof Interruptable) { - Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); - final Interruptable interruptable = (Interruptable) client; - interruptable.onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE, - 0 /* vendorCode */); + public void onBinderDied() { + final BaseClientMonitor client = mScheduler.getCurrentClient(); + if (client instanceof Interruptable) { + Slog.e(mTag, "Sending ERROR_HW_UNAVAILABLE for client: " + client); + final Interruptable interruptable = (Interruptable) client; + interruptable.onError(FingerprintManager.FINGERPRINT_ERROR_HW_UNAVAILABLE, + 0 /* vendorCode */); - mScheduler.recordCrashState(); + FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, + BiometricsProtoEnums.MODALITY_FINGERPRINT, + BiometricsProtoEnums.ISSUE_HAL_DEATH); + } - FrameworkStatsLog.write(FrameworkStatsLog.BIOMETRIC_SYSTEM_HEALTH_ISSUE_DETECTED, - BiometricsProtoEnums.MODALITY_FINGERPRINT, - BiometricsProtoEnums.ISSUE_HAL_DEATH); - mCurrentSession = null; - } - }); + mScheduler.recordCrashState(); + mScheduler.reset(); + mCurrentSession = null; } }