From 086031e1a09b95c4f2f8f94fb7fa24fb67b77146 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Wed, 18 Jan 2023 04:59:09 +0000 Subject: [PATCH] Share process between SandboxedDetectionServices Bind HotwordDetectionService and SandboxedDetectionService in to a single isolated process instance and share AppOps and policy between the two services. Bug: 255597123 Test: manual - sample app can open camera with few wip CLs Change-Id: Ibb68532435cdd1fba724010b538ecf065c517b0c --- .../permission/PermissionManagerService.java | 8 ++-- .../android/server/policy/AppOpsPolicy.java | 3 +- .../HotwordDetectionConnection.java | 45 ++++++++++++------- 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index 8973adc9c07ef..a8369a8904ba9 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -16,6 +16,7 @@ package com.android.server.pm.permission; +import static android.Manifest.permission.CAMERA; import static android.Manifest.permission.CAPTURE_AUDIO_HOTWORD; import static android.Manifest.permission.CAPTURE_AUDIO_OUTPUT; import static android.Manifest.permission.RECORD_AUDIO; @@ -1377,14 +1378,15 @@ public class PermissionManagerService extends IPermissionManager.Stub { boolean permissionGranted = context.checkPermission(permission, /*pid*/ -1, uid) == PackageManager.PERMISSION_GRANTED; - // Override certain permissions checks for the HotwordDetectionService. This service is - // an isolated service, which ordinarily cannot hold permissions. + // Override certain permissions checks for the shared isolated process for both + // HotwordDetectionService and VisualQueryDetectionService, which ordinarily cannot hold + // any permissions. // There's probably a cleaner, more generalizable way to do this. For now, this is // the only use case for this, so simply override here. if (!permissionGranted && Process.isIsolated(uid) // simple check which fails-fast for the common case && (permission.equals(RECORD_AUDIO) || permission.equals(CAPTURE_AUDIO_HOTWORD) - || permission.equals(CAPTURE_AUDIO_OUTPUT))) { + || permission.equals(CAPTURE_AUDIO_OUTPUT) || permission.equals(CAMERA))) { HotwordDetectionServiceProvider hotwordServiceProvider = permissionManagerServiceInt.getHotwordDetectionServiceProvider(); permissionGranted = hotwordServiceProvider != null diff --git a/services/core/java/com/android/server/policy/AppOpsPolicy.java b/services/core/java/com/android/server/policy/AppOpsPolicy.java index 383249f41f397..401eac6e2d199 100644 --- a/services/core/java/com/android/server/policy/AppOpsPolicy.java +++ b/services/core/java/com/android/server/policy/AppOpsPolicy.java @@ -447,7 +447,8 @@ public final class AppOpsPolicy implements AppOpsManagerInternal.CheckOpsDelegat // package, so we don't need to modify it. if (Process.isIsolated(uid) // simple check which fails-fast for the common case && (code == AppOpsManager.OP_RECORD_AUDIO - || code == AppOpsManager.OP_RECORD_AUDIO_HOTWORD)) { + || code == AppOpsManager.OP_RECORD_AUDIO_HOTWORD + || code == AppOpsManager.OP_CAMERA)) { final HotwordDetectionServiceIdentity hotwordDetectionServiceIdentity = mVoiceInteractionManagerInternal.getHotwordDetectionServiceIdentity(); if (hotwordDetectionServiceIdentity != null diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java index 665d5e7604253..22cce1b4b5867 100644 --- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java +++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java @@ -108,7 +108,7 @@ final class HotwordDetectionConnection { final int mUser; final Context mContext; volatile HotwordDetectionServiceIdentity mIdentity; - //TODO: add similar identity for visual query service for the use of content capturing + //TODO: Consider rename this to SandboxedDetectionIdentity private Instant mLastRestartInstant; private ScheduledFuture mDebugHotwordLoggingTimeoutFuture = null; @@ -117,6 +117,7 @@ final class HotwordDetectionConnection { @GuardedBy("mLock") @Nullable private final Identity mVoiceInteractorIdentity; + private int mRestartCount = 0; @NonNull private ServiceConnection mRemoteHotwordDetectionService; @NonNull private ServiceConnection mRemoteVisualQueryDetectionService; private IBinder mAudioFlinger; @@ -234,10 +235,6 @@ final class HotwordDetectionConnection { mDebugHotwordLogging = false; unbindVisualQueryDetectionService(); unbindHotwordDetectionService(); - if (mIdentity != null) { - removeServiceUidForAudioPolicy(mIdentity.getIsolatedUid()); - } - mIdentity = null; if (mCancellationTaskFuture != null) { mCancellationTaskFuture.cancel(/* mayInterruptIfRunning= */ true); } @@ -246,18 +243,34 @@ final class HotwordDetectionConnection { } } + @SuppressWarnings("GuardedBy") private void unbindVisualQueryDetectionService() { if (mRemoteVisualQueryDetectionService != null) { mRemoteVisualQueryDetectionService.unbind(); - //TODO: Set visual query detection service provider to null + mRemoteVisualQueryDetectionService = null; } + resetDetectionProcessIdentityIfEmptyLocked(); } + @SuppressWarnings("GuardedBy") private void unbindHotwordDetectionService() { if (mRemoteHotwordDetectionService != null) { mRemoteHotwordDetectionService.unbind(); + mRemoteHotwordDetectionService = null; + } + resetDetectionProcessIdentityIfEmptyLocked(); + } + + // TODO(b/266669849): Clean up SuppressWarnings for calling methods. + @GuardedBy("mLock") + private void resetDetectionProcessIdentityIfEmptyLocked() { + if (mRemoteHotwordDetectionService == null && mRemoteVisualQueryDetectionService == null) { LocalServices.getService(PermissionManagerServiceInternal.class) .setHotwordDetectionServiceProvider(null); + if (mIdentity != null) { + removeServiceUidForAudioPolicy(mIdentity.getIsolatedUid()); + } + mIdentity = null; } } @@ -427,10 +440,10 @@ final class HotwordDetectionConnection { ServiceConnection oldHotwordConnection = mRemoteHotwordDetectionService; ServiceConnection oldVisualQueryDetectionConnection = mRemoteVisualQueryDetectionService; HotwordDetectionServiceIdentity previousIdentity = mIdentity; - //TODO: Add previousIdentity for visual query detection service mLastRestartInstant = Instant.now(); // Recreate connection to reset the cache. + mRestartCount++; mRemoteHotwordDetectionService = mHotwordDetectionServiceConnectionFactory.createLocked(); mRemoteVisualQueryDetectionService = @@ -458,6 +471,9 @@ final class HotwordDetectionConnection { oldVisualQueryDetectionConnection.unbind(); } + // TODO(b/266670431): Handles identity resetting for the new process to make sure the + // correct identity is provided. + if (previousIdentity != null) { removeServiceUidForAudioPolicy(previousIdentity.getIsolatedUid()); } @@ -531,8 +547,7 @@ final class HotwordDetectionConnection { && mRemoteHotwordDetectionService != null && mRemoteHotwordDetectionService.isBound()); pw.print(prefix); pw.print("mRestartCount="); - pw.println(mHotwordDetectionServiceConnectionFactory.mRestartCount); - pw.println(mVisualQueryDetectionServiceConnectionFactory.mRestartCount); + pw.println(mRestartCount); pw.print(prefix); pw.print("mLastRestartInstant="); pw.println(mLastRestartInstant); pw.print(prefix); pw.print("mDetectorType="); pw.println(HotwordDetector.detectorTypeToString(mDetectorType)); @@ -547,8 +562,6 @@ final class HotwordDetectionConnection { private final Intent mIntent; private final int mBindingFlags; - private int mRestartCount = 0; - ServiceConnectionFactory(@NonNull Intent intent, boolean bindInstantServiceAllowed) { mIntent = intent; mBindingFlags = bindInstantServiceAllowed ? Context.BIND_ALLOW_INSTANT : 0; @@ -558,7 +571,7 @@ final class HotwordDetectionConnection { ServiceConnection connection = new ServiceConnection(mContext, mIntent, mBindingFlags, mUser, ISandboxedDetectionService.Stub::asInterface, - mRestartCount++ % MAX_ISOLATED_PROCESS_NUMBER); + mRestartCount % MAX_ISOLATED_PROCESS_NUMBER); connection.connect(); updateAudioFlinger(connection, mAudioFlinger); @@ -660,13 +673,11 @@ final class HotwordDetectionConnection { HOTWORD_DETECTOR_EVENTS__EVENT__REQUEST_BIND_SERVICE, mVoiceInteractionServiceUid); } - String instancePrefix = - mIntent.getAction().equals(HotwordDetectionService.SERVICE_INTERFACE) - ? "hotword_detector_" : "visual_query_detector_"; boolean bindResult = mContext.bindIsolatedService( mIntent, - Context.BIND_AUTO_CREATE | Context.BIND_FOREGROUND_SERVICE | mBindingFlags, - instancePrefix + mInstanceNumber, + Context.BIND_SHARED_ISOLATED_PROCESS | Context.BIND_AUTO_CREATE + | Context.BIND_FOREGROUND_SERVICE | mBindingFlags, + "hotword_detector_" + mInstanceNumber, mExecutor, serviceConnection); if (!bindResult) {