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

Commit 1fade2fc authored by Calin Juravle's avatar Calin Juravle
Browse files

Fix potential race in DexManager 

The internal cache of DexManager holding information about the code
locations may race if an install happens at the same time with a dex
load.

Test: runtest -x services/tests/servicestests/src/com/android/server/pm/dex/DexManagerTests.java
Bug: 37922353

Change-Id: Ia28fb54d6859ec185aaaf8e0ed68981004b173e5
parent 3b213ed4

services/core/java/com/android/server/pm/dex/DexManager.java

+25 −17
Original line number Original line Diff line number Diff line
@@ -60,6 +60,7 @@ public class DexManager { 
    // Maps package name to code locations.

    // Maps package name to code locations.

    // It caches the code locations for the installed packages. This allows for

    // It caches the code locations for the installed packages. This allows for

    // faster lookups (no locks) when finding what package owns the dex file.

    // faster lookups (no locks) when finding what package owns the dex file.

    @GuardedBy("mPackageCodeLocationsCache")

    private final Map<String, PackageCodeLocations> mPackageCodeLocationsCache;

    private final Map<String, PackageCodeLocations> mPackageCodeLocationsCache;





    // PackageDexUsage handles the actual I/O operations. It is responsible to

    // PackageDexUsage handles the actual I/O operations. It is responsible to
@@ -204,7 +205,7 @@ public class DexManager { 
        // In case there was an update, write the package use info to disk async.

        // In case there was an update, write the package use info to disk async.

        // Note that we do the writing here and not in PackageDexUsage in order to be

        // Note that we do the writing here and not in PackageDexUsage in order to be

        // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs

        // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs

        // multiple updates in PackaeDexUsage before writing it).

        // multiple updates in PackageDexUsage before writing it).

        if (mPackageDexUsage.clearUsedByOtherApps(packageName)) {

        if (mPackageDexUsage.clearUsedByOtherApps(packageName)) {

            mPackageDexUsage.maybeWriteAsync();

            mPackageDexUsage.maybeWriteAsync();

        }

        }
@@ -224,7 +225,7 @@ public class DexManager { 
        // In case there was an update, write the package use info to disk async.

        // In case there was an update, write the package use info to disk async.

        // Note that we do the writing here and not in PackageDexUsage in order to be

        // Note that we do the writing here and not in PackageDexUsage in order to be

        // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs

        // consistent with other methods in DexManager (e.g. reconcileSecondaryDexFiles performs

        // multiple updates in PackaeDexUsage before writing it).

        // multiple updates in PackageDexUsage before writing it).

        if (updated) {

        if (updated) {

            mPackageDexUsage.maybeWriteAsync();

            mPackageDexUsage.maybeWriteAsync();

        }

        }
@@ -243,8 +244,12 @@ public class DexManager { 




    private void cachePackageCodeLocation(String packageName, String baseCodePath,

    private void cachePackageCodeLocation(String packageName, String baseCodePath,

            String[] splitCodePaths, String[] dataDirs, int userId) {

            String[] splitCodePaths, String[] dataDirs, int userId) {

        synchronized (mPackageCodeLocationsCache) {

            PackageCodeLocations pcl = putIfAbsent(mPackageCodeLocationsCache, packageName,

            PackageCodeLocations pcl = putIfAbsent(mPackageCodeLocationsCache, packageName,

                    new PackageCodeLocations(packageName, baseCodePath, splitCodePaths));

                    new PackageCodeLocations(packageName, baseCodePath, splitCodePaths));

            // TODO(calin): We are forced to extend the scope of this synchronization because

            // the values of the cache (PackageCodeLocations) are updated in place.

            // Make PackageCodeLocations immutable to simplify the synchronization reasoning.

            pcl.updateCodeLocation(baseCodePath, splitCodePaths);

            pcl.updateCodeLocation(baseCodePath, splitCodePaths);

            if (dataDirs != null) {

            if (dataDirs != null) {

                for (String dataDir : dataDirs) {

                for (String dataDir : dataDirs) {
@@ -258,6 +263,7 @@ public class DexManager { 
                }

                }

            }

            }

        }

        }

    }





    private void loadInternal(Map<Integer, List<PackageInfo>> existingPackages) {

    private void loadInternal(Map<Integer, List<PackageInfo>> existingPackages) {

        Map<String, Set<Integer>> packageToUsersMap = new HashMap<>();

        Map<String, Set<Integer>> packageToUsersMap = new HashMap<>();
@@ -479,12 +485,14 @@ public class DexManager { 
        // The loadingPackage does not own the dex file.

        // The loadingPackage does not own the dex file.

        // Perform a reverse look-up in the cache to detect if any package has ownership.

        // Perform a reverse look-up in the cache to detect if any package has ownership.

        // Note that we can have false negatives if the cache falls out of date.

        // Note that we can have false negatives if the cache falls out of date.

        synchronized (mPackageCodeLocationsCache) {

            for (PackageCodeLocations pcl : mPackageCodeLocationsCache.values()) {

            for (PackageCodeLocations pcl : mPackageCodeLocationsCache.values()) {

                outcome = pcl.searchDex(dexPath, userId);

                outcome = pcl.searchDex(dexPath, userId);

                if (outcome != DEX_SEARCH_NOT_FOUND) {

                if (outcome != DEX_SEARCH_NOT_FOUND) {

                    return new DexSearchResult(pcl.mPackageName, outcome);

                    return new DexSearchResult(pcl.mPackageName, outcome);

                }

                }

            }

            }

        }





        if (DEBUG) {

        if (DEBUG) {

            // TODO(calin): Consider checking for /data/data symlink.

            // TODO(calin): Consider checking for /data/data symlink.