Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit c3c67f1c authored by Evan Severson's avatar Evan Severson
Browse files

Only add packageOps if they don't already exist in systemReady 

We were overwriting them when we try to initialize for all remaining
packages that didn't have state from the persistence read. Also we need
to be sure to add Op objects for codes that weren't added during the
earlier recent_accesses round.

Test: Boot and dumpsys
      Verify new testcase with and without fix
      Verify persisted files in upgrade cases and normal boot cases
Fixes: 273813938
Bug: 272810483
Change-Id: Ibf814b9a3318420d49a77c35f0bab7e6763f8953
parent 13f25d78

services/core/java/com/android/server/appop/AppOpsService.java

+60 −21
Original line number Diff line number Diff line
@@ -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) {

            int[] userIds = umi.getUserIds();

            try (PackageManagerLocal.UnfilteredSnapshot snapshot =

                         getPackageManagerLocal().withUnfilteredSnapshot()) {

                Map<String, PackageState> packageStates = snapshot.getPackageStates();

                for (int i = 0; i < userIds.length; i++) {

                    int userId = userIds[i];

                initializeUserUidStatesLocked(userId);

                    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, Map<String,

            PackageState> packageStates) {

        for (Map.Entry<String, PackageState> entry : packageStates.entrySet()) {

            int appId = entry.getValue().getAppId();

            String packageName = entry.getKey();



            initializePackageUidStateLocked(userId, appId, packageName);

        }

    }



    private void initializeUserUidStatesLocked(int userId) {

        ArrayMap<String, ? extends PackageStateInternal> packageStates =

                getPackageManagerInternal().getPackageStates();

        for (int j = 0; j < packageStates.size(); j++) {

            PackageStateInternal packageState = packageStates.valueAt(j);

            int uid = UserHandle.getUid(userId, packageState.getAppId());

    /*

      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);

            String packageName = packageStates.keyAt(j);

            Ops ops = new Ops(packageName, uidState);

        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.get(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}

     */

services/tests/mockingservicestests/src/com/android/server/appop/AppOpsServiceTest.java

+48 −0
Original line number Diff line number Diff line
@@ -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<String, PackageState> 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<PackageOps> ops = mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName,

                new int[]{OP_READ_SMS});

        assertNotNull(ops);

        for (int i = 0; i < ops.size(); i++) {

            List<OpEntry> opEntries = ops.get(i).getOps();

            for (int j = 0; j < opEntries.size(); j++) {

                Map<String, AppOpsManager.AttributedOpEntry> attributedOpEntries = opEntries.get(

                        j).getAttributedOpEntries();

                assertNotEquals(-1, attributedOpEntries.get(null)

                        .getLastAccessTime(OP_FLAG_SELF));

            }

        }

    }



    private List<PackageOps> getLoggedOps() {

        return mAppOpsService.getOpsForPackage(mMyUid, sMyPackageName, null /* all ops */);

    }