Add audioserver permission caching

Audioserver upcalls to system_server for permission checks.
This causes deadlocks due to binder threadpool starvation.

Add permission caching functionality to AudioServerPermissionProvider.

We utilize the PermissionManager cache invalidation scheme, which
updates a sysprop whenever permissions might have changed. We maintain a
list of uids for each permission that audioserver is interested in, and
whenever the sysprop is updated, we check if the set of uids which hold
each perm is updated. If so, we push the new set of uids holding the
perm to system server. Ideally we wouldn't iterate over perms cross
uids, but there isn't a more efficient way to aggregate the info. We use
the existing app-id cache to avoid expensive package manager calls.

We should limit the number of permissions that audioserver checks, and
route calls through system server whenever possible, for performance
reasons.

Bug: 338089555
Test: atest AudioServerPermissionProviderTest
Flag: com.android.media.audio.audioserver_permissions

Change-Id: I8b3732a4b15b94cf90e03bb1604c126973288edf

services/core/java/com/android/server/audio/AudioServerPermissionProvider.java

@@ -16,40 +16,76 @@
 
 package com.android.server.audio;
 
+import static android.Manifest.permission.CALL_AUDIO_INTERCEPTION;
+import static android.Manifest.permission.MODIFY_AUDIO_ROUTING;
+import static android.Manifest.permission.MODIFY_PHONE_STATE;
+import static android.Manifest.permission.RECORD_AUDIO;
+
 import android.annotation.Nullable;
 import android.os.RemoteException;
 import android.os.UserHandle;
 import android.util.ArraySet;
+import android.util.IntArray;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.media.permission.INativePermissionController;
+import com.android.media.permission.PermissionEnum;
 import com.android.media.permission.UidPackageState;
 import com.android.server.pm.pkg.PackageState;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.function.BiPredicate;
+import java.util.function.Supplier;
 import java.util.stream.Collector;
 import java.util.stream.Collectors;
 
 /** Responsible for synchronizing system server permission state to the native audioserver. */
 public class AudioServerPermissionProvider {
 
+    static final String[] MONITORED_PERMS = new String[PermissionEnum.ENUM_SIZE];
+
+    static {
+        MONITORED_PERMS[PermissionEnum.MODIFY_AUDIO_ROUTING] = MODIFY_AUDIO_ROUTING;
+        MONITORED_PERMS[PermissionEnum.MODIFY_PHONE_STATE] = MODIFY_PHONE_STATE;
+        MONITORED_PERMS[PermissionEnum.RECORD_AUDIO] = RECORD_AUDIO;
+        MONITORED_PERMS[PermissionEnum.CALL_AUDIO_INTERCEPTION] = CALL_AUDIO_INTERCEPTION;
+    }
+
     private final Object mLock = new Object();
+    private final Supplier<int[]> mUserIdSupplier;
+    private final BiPredicate<Integer, String> mPermissionPredicate;
 
     @GuardedBy("mLock")
     private INativePermissionController mDest;
 
     @GuardedBy("mLock")
     private final Map<Integer, Set<String>> mPackageMap;
+    // Values are sorted
+    @GuardedBy("mLock")
+    private final int[][] mPermMap = new int[PermissionEnum.ENUM_SIZE][];
+
+    @GuardedBy("mLock")
+    private boolean mIsUpdateDeferred = true;
 
     /**
      * @param appInfos - PackageState for all apps on the device, used to populate init state
      */
-    public AudioServerPermissionProvider(Collection<PackageState> appInfos) {
+    public AudioServerPermissionProvider(
+            Collection<PackageState> appInfos,
+            BiPredicate<Integer, String> permissionPredicate,
+            Supplier<int[]> userIdSupplier) {
+        for (int i = 0; i < PermissionEnum.ENUM_SIZE; i++) {
+            Objects.requireNonNull(MONITORED_PERMS[i]);
+        }
+        mUserIdSupplier = userIdSupplier;
+        mPermissionPredicate = permissionPredicate;
         // Initialize the package state
         mPackageMap = generatePackageMappings(appInfos);
     }
@@ -64,6 +100,18 @@ public class AudioServerPermissionProvider {
         synchronized (mLock) {
             mDest = pc;
             resetNativePackageState();
+            try {
+                for (byte i = 0; i < PermissionEnum.ENUM_SIZE; i++) {
+                    if (mIsUpdateDeferred) {
+                        mPermMap[i] = getUidsHoldingPerm(MONITORED_PERMS[i]);
+                    }
+                    mDest.populatePermissionState(i, mPermMap[i]);
+                }
+                mIsUpdateDeferred = false;
+            } catch (RemoteException e) {
+                // We will re-init the state when the service comes back up
+                mDest = null;
+            }
         }
     }
 
@@ -106,6 +154,30 @@ public class AudioServerPermissionProvider {
         }
     }
 
+    /** Called whenever any package/permission changes occur which invalidate uids holding perms */
+    public void onPermissionStateChanged() {
+        synchronized (mLock) {
+            if (mDest == null) {
+                mIsUpdateDeferred = true;
+                return;
+            }
+            try {
+                for (byte i = 0; i < PermissionEnum.ENUM_SIZE; i++) {
+                    var newPerms = getUidsHoldingPerm(MONITORED_PERMS[i]);
+                    if (!Arrays.equals(newPerms, mPermMap[i])) {
+                        mPermMap[i] = newPerms;
+                        mDest.populatePermissionState(i, newPerms);
+                    }
+                }
+            } catch (RemoteException e) {
+                // We will re-init the state when the service comes back up
+                mDest = null;
+                // We didn't necessarily finish
+                mIsUpdateDeferred = true;
+            }
+        }
+    }
+
     /** Called when full syncing package state to audioserver. */
     @GuardedBy("mLock")
     private void resetNativePackageState() {
@@ -128,6 +200,23 @@ public class AudioServerPermissionProvider {
         }
     }
 
+    @GuardedBy("mLock")
+    /** Return all uids (not app-ids) which currently hold a given permission. Not app-op aware */
+    private int[] getUidsHoldingPerm(String perm) {
+        IntArray acc = new IntArray();
+        for (int userId : mUserIdSupplier.get()) {
+            for (int appId : mPackageMap.keySet()) {
+                int uid = UserHandle.getUid(userId, appId);
+                if (mPermissionPredicate.test(uid, perm)) {
+                    acc.add(uid);
+                }
+            }
+        }
+        var unwrapped = acc.toArray();
+        Arrays.sort(unwrapped);
+        return unwrapped;
+    }
+
     /**
      * Aggregation operation on all package states list: groups by states by app-id and merges the
      * packageName for each state into an ArraySet.

services/core/java/com/android/server/audio/AudioService.java

@@ -204,6 +204,7 @@ import android.os.VibrationAttributes;
 import android.os.VibrationEffect;
 import android.os.Vibrator;
 import android.os.VibratorManager;
+import android.permission.PermissionManager;
 import android.provider.Settings;
 import android.provider.Settings.System;
 import android.service.notification.ZenModeConfig;
@@ -241,6 +242,7 @@ import com.android.server.pm.PackageManagerLocal;
 import com.android.server.pm.UserManagerInternal;
 import com.android.server.pm.UserManagerInternal.UserRestrictionsListener;
 import com.android.server.pm.UserManagerService;
+import com.android.server.pm.permission.PermissionManagerServiceInternal;
 import com.android.server.pm.pkg.PackageState;
 import com.android.server.utils.EventLogger;
 import com.android.server.wm.ActivityTaskManagerInternal;
@@ -269,6 +271,8 @@ import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BooleanSupplier;
 import java.util.stream.Collectors;
@@ -11878,12 +11882,40 @@ public class AudioService extends IAudioService.Stub
                         .withUnfilteredSnapshot()) {
             packageStates = snapshot.getPackageStates().values();
         }
-        var provider = new AudioServerPermissionProvider(packageStates);
+        var umi = LocalServices.getService(UserManagerInternal.class);
+        var pmsi = LocalServices.getService(PermissionManagerServiceInternal.class);
+        var provider = new AudioServerPermissionProvider(packageStates,
+                (Integer uid, String perm) -> (pmsi.checkUidPermission(uid, perm,
+                        Context.DEVICE_ID_DEFAULT) == PackageManager.PERMISSION_GRANTED),
+                () -> umi.getUserIds()
+                );
         audioPolicy.registerOnStartTask(() -> {
             provider.onServiceStart(audioPolicy.getPermissionController());
         });
         // Set up event listeners
+        // Must be kept in sync with PermissionManager
+        Runnable cacheSysPropHandler = new Runnable() {
+            private AtomicReference<SystemProperties.Handle> mHandle = new AtomicReference();
+            private AtomicLong mNonce = new AtomicLong();
+            @Override
+            public void run() {
+                if (mHandle.get() == null) {
+                    // Cache the handle
+                    mHandle.compareAndSet(null, SystemProperties.find(
+                            PermissionManager.CACHE_KEY_PACKAGE_INFO));
+                }
+                long nonce;
+                SystemProperties.Handle ref;
+                if ((ref = mHandle.get()) != null && (nonce = ref.getLong(0)) != 0 &&
+                        mNonce.getAndSet(nonce) != nonce) {
+                    audioserverExecutor.execute(() -> provider.onPermissionStateChanged());
+                }
+            }
+        };
+
+        SystemProperties.addChangeCallback(cacheSysPropHandler);
+
         IntentFilter packageUpdateFilter = new IntentFilter();
         packageUpdateFilter.addAction(ACTION_PACKAGE_ADDED);
         packageUpdateFilter.addAction(ACTION_PACKAGE_REMOVED);

services/tests/servicestests/src/com/android/server/audio/AudioServerPermissionProviderTest.java

@@ -15,15 +15,23 @@
  */
 package com.android.server.audio;
 
+import static com.android.server.audio.AudioServerPermissionProvider.MONITORED_PERMS;
+
+import static org.mockito.AdditionalMatchers.aryEq;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyByte;
 import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import android.os.RemoteException;
 import android.platform.test.annotations.Presubmit;
 
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -45,6 +53,8 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.function.BiPredicate;
+import java.util.function.Supplier;
 
 @RunWith(AndroidJUnit4.class)
 @Presubmit
@@ -64,6 +74,9 @@ public final class AudioServerPermissionProviderTest {
     @Mock public PackageState mMockPackageStateFive_10001_four;
     @Mock public PackageState mMockPackageStateSix_10000_two;
 
+    @Mock public BiPredicate<Integer, String> mMockPermPred;
+    @Mock public Supplier<int[]> mMockUserIdSupplier;
+
     public List<UidPackageState> mInitPackageListExpected;
 
     // Argument matcher which matches that the state is equal even if the package names are out of
@@ -148,6 +161,13 @@ public final class AudioServerPermissionProviderTest {
 
         when(mMockPackageStateSix_10000_two.getAppId()).thenReturn(10000);
         when(mMockPackageStateSix_10000_two.getPackageName()).thenReturn("com.package.two");
+
+        when(mMockUserIdSupplier.get()).thenReturn(new int[] {0, 1});
+
+        when(mMockPermPred.test(eq(10000), eq(MONITORED_PERMS[0]))).thenReturn(true);
+        when(mMockPermPred.test(eq(110001), eq(MONITORED_PERMS[0]))).thenReturn(true);
+        when(mMockPermPred.test(eq(10001), eq(MONITORED_PERMS[1]))).thenReturn(true);
+        when(mMockPermPred.test(eq(110000), eq(MONITORED_PERMS[1]))).thenReturn(true);
     }
 
     @Test
@@ -168,7 +188,9 @@ public final class AudioServerPermissionProviderTest {
                         createUidPackageState(
                                 10001, List.of("com.package.two", "com.package.four")));
 
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
         verify(mMockPc)
                 .populatePackagesForUids(argThat(new PackageStateListMatcher(expectedPackageList)));
@@ -179,7 +201,9 @@ public final class AudioServerPermissionProviderTest {
         // 10000: one | 10001: two
         var initPackageListData =
                 List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
 
         // new uid, including user component
@@ -196,7 +220,9 @@ public final class AudioServerPermissionProviderTest {
         // 10000: one | 10001: two
         var initPackageListData =
                 List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
 
         // Includes user-id
@@ -211,7 +237,9 @@ public final class AudioServerPermissionProviderTest {
         // 10000: one | 10001: two
         var initPackageListData =
                 List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
 
         // Includes user-id
@@ -233,7 +261,9 @@ public final class AudioServerPermissionProviderTest {
                         mMockPackageStateOne_10000_one,
                         mMockPackageStateTwo_10001_two,
                         mMockPackageStateSix_10000_two);
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
 
         // Includes user-id
@@ -255,7 +285,9 @@ public final class AudioServerPermissionProviderTest {
                         mMockPackageStateOne_10000_one,
                         mMockPackageStateTwo_10001_two,
                         mMockPackageStateSix_10000_two);
-        mPermissionProvider = new AudioServerPermissionProvider(initPackageListData);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
         mPermissionProvider.onServiceStart(mMockPc);
         mPermissionProvider.onModifyPackageState(1_10000, "com.package.one", true /* isRemove */);
         verify(mMockPc)
@@ -299,6 +331,72 @@ public final class AudioServerPermissionProviderTest {
         verify(newMockPc).updatePackagesForUid(any());
     }
 
+    @Test
+    public void testPermissionsPopulated_onStart() throws Exception {
+        // expected state from setUp:
+        // PERM[0]: [10000, 110001]
+        // PERM[1]: [10001, 110000]
+        // PERM[...]: []
+        var initPackageListData =
+                List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
+
+        mPermissionProvider.onServiceStart(mMockPc);
+        verify(mMockPc).populatePermissionState(eq((byte) 0), aryEq(new int[] {10000, 110001}));
+        verify(mMockPc).populatePermissionState(eq((byte) 1), aryEq(new int[] {10001, 110000}));
+        for (int i = 2; i < MONITORED_PERMS.length; i++) {
+            verify(mMockPc).populatePermissionState(eq((byte) i), aryEq(new int[] {}));
+        }
+        verify(mMockPc, times(MONITORED_PERMS.length)).populatePermissionState(anyByte(), any());
+    }
+
+    @Test
+    public void testPermissionsPopulated_onChange() throws Exception {
+        var initPackageListData =
+                List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
+
+        mPermissionProvider.onServiceStart(mMockPc);
+        clearInvocations(mMockPc);
+        // Ensure the provided permission state is changed
+        when(mMockPermPred.test(eq(110001), eq(MONITORED_PERMS[1]))).thenReturn(true);
+
+        mPermissionProvider.onPermissionStateChanged();
+        verify(mMockPc)
+                .populatePermissionState(eq((byte) 1), aryEq(new int[] {10001, 110000, 110001}));
+        verify(mMockPc).populatePermissionState(anyByte(), any()); // exactly once
+    }
+
+    @Test
+    public void testPermissionPopulatedDeferred_onDeadService() throws Exception {
+        var initPackageListData =
+                List.of(mMockPackageStateOne_10000_one, mMockPackageStateTwo_10001_two);
+        mPermissionProvider =
+                new AudioServerPermissionProvider(
+                        initPackageListData, mMockPermPred, mMockUserIdSupplier);
+
+        // throw on the first call to mark the service as dead
+        doThrow(new RemoteException())
+                .doNothing()
+                .when(mMockPc)
+                .populatePermissionState(anyByte(), any());
+        mPermissionProvider.onServiceStart(mMockPc);
+        clearInvocations(mMockPc);
+        clearInvocations(mMockPermPred);
+
+        mPermissionProvider.onPermissionStateChanged();
+        verify(mMockPermPred, never()).test(any(), any());
+        verify(mMockPc, never()).populatePermissionState(anyByte(), any());
+        mPermissionProvider.onServiceStart(mMockPc);
+        for (int i = 0; i < MONITORED_PERMS.length; i++) {
+            verify(mMockPc).populatePermissionState(eq((byte) i), any());
+        }
+    }
+
     private static UidPackageState createUidPackageState(int uid, List<String> packages) {
         var res = new UidPackageState();
         res.uid = uid;