diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java index fc22935736a35..965a07b51e123 100644 --- a/services/core/java/com/android/server/appop/AppOpsService.java +++ b/services/core/java/com/android/server/appop/AppOpsService.java @@ -157,10 +157,10 @@ import com.android.server.LockGuard; import com.android.server.SystemServerInitThreadPool; import com.android.server.SystemServiceManager; import com.android.server.pm.PackageList; +import com.android.server.pm.PackageManagerLocal; import com.android.server.pm.UserManagerInternal; import com.android.server.pm.pkg.AndroidPackage; import com.android.server.pm.pkg.PackageState; -import com.android.server.pm.pkg.PackageStateInternal; import com.android.server.pm.pkg.component.ParsedAttribution; import com.android.server.policy.AppOpsPolicy; @@ -367,6 +367,9 @@ public class AppOpsService extends IAppOpsService.Stub { /** Package Manager internal. Access via {@link #getPackageManagerInternal()} */ private @Nullable PackageManagerInternal mPackageManagerInternal; + /** Package Manager local. Access via {@link #getPackageManagerLocal()} */ + private @Nullable PackageManagerLocal mPackageManagerLocal; + /** User Manager internal. Access via {@link #getUserManagerInternal()} */ private @Nullable UserManagerInternal mUserManagerInternal; @@ -1189,42 +1192,64 @@ public class AppOpsService extends IAppOpsService.Stub { /** * Initialize uid state objects for state contained in the checking service. */ - private void initializeUidStates() { + @VisibleForTesting + void initializeUidStates() { UserManagerInternal umi = getUserManagerInternal(); - int[] userIds = umi.getUserIds(); synchronized (this) { - for (int i = 0; i < userIds.length; i++) { - int userId = userIds[i]; - initializeUserUidStatesLocked(userId); + int[] userIds = umi.getUserIds(); + try (PackageManagerLocal.UnfilteredSnapshot snapshot = + getPackageManagerLocal().withUnfilteredSnapshot()) { + Map packageStates = snapshot.getPackageStates(); + for (int i = 0; i < userIds.length; i++) { + int userId = userIds[i]; + initializeUserUidStatesLocked(userId, packageStates); + } } } } private void initializeUserUidStates(int userId) { synchronized (this) { - initializeUserUidStatesLocked(userId); + try (PackageManagerLocal.UnfilteredSnapshot snapshot = + getPackageManagerLocal().withUnfilteredSnapshot()) { + initializeUserUidStatesLocked(userId, snapshot.getPackageStates()); + } } } - private void initializeUserUidStatesLocked(int userId) { - ArrayMap packageStates = - getPackageManagerInternal().getPackageStates(); - for (int j = 0; j < packageStates.size(); j++) { - PackageStateInternal packageState = packageStates.valueAt(j); - int uid = UserHandle.getUid(userId, packageState.getAppId()); - UidState uidState = getUidStateLocked(uid, true); - String packageName = packageStates.keyAt(j); - Ops ops = new Ops(packageName, uidState); - uidState.pkgOps.put(packageName, ops); + private void initializeUserUidStatesLocked(int userId, Map packageStates) { + for (Map.Entry entry : packageStates.entrySet()) { + int appId = entry.getValue().getAppId(); + String packageName = entry.getKey(); - SparseIntArray packageModes = - mAppOpsCheckingService.getNonDefaultPackageModes(packageName, userId); - for (int k = 0; k < packageModes.size(); k++) { - int code = packageModes.get(k); + initializePackageUidStateLocked(userId, appId, packageName); + } + } + + /* + Be careful not to clear any existing data; only want to add objects that don't already exist. + */ + private void initializePackageUidStateLocked(int userId, int appId, String packageName) { + int uid = UserHandle.getUid(userId, appId); + UidState uidState = getUidStateLocked(uid, true); + Ops ops = uidState.pkgOps.get(packageName); + if (ops == null) { + ops = new Ops(packageName, uidState); + uidState.pkgOps.put(packageName, ops); + } + + SparseIntArray packageModes = + mAppOpsCheckingService.getNonDefaultPackageModes(packageName, userId); + for (int k = 0; k < packageModes.size(); k++) { + int code = packageModes.keyAt(k); + + if (ops.indexOfKey(code) < 0) { ops.put(code, new Op(uidState, packageName, code, uid)); } - uidState.evalForegroundOps(); } + + uidState.evalForegroundOps(); } /** @@ -3648,6 +3673,20 @@ public class AppOpsService extends IAppOpsService.Stub { return mPackageManagerInternal; } + /** + * @return {@link PackageManagerLocal} + */ + private @NonNull PackageManagerLocal getPackageManagerLocal() { + if (mPackageManagerLocal == null) { + mPackageManagerLocal = LocalManagerRegistry.getManager(PackageManagerLocal.class); + } + if (mPackageManagerLocal == null) { + throw new IllegalStateException("PackageManagerLocal not loaded"); + } + + return mPackageManagerLocal; + } + /** * @return {@link UserManagerInternal} */ diff --git a/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java index f86e4644d8b91..44ec26ea65e0c 100644 --- a/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java @@ -19,9 +19,11 @@ import static android.app.AppOpsManager.MODE_ALLOWED; import static android.app.AppOpsManager.MODE_ERRORED; import static android.app.AppOpsManager.OP_COARSE_LOCATION; import static android.app.AppOpsManager.OP_FLAGS_ALL; +import static android.app.AppOpsManager.OP_FLAG_SELF; import static android.app.AppOpsManager.OP_READ_SMS; import static android.app.AppOpsManager.OP_WIFI_SCAN; import static android.app.AppOpsManager.OP_WRITE_SMS; +import static android.os.UserHandle.getUserId; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing; import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; @@ -33,12 +35,15 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; +import android.app.AppOpsManager; import android.app.AppOpsManager.OpEntry; import android.app.AppOpsManager.PackageOps; import android.content.ContentResolver; @@ -48,14 +53,19 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.Process; import android.provider.Settings; +import android.util.ArrayMap; import androidx.test.InstrumentationRegistry; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; import com.android.dx.mockito.inline.extended.StaticMockitoSession; +import com.android.server.LocalManagerRegistry; import com.android.server.LocalServices; +import com.android.server.pm.PackageManagerLocal; +import com.android.server.pm.UserManagerInternal; import com.android.server.pm.pkg.AndroidPackage; +import com.android.server.pm.pkg.PackageState; import com.android.server.pm.pkg.PackageStateInternal; import org.junit.After; @@ -67,6 +77,7 @@ import org.mockito.quality.Strictness; import java.io.File; import java.util.Collections; import java.util.List; +import java.util.Map; /** * Unit tests for AppOpsService. Covers functionality that is difficult to test using CTS tests @@ -133,6 +144,7 @@ public class AppOpsServiceTest { mMockingSession = mockitoSession() .strictness(Strictness.LENIENT) .spyStatic(LocalServices.class) + .spyStatic(LocalManagerRegistry.class) .spyStatic(Settings.Global.class) .startMocking(); @@ -152,6 +164,23 @@ public class AppOpsServiceTest { doReturn(mockPackageManagerInternal).when( () -> LocalServices.getService(PackageManagerInternal.class)); + PackageManagerLocal mockPackageManagerLocal = mock(PackageManagerLocal.class); + PackageManagerLocal.UnfilteredSnapshot mockUnfilteredSnapshot = + mock(PackageManagerLocal.UnfilteredSnapshot.class); + PackageState mockMyPS = mock(PackageState.class); + ArrayMap packageStates = new ArrayMap<>(); + packageStates.put(sMyPackageName, mockMyPS); + when(mockMyPS.getAppId()).thenReturn(mMyUid); + when(mockUnfilteredSnapshot.getPackageStates()).thenReturn(packageStates); + when(mockPackageManagerLocal.withUnfilteredSnapshot()).thenReturn(mockUnfilteredSnapshot); + doReturn(mockPackageManagerLocal).when( + () -> LocalManagerRegistry.getManager(PackageManagerLocal.class)); + + UserManagerInternal mockUserManagerInternal = mock(UserManagerInternal.class); + when(mockUserManagerInternal.getUserIds()).thenReturn(new int[] {getUserId(mMyUid)}); + doReturn(mockUserManagerInternal).when( + () -> LocalServices.getService(UserManagerInternal.class)); + // Mock behavior to use specific Settings.Global.APPOP_HISTORY_PARAMETERS doReturn(null).when(() -> Settings.Global.getString(any(ContentResolver.class), eq(Settings.Global.APPOP_HISTORY_PARAMETERS))); @@ -337,6 +366,25 @@ public class AppOpsServiceTest { assertThat(getLoggedOps()).isNull(); } + @Test + public void testUidStateInitializationDoesntClearState() throws InterruptedException { + mAppOpsService.setMode(OP_READ_SMS, mMyUid, sMyPackageName, MODE_ALLOWED); + mAppOpsService.noteOperation(OP_READ_SMS, mMyUid, sMyPackageName, null, false, null, false); + mAppOpsService.initializeUidStates(); + List ops = mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName, + new int[]{OP_READ_SMS}); + assertNotNull(ops); + for (int i = 0; i < ops.size(); i++) { + List opEntries = ops.get(i).getOps(); + for (int j = 0; j < opEntries.size(); j++) { + Map attributedOpEntries = opEntries.get( + j).getAttributedOpEntries(); + assertNotEquals(-1, attributedOpEntries.get(null) + .getLastAccessTime(OP_FLAG_SELF)); + } + } + } + private List getLoggedOps() { return mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName, null /* all ops */); }