From ec669ddecd25bf6c2f36c1d63fd662892edd9da0 Mon Sep 17 00:00:00 2001 From: lpeter Date: Thu, 6 Oct 2022 07:26:30 +0000 Subject: [PATCH] Refactor updateState method for trusted hotword Test: atest CtsVoiceInteractionTestCases Bug: 193232191 Change-Id: I781f9595e9c5016ba81e478e7f550d805699f7fb --- .../voice/AbstractHotwordDetector.java | 17 +++++++---- .../voice/AlwaysOnHotwordDetector.java | 4 +-- .../voice/SoftwareHotwordDetector.java | 4 +-- .../app/IVoiceInteractionManagerService.aidl | 19 ++++++++++++- .../VoiceInteractionManagerService.java | 28 +++++++++++-------- .../VoiceInteractionManagerServiceImpl.java | 23 +++++++++++++-- 6 files changed, 67 insertions(+), 28 deletions(-) diff --git a/core/java/android/service/voice/AbstractHotwordDetector.java b/core/java/android/service/voice/AbstractHotwordDetector.java index a2ca5a3e2f358..5d3852b8e81e6 100644 --- a/core/java/android/service/voice/AbstractHotwordDetector.java +++ b/core/java/android/service/voice/AbstractHotwordDetector.java @@ -126,21 +126,26 @@ abstract class AbstractHotwordDetector implements HotwordDetector { Slog.d(TAG, "updateState()"); } throwIfDetectorIsNoLongerActive(); - synchronized (mLock) { - updateStateLocked(options, sharedMemory, null /* callback */, mDetectorType); + try { + mManagerService.updateState(options, sharedMemory); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } } - protected void updateStateLocked(@Nullable PersistableBundle options, - @Nullable SharedMemory sharedMemory, IHotwordRecognitionStatusCallback callback, + protected void initAndVerifyDetector( + @Nullable PersistableBundle options, + @Nullable SharedMemory sharedMemory, + @NonNull IHotwordRecognitionStatusCallback callback, int detectorType) { if (DEBUG) { - Slog.d(TAG, "updateStateLocked()"); + Slog.d(TAG, "initAndVerifyDetector()"); } Identity identity = new Identity(); identity.packageName = ActivityThread.currentOpPackageName(); try { - mManagerService.updateState(identity, options, sharedMemory, callback, detectorType); + mManagerService.initAndVerifyDetector(identity, options, sharedMemory, callback, + detectorType); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } diff --git a/core/java/android/service/voice/AlwaysOnHotwordDetector.java b/core/java/android/service/voice/AlwaysOnHotwordDetector.java index d01e7feba36e0..d58f6d35981f6 100644 --- a/core/java/android/service/voice/AlwaysOnHotwordDetector.java +++ b/core/java/android/service/voice/AlwaysOnHotwordDetector.java @@ -817,10 +817,8 @@ public class AlwaysOnHotwordDetector extends AbstractHotwordDetector { @Override void initialize(@Nullable PersistableBundle options, @Nullable SharedMemory sharedMemory) { - // TODO: transition to use an API that is not updateState to provide - // onHotwordDetectionServiceInitialized status to external callback if (mSupportHotwordDetectionService) { - updateStateLocked(options, sharedMemory, mInternalCallback, + initAndVerifyDetector(options, sharedMemory, mInternalCallback, DETECTOR_TYPE_TRUSTED_HOTWORD_DSP); } try { diff --git a/core/java/android/service/voice/SoftwareHotwordDetector.java b/core/java/android/service/voice/SoftwareHotwordDetector.java index 02561c9410e32..11688df2f1a46 100644 --- a/core/java/android/service/voice/SoftwareHotwordDetector.java +++ b/core/java/android/service/voice/SoftwareHotwordDetector.java @@ -69,9 +69,7 @@ class SoftwareHotwordDetector extends AbstractHotwordDetector { @Override void initialize(@Nullable PersistableBundle options, @Nullable SharedMemory sharedMemory) { - // TODO: transition to use an API that is not updateState to provide - // onHotwordDetectionServiceInitialized status to external callback - updateStateLocked(options, sharedMemory, + initAndVerifyDetector(options, sharedMemory, new InitializationStateListener(mHandler, mCallback), DETECTOR_TYPE_TRUSTED_HOTWORD_SOFTWARE); } diff --git a/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl b/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl index bbcf982583ceb..9f23f2474257e 100644 --- a/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl +++ b/core/java/com/android/internal/app/IVoiceInteractionManagerService.aidl @@ -245,6 +245,23 @@ interface IVoiceInteractionManagerService { /** * Set configuration and pass read-only data to hotword detection service. + * + * @param options Application configuration data to provide to the + * {@link HotwordDetectionService}. PersistableBundle does not allow any remotable objects or + * other contents that can be used to communicate with other processes. + * @param sharedMemory The unrestricted data blob to provide to the + * {@link HotwordDetectionService}. Use this to provide the hotword models data or other + * such data to the trusted process. + */ + @EnforcePermission("MANAGE_HOTWORD_DETECTION") + void updateState( + in PersistableBundle options, + in SharedMemory sharedMemory); + + /** + * Set configuration and pass read-only data to hotword detection service when creating + * the detector. + * * Caller must provide an identity, used for permission tracking purposes. * The uid/pid elements of the identity will be ignored by the server and replaced with the ones * provided by binder. @@ -259,7 +276,7 @@ interface IVoiceInteractionManagerService { * @param detectorType Indicate which detector is used. */ @EnforcePermission("MANAGE_HOTWORD_DETECTION") - void updateState( + void initAndVerifyDetector( in Identity originatorIdentity, in PersistableBundle options, in SharedMemory sharedMemory, diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java index 79546b800c2b9..151ff80adce85 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerService.java @@ -1242,6 +1242,19 @@ public class VoiceInteractionManagerService extends SystemService { @android.annotation.EnforcePermission(android.Manifest.permission.MANAGE_HOTWORD_DETECTION) @Override public void updateState( + @Nullable PersistableBundle options, + @Nullable SharedMemory sharedMemory) { + synchronized (this) { + enforceIsCurrentVoiceInteractionService(); + + Binder.withCleanCallingIdentity( + () -> mImpl.updateStateLocked(options, sharedMemory)); + } + } + + @android.annotation.EnforcePermission(android.Manifest.permission.MANAGE_HOTWORD_DETECTION) + @Override + public void initAndVerifyDetector( @NonNull Identity voiceInteractorIdentity, @Nullable PersistableBundle options, @Nullable SharedMemory sharedMemory, @@ -1250,21 +1263,12 @@ public class VoiceInteractionManagerService extends SystemService { synchronized (this) { enforceIsCurrentVoiceInteractionService(); - if (mImpl == null) { - Slog.w(TAG, "updateState without running voice interaction service"); - return; - } - voiceInteractorIdentity.uid = Binder.getCallingUid(); voiceInteractorIdentity.pid = Binder.getCallingPid(); - final long caller = Binder.clearCallingIdentity(); - try { - mImpl.updateStateLocked( - voiceInteractorIdentity, options, sharedMemory, callback, detectorType); - } finally { - Binder.restoreCallingIdentity(caller); - } + Binder.withCleanCallingIdentity( + () -> mImpl.initAndVerifyDetectorLocked(voiceInteractorIdentity, options, + sharedMemory, callback, detectorType)); } } diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java index dcf7b787c61a3..5d1901d96bcd8 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/VoiceInteractionManagerServiceImpl.java @@ -551,12 +551,31 @@ class VoiceInteractionManagerServiceImpl implements VoiceInteractionSessionConne } public void updateStateLocked( + @Nullable PersistableBundle options, + @Nullable SharedMemory sharedMemory) { + Slog.v(TAG, "updateStateLocked"); + + if (sharedMemory != null && !sharedMemory.setProtect(OsConstants.PROT_READ)) { + Slog.w(TAG, "Can't set sharedMemory to be read-only"); + throw new IllegalStateException("Can't set sharedMemory to be read-only"); + } + + if (mHotwordDetectionConnection == null) { + Slog.w(TAG, "update State, but no hotword detection connection"); + throw new IllegalStateException("Hotword detection connection not found"); + } + synchronized (mHotwordDetectionConnection.mLock) { + mHotwordDetectionConnection.updateStateLocked(options, sharedMemory); + } + } + + public void initAndVerifyDetectorLocked( @NonNull Identity voiceInteractorIdentity, @Nullable PersistableBundle options, @Nullable SharedMemory sharedMemory, IHotwordRecognitionStatusCallback callback, int detectorType) { - Slog.v(TAG, "updateStateLocked"); + Slog.v(TAG, "initAndVerifyDetectorLocked"); int voiceInteractionServiceUid = mInfo.getServiceInfo().applicationInfo.uid; if (mHotwordDetectionComponentName == null) { Slog.w(TAG, "Hotword detection service name not found"); @@ -614,8 +633,6 @@ class VoiceInteractionManagerServiceImpl implements VoiceInteractionSessionConne mInfo.getServiceInfo().applicationInfo.uid, voiceInteractorIdentity, mHotwordDetectionComponentName, mUser, /* bindInstantServiceAllowed= */ false, options, sharedMemory, callback, detectorType); - } else { - mHotwordDetectionConnection.updateStateLocked(options, sharedMemory); } }