From 1e03bc0539308e9b99da1e738d437f196a5e9074 Mon Sep 17 00:00:00 2001 From: Nicholas Ambur Date: Tue, 10 Mar 2020 13:17:17 -0700 Subject: [PATCH 1/2] Crash service on SoundTriger HAL fatal error Throwing an exception is not enough in this case. When the HAL behaves unexpectedly, the system service and the HAL must be reset and the client must be notified. Without a full reset in this catastrophic case, the state of the HAL and the system service cannot be guaranteed to the client. Bug: 150569186 Test: Intentionally load an incompatible voice model and confirm the device can recover. Change-Id: Ia7c3f25e48f04bf32a96c64ec998fdfa52459685 --- .../voice/AlwaysOnHotwordDetector.java | 15 ++++--- .../voice/VoiceInteractionService.java | 43 +++++++++++++++---- .../SoundTriggerMiddlewareValidation.java | 9 ++++ 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java index 7d070b1d9df67..f8265d6366ab1 100644 --- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java +++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java @@ -780,15 +780,16 @@ public class AlwaysOnHotwordDetector { audioCapabilities |= AUDIO_CAPABILITY_NOISE_SUPPRESSION; } - int code = STATUS_ERROR; + int code; try { code = mModelManagementService.startRecognition( mKeyphraseMetadata.id, mLocale.toLanguageTag(), mInternalCallback, new RecognitionConfig(captureTriggerAudio, allowMultipleTriggers, recognitionExtra, null /* additional data */, audioCapabilities)); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in startRecognition!", e); + throw e.rethrowFromSystemServer(); } + if (code != STATUS_OK) { Slog.w(TAG, "startRecognition() failed with error code " + code); } @@ -796,12 +797,12 @@ public class AlwaysOnHotwordDetector { } private int stopRecognitionLocked() { - int code = STATUS_ERROR; + int code; try { code = mModelManagementService.stopRecognition(mKeyphraseMetadata.id, mInternalCallback); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in stopRecognition!", e); + throw e.rethrowFromSystemServer(); } if (code != STATUS_OK) { @@ -968,12 +969,12 @@ public class AlwaysOnHotwordDetector { } } - ModuleProperties dspModuleProperties = null; + ModuleProperties dspModuleProperties; try { dspModuleProperties = mModelManagementService.getDspModuleProperties(); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in getDspProperties!", e); + throw e.rethrowFromSystemServer(); } // No DSP available @@ -989,7 +990,7 @@ public class AlwaysOnHotwordDetector { mKeyphraseMetadata = mModelManagementService.getEnrolledKeyphraseMetadata( mText, mLocale.toLanguageTag()); } catch (RemoteException e) { - Slog.w(TAG, "RemoteException in internalUpdateEnrolledKeyphraseMetadata", e); + throw e.rethrowFromSystemServer(); } } } diff --git a/core/java/android/service/voice/VoiceInteractionService.java b/core/java/android/service/voice/VoiceInteractionService.java index b54e4d9b876d1..45d3465fdae8a 100644 --- a/core/java/android/service/voice/VoiceInteractionService.java +++ b/core/java/android/service/voice/VoiceInteractionService.java @@ -35,6 +35,7 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.provider.Settings; import android.util.ArraySet; +import android.util.Log; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.IVoiceActionCheckCallback; @@ -47,6 +48,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Objects; import java.util.Set; /** @@ -63,6 +65,8 @@ import java.util.Set; * separate process from this one. */ public class VoiceInteractionService extends Service { + static final String TAG = VoiceInteractionService.class.getSimpleName(); + /** * The {@link Intent} that must be declared as handled by the service. * To be supported, the service must also require the @@ -240,9 +244,22 @@ public class VoiceInteractionService extends Service { public void onReady() { mSystemService = IVoiceInteractionManagerService.Stub.asInterface( ServiceManager.getService(Context.VOICE_INTERACTION_MANAGER_SERVICE)); + Objects.requireNonNull(mSystemService); + try { + mSystemService.asBinder().linkToDeath(mDeathRecipient, 0); + } catch (RemoteException e) { + Log.wtf(TAG, "unable to link to death with system service"); + } mKeyphraseEnrollmentInfo = new KeyphraseEnrollmentInfo(getPackageManager()); } + private IBinder.DeathRecipient mDeathRecipient = () -> { + Log.e(TAG, "system service binder died shutting down"); + Handler.getMain().executeOrSendMessage(PooledLambda.obtainMessage( + VoiceInteractionService::onShutdownInternal, VoiceInteractionService.this)); + }; + + private void onShutdownInternal() { onShutdown(); // Stop any active recognitions when shutting down. @@ -349,16 +366,24 @@ public class VoiceInteractionService extends Service { } private void safelyShutdownHotwordDetector() { - try { - synchronized (mLock) { - if (mHotwordDetector != null) { - mHotwordDetector.stopRecognition(); - mHotwordDetector.invalidate(); - mHotwordDetector = null; - } + synchronized (mLock) { + if (mHotwordDetector == null) { + return; } - } catch (Exception ex) { - // Ignore. + + try { + mHotwordDetector.stopRecognition(); + } catch (Exception ex) { + // Ignore. + } + + try { + mHotwordDetector.invalidate(); + } catch (Exception ex) { + // Ignore. + } + + mHotwordDetector = null; } } diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java index 63048f6b95b31..4834ca85159ee 100644 --- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java +++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerMiddlewareValidation.java @@ -34,6 +34,7 @@ import android.media.soundtrigger_middleware.SoundModel; import android.media.soundtrigger_middleware.SoundTriggerModuleDescriptor; import android.media.soundtrigger_middleware.Status; import android.os.IBinder; +import android.os.Process; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.util.Log; @@ -139,6 +140,14 @@ public class SoundTriggerMiddlewareValidation implements ISoundTriggerMiddleware throw new ServiceSpecificException(((RecoverableException) e).errorCode, e.getMessage()); } + + /* Throwing an exception is not enough in this case. When the HAL behaves unexpectedly, the + system service and the HAL must be reset and the client must be notified. Without a full + reset in this catastrophic case, the state of the HAL and the system service cannot be + guaranteed to the client. + */ + Log.wtf(TAG, "Crashing system server due to unrecoverable exception", e); + Process.killProcess(Process.myPid()); throw new InternalServerError(e); } From 78a22682fbd60d1249d1622a22a2ee24ba38ee79 Mon Sep 17 00:00:00 2001 From: Nicholas Ambur Date: Mon, 23 Mar 2020 15:42:21 -0700 Subject: [PATCH 2/2] disable SoundTriggerService in safe mode Bug: 150569186 Test: confirm `dumpsys voiceinteraction` shows no active service && confirm `dumpsys soundtrigger_middleware` shows no API calls. Change-Id: I6b5879c662826e86a6f41f3c3aea444f0674f24c --- .../android/server/soundtrigger/SoundTriggerService.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java index 3196758996be7..00cb6dc7400dd 100644 --- a/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java +++ b/services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java @@ -207,7 +207,13 @@ public class SoundTriggerService extends SystemService { @Override public void onBootPhase(int phase) { - if (PHASE_SYSTEM_SERVICES_READY == phase) { + Slog.d(TAG, "onBootPhase: " + phase + " : " + isSafeMode()); + if (PHASE_DEVICE_SPECIFIC_SERVICES_READY == phase) { + if (isSafeMode()) { + Slog.w(TAG, "not enabling SoundTriggerService in safe mode"); + return; + } + initSoundTriggerHelper(); mLocalSoundTriggerService.setSoundTriggerHelper(mSoundTriggerHelper); } else if (PHASE_THIRD_PARTY_APPS_CAN_START == phase) {