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

Commit f9fadf81 authored by TYM Tsai's avatar TYM Tsai
Browse files

Fix deadlock between PMS and UMS via writeSettings

Move getting active users out of mLock to fix deadlock.

Symptom:
PMS: hold mLock      ->  UMS: ask mUsersLock
UMS: hold mUsersLock ->  PMS: ask mLock

Bug: 407169095
Flag: EXEMPT bugfix
Test: atest PackageManagerServiceServerTests
Test: atest CtsPackageInstallTestCases
Change-Id: I7d0978eb32f1e15d2aff66aca6332ec573d26212
parent 168d2b61
Loading
Loading
Loading
Loading
+8 −4
Original line number Diff line number Diff line
@@ -126,6 +126,7 @@ import android.content.pm.PermissionInfo;
import android.content.pm.SharedLibraryInfo;
import android.content.pm.Signature;
import android.content.pm.SigningDetails;
import android.content.pm.UserInfo;
import android.content.pm.VerifierInfo;
import android.content.pm.parsing.result.ParseResult;
import android.content.pm.parsing.result.ParseTypeImpl;
@@ -2551,6 +2552,7 @@ final class InstallPackageHelper {
                    PackageManagerException.INTERNAL_ERROR_MISSING_USER));
            return;
        }
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        synchronized (mPm.mLock) {
            // For system-bundled packages, we assume that installing an upgraded version
            // of the package implies that the user actually wants to run that new code,
@@ -2772,7 +2774,7 @@ final class InstallPackageHelper {
            installRequest.setReturnCode(PackageManager.INSTALL_SUCCEEDED);
            //to update install status
            Trace.traceBegin(TRACE_TAG_PACKAGE_MANAGER, "writeSettings");
            mPm.writeSettingsLPrTEMP();
            mPm.writeSettingsLPrTEMP(activeUsers);
            Trace.traceEnd(TRACE_TAG_PACKAGE_MANAGER);
        }

@@ -3322,6 +3324,7 @@ final class InstallPackageHelper {
            @NonNull PackageSetting stubPkgSetting) {
        final int parseFlags = mPm.getDefParseFlags() | ParsingPackageUtils.PARSE_CHATTY
                | ParsingPackageUtils.PARSE_ENFORCE_CODE;
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        try (PackageManagerTracedLock installLock = mPm.mInstallLock.acquireLock()) {
            final AndroidPackage pkg;
            try (PackageFreezer freezer =
@@ -3342,7 +3345,7 @@ final class InstallPackageHelper {
                            Process.INVALID_UID /* previousAppId */,
                            PermissionManagerServiceInternal.PackageInstalledParams.DEFAULT,
                            UserHandle.USER_ALL);
                    mPm.writeSettingsLPrTEMP();
                    mPm.writeSettingsLPrTEMP(activeUsers);
                    // Since compressed package can be system app only, we do not need to
                    // set restricted settings on it.
                }
@@ -3375,7 +3378,7 @@ final class InstallPackageHelper {
                            stubPs.setEnabled(COMPONENT_ENABLED_STATE_DISABLED,
                                    UserHandle.USER_SYSTEM, "android");
                        }
                        mPm.writeSettingsLPrTEMP();
                        mPm.writeSettingsLPrTEMP(activeUsers);
                    }
                }
                return false;
@@ -3570,6 +3573,7 @@ final class InstallPackageHelper {
    private void setPackageInstalledForSystemPackage(@NonNull AndroidPackage pkg,
            @NonNull int[] allUserHandles, @Nullable int[] origUserHandles,
            boolean writeSettings) {
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        // writer
        synchronized (mPm.mLock) {
            PackageSetting ps = mPm.mSettings.getPackageLPr(pkg.getPackageName());
@@ -3614,7 +3618,7 @@ final class InstallPackageHelper {

            // can downgrade to reader here
            if (writeSettings) {
                mPm.writeSettingsLPrTEMP();
                mPm.writeSettingsLPrTEMP(activeUsers);
            }
        }
    }
+16 −10
Original line number Diff line number Diff line
@@ -1633,10 +1633,11 @@ public class PackageManagerService implements PackageSender, TestUtilityService
    }

    void writeSettings(boolean sync) {
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mUserManager);
        synchronized (mLock) {
            mHandler.removeMessages(WRITE_SETTINGS);
            mBackgroundHandler.removeMessages(WRITE_DIRTY_PACKAGE_RESTRICTIONS);
            writeSettingsLPrTEMP(sync);
            writeSettingsLPrTEMP(activeUsers, sync);
            synchronized (mDirtyUsers) {
                mDirtyUsers.clear();
            }
@@ -1644,9 +1645,10 @@ public class PackageManagerService implements PackageSender, TestUtilityService
    }

    void writePackageList(int userId) {
        List<UserInfo> activeUsers = Settings.getActiveUsers(mUserManager);
        synchronized (mLock) {
            mHandler.removeMessages(WRITE_PACKAGE_LIST);
            mSettings.writePackageListLPr(userId);
            mSettings.writePackageListLPr(activeUsers, userId);
        }
    }

@@ -2452,7 +2454,8 @@ public class PackageManagerService implements PackageSender, TestUtilityService

            // can downgrade to reader
            t.traceBegin("write settings");
            writeSettingsLPrTEMP();
            final List<UserInfo> activeUsers = Settings.getActiveUsers(mUserManager);
            writeSettingsLPrTEMP(activeUsers);
            t.traceEnd();
            EventLog.writeEvent(EventLogTags.BOOT_PROGRESS_PMS_READY,
                    SystemClock.uptimeMillis());
@@ -4437,13 +4440,14 @@ public class PackageManagerService implements PackageSender, TestUtilityService

    /** Called by UserManagerService */
    void cleanUpUser(UserManagerService userManager, @UserIdInt int userId) {
        final List<UserInfo> activeUsers = Settings.getActiveUsers(userManager);
        synchronized (mLock) {
            synchronized (mDirtyUsers) {
                mDirtyUsers.remove(userId);
            }
            mUserNeedsBadging.delete(userId);
            mDeletePackageHelper.removeUnusedPackagesLPw(userManager, userId);
            mSettings.removeUserLPw(userId);
            mSettings.removeUserLPw(activeUsers, userId);
            mPendingBroadcasts.remove(userId);
            mAppsFilter.onUserDeleted(snapshotComputer(), userId);
            mPermissionManager.onUserRemoved(userId);
@@ -5456,8 +5460,9 @@ public class PackageManagerService implements PackageSender, TestUtilityService
        public VerifierDeviceIdentity getVerifierDeviceIdentity() throws RemoteException {
            getVerifierDeviceIdentity_enforcePermission();

            final List<UserInfo> activeUsers = Settings.getActiveUsers(mUserManager);
            synchronized (mLock) {
                return mSettings.getVerifierDeviceIdentityLPw(mLiveComputer);
                return mSettings.getVerifierDeviceIdentityLPw(mLiveComputer, activeUsers);
            }
        }

@@ -7105,11 +7110,12 @@ public class PackageManagerService implements PackageSender, TestUtilityService

        @Override
        public void writeSettings(boolean async) {
            final List<UserInfo> activeUsers = Settings.getActiveUsers(mUserManager);
            synchronized (mLock) {
                if (async) {
                    scheduleWriteSettings();
                } else {
                    writeSettingsLPrTEMP();
                    writeSettingsLPrTEMP(activeUsers, /* sync= */ false);
                }
            }
        }
@@ -7704,15 +7710,15 @@ public class PackageManagerService implements PackageSender, TestUtilityService
     * TODO(b/182523293): This should be removed once we finish migration of permission storage.
     */
    @SuppressWarnings("GuardedBy")
    void writeSettingsLPrTEMP(boolean sync) {
    void writeSettingsLPrTEMP(List<UserInfo> users, boolean sync) {
        snapshotComputer(false);
        mPermissionManager.writeLegacyPermissionsTEMP(mSettings.mPermissions);
        mSettings.writeLPr(mLiveComputer, sync);
        mSettings.writeLPr(mLiveComputer, users, sync);
    }

    // Default async version.
    void writeSettingsLPrTEMP() {
        writeSettingsLPrTEMP(/*sync=*/false);
    void writeSettingsLPrTEMP(List<UserInfo> users) {
        writeSettingsLPrTEMP(users, /*sync=*/false);
    }

    @Override
+3 −1
Original line number Diff line number Diff line
@@ -33,6 +33,7 @@ import android.annotation.Nullable;
import android.annotation.SpecialUsers.CanBeALL;
import android.annotation.UserIdInt;
import android.content.pm.PackageManager;
import android.content.pm.UserInfo;
import android.content.pm.parsing.ApkLiteParseUtils;
import android.content.pm.parsing.PackageLite;
import android.content.pm.parsing.result.ParseResult;
@@ -473,11 +474,12 @@ final class RemovePackageHelper {
                }
            }
        }
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        synchronized (mPm.mLock) {
            // can downgrade to reader
            if (writeSettings) {
                // Save settings now
                mPm.writeSettingsLPrTEMP();
                mPm.writeSettingsLPrTEMP(activeUsers);
            }
            if (installedStateChanged) {
                mPm.mSettings.writeKernelMappingLPr(deletedPs);
+24 −14
Original line number Diff line number Diff line
@@ -2817,7 +2817,13 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
        }
    }

    /** only for test */
    void writeLPr(@NonNull Computer computer, boolean sync) {
        final List<UserInfo> activeUsers = getActiveUsers(UserManagerService.getInstance());
        writeLPr(computer, activeUsers, sync);
    }

    void writeLPr(@NonNull Computer computer, List<UserInfo> users, boolean sync) {
        //Debug.startMethodTracing("/data/system/packageprof", 8 * 1024 * 1024);

        final long startTime = SystemClock.uptimeMillis();
@@ -2918,7 +2924,7 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
                atomicFile.finishWrite(str);

                writeKernelMappingLPr();
                writePackageListLPr();
                writePackageListLPr(users);
                writeAllUsersPackageRestrictionsLPr(sync);
                writeAllRuntimePermissionsLPr();
                com.android.internal.logging.EventLogTags.writeCommitSysConfigFile(
@@ -3035,11 +3041,11 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
        }
    }

    void writePackageListLPr() {
        writePackageListLPr(-1);
    void writePackageListLPr(List<UserInfo> users) {
        writePackageListLPr(users, -1);
    }

    void writePackageListLPr(int creatingUserId) {
    void writePackageListLPr(List<UserInfo> users, int creatingUserId) {
        String filename = mPackageListFilename.getAbsolutePath();
        String ctx = SELinux.fileSelabelLookup(filename);
        if (ctx == null) {
@@ -3051,15 +3057,14 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
            Slog.wtf(TAG, "Failed to set packages.list SELinux context");
        }
        try {
            writePackageListLPrInternal(creatingUserId);
            writePackageListLPrInternal(users, creatingUserId);
        } finally {
            SELinux.setFSCreateContext(null);
        }
    }

    private void writePackageListLPrInternal(int creatingUserId) {
    private void writePackageListLPrInternal(List<UserInfo> users, int creatingUserId) {
        // Only derive GIDs for active users (not dying)
        final List<UserInfo> users = getActiveUsers(UserManagerService.getInstance(), true);
        int[] userIds = new int[users.size()];
        for (int i = 0; i < userIds.length; i++) {
            userIds[i] = users.get(i).id;
@@ -4820,7 +4825,13 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
        t.traceEnd(); // createNewUser
    }

    /** only for test */
    void removeUserLPw(int userId) {
        final List<UserInfo> activeUsers = getActiveUsers(UserManagerService.getInstance());
        removeUserLPw(activeUsers, userId);
    }

    void removeUserLPw(List<UserInfo> users, int userId) {
        Set<Entry<String, PackageSetting>> entries = mPackages.entrySet();
        for (Entry<String, PackageSetting> entry : entries) {
            entry.getValue().removeUser(userId);
@@ -4837,7 +4848,7 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
        mRuntimePermissionsPersistence.onUserRemoved(userId);
        mDomainVerificationManager.clearUser(userId);

        writePackageListLPr();
        writePackageListLPr(users);

        // Inform kernel that the user was removed, so that packages are marked uninstalled
        // for sdcardfs
@@ -4872,11 +4883,12 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
        }
    }

    public VerifierDeviceIdentity getVerifierDeviceIdentityLPw(@NonNull Computer computer) {
    public VerifierDeviceIdentity getVerifierDeviceIdentityLPw(@NonNull Computer computer,
            List<UserInfo> users) {
        if (mVerifierDeviceIdentity == null) {
            mVerifierDeviceIdentity = VerifierDeviceIdentity.generate();

            writeLPr(computer, /*sync=*/false);
            writeLPr(computer, users, /*sync=*/false);
        }

        return mVerifierDeviceIdentity;
@@ -4951,13 +4963,11 @@ public final class Settings implements Watchable, Snappable, ResilientAtomicFile
     * Returns the list of users on the device, excluding pre-created ones.
     *
     * @param userManager UserManagerService instance
     * @param excludeDying Indicates whether to exclude any users marked for deletion.
     *
     * @return the list of users
     */
    private static List<UserInfo> getActiveUsers(UserManagerService userManager,
            boolean excludeDying) {
        return getUsers(userManager, excludeDying, /* excludePreCreated= */ true);
    static List<UserInfo> getActiveUsers(UserManagerService userManager) {
        return getUsers(userManager, /* excludeDying= */ true, /* excludePreCreated= */ true);
    }

    /**
+6 −3
Original line number Diff line number Diff line
@@ -108,6 +108,7 @@ public final class StorageEventHelper extends StorageEventListener {
        }

        // Remove any apps installed on the forgotten volume
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        synchronized (mPm.mLock) {
            final List<? extends PackageStateInternal> packages =
                    mPm.mSettings.getVolumePackagesLPr(fsUuid);
@@ -125,7 +126,7 @@ public final class StorageEventHelper extends StorageEventListener {
            }

            mPm.mSettings.onVolumeForgotten(fsUuid);
            mPm.writeSettingsLPrTEMP();
            mPm.writeSettingsLPrTEMP(activeUsers);
        }
    }

@@ -207,6 +208,7 @@ public final class StorageEventHelper extends StorageEventListener {
            }
        }

        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        synchronized (mPm.mLock) {
            final boolean isUpgrade = !PackagePartitions.FINGERPRINT.equals(ver.fingerprint);
            if (isUpgrade) {
@@ -219,7 +221,7 @@ public final class StorageEventHelper extends StorageEventListener {
            // Yay, everything is now upgraded
            ver.forceCurrent();

            mPm.writeSettingsLPrTEMP();
            mPm.writeSettingsLPrTEMP(activeUsers);
        }

        for (PackageFreezer freezer : freezers) {
@@ -246,6 +248,7 @@ public final class StorageEventHelper extends StorageEventListener {
        }

        final int[] userIds = mPm.mUserManager.getUserIds();
        final List<UserInfo> activeUsers = Settings.getActiveUsers(mPm.mUserManager);
        final ArrayList<AndroidPackage> unloaded = new ArrayList<>();
        try (PackageManagerTracedLock installLock = mPm.mInstallLock.acquireLock()) {
            synchronized (mPm.mLock) {
@@ -274,7 +277,7 @@ public final class StorageEventHelper extends StorageEventListener {
                    AttributeCache.instance().removePackage(ps.getPackageName());
                }

                mPm.writeSettingsLPrTEMP();
                mPm.writeSettingsLPrTEMP(activeUsers);
            }
        }