From 9ba9092d836b8db79ae3884d2f161400ea237352 Mon Sep 17 00:00:00 2001 From: Calin Juravle Date: Thu, 18 Feb 2021 15:37:10 -0800 Subject: [PATCH] Marked apks loaded by isolated processes as usedByOthers Isolated processes can only access world readable optimized files. In order to not penalize their execution, we mark any dex files they load as usedByOthers. This will prevent speed-profile optimizations which marks the optimized files private to the app process. Test: DexManagerTests Bug: 163018062 Change-Id: Ib041584926ea142f37993f434a728d90db30e9bc --- .../server/pm/PackageManagerService.java | 10 +++-- .../com/android/server/pm/dex/DexManager.java | 39 ++++++++++++++----- .../server/pm/dex/DexManagerTests.java | 32 +++++++++++---- 3 files changed, 61 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 427bb2d9bd34d..b8f4a7d2e3b76 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -12135,10 +12135,10 @@ public class PackageManagerService extends IPackageManager.Stub @Override public void notifyDexLoad(String loadingPackageName, Map classLoaderContextMap, String loaderIsa) { - if (PLATFORM_PACKAGE_NAME.equals(loadingPackageName) - && Binder.getCallingUid() != Process.SYSTEM_UID) { + int callingUid = Binder.getCallingUid(); + if (PLATFORM_PACKAGE_NAME.equals(loadingPackageName) && callingUid != Process.SYSTEM_UID) { Slog.w(TAG, "Non System Server process reporting dex loads as system server. uid=" - + Binder.getCallingUid()); + + callingUid); // Do not record dex loads from processes pretending to be system server. // Only the system server should be assigned the package "android", so reject calls // that don't satisfy the constraint. @@ -12149,6 +12149,7 @@ public class PackageManagerService extends IPackageManager.Stub // in order to verify the expectations. return; } + int userId = UserHandle.getCallingUserId(); ApplicationInfo ai = getApplicationInfo(loadingPackageName, /*flags*/ 0, userId); if (ai == null) { @@ -12156,7 +12157,8 @@ public class PackageManagerService extends IPackageManager.Stub + loadingPackageName + ", user=" + userId); return; } - mDexManager.notifyDexLoad(ai, classLoaderContextMap, loaderIsa, userId); + mDexManager.notifyDexLoad(ai, classLoaderContextMap, loaderIsa, userId, + Process.isIsolated(callingUid)); } @Override diff --git a/services/core/java/com/android/server/pm/dex/DexManager.java b/services/core/java/com/android/server/pm/dex/DexManager.java index 37f317557aeb5..32ba26c2d5edf 100644 --- a/services/core/java/com/android/server/pm/dex/DexManager.java +++ b/services/core/java/com/android/server/pm/dex/DexManager.java @@ -86,6 +86,11 @@ public class DexManager { // However it can load verification data - thus we pick the "verify" compiler filter. private static final String SYSTEM_SERVER_COMPILER_FILTER = "verify"; + // The suffix we add to the package name when the loading happens in an isolated process. + // Note that the double dot creates and "invalid" package name which makes it clear that this + // is an artificially constructed name. + private static final String ISOLATED_PROCESS_PACKAGE_SUFFIX = "..isolated"; + private final Context mContext; // Maps package name to code locations. @@ -166,12 +171,14 @@ public class DexManager { * the class loader context that was used to load them. * @param loaderIsa the ISA of the app loading the dex files * @param loaderUserId the user id which runs the code loading the dex files + * @param loaderIsIsolatedProcess whether or not the loading process is isolated. */ public void notifyDexLoad(ApplicationInfo loadingAppInfo, - Map classLoaderContextMap, String loaderIsa, int loaderUserId) { + Map classLoaderContextMap, String loaderIsa, int loaderUserId, + boolean loaderIsIsolatedProcess) { try { notifyDexLoadInternal(loadingAppInfo, classLoaderContextMap, loaderIsa, - loaderUserId); + loaderUserId, loaderIsIsolatedProcess); } catch (Exception e) { Slog.w(TAG, "Exception while notifying dex load for package " + loadingAppInfo.packageName, e); @@ -181,7 +188,7 @@ public class DexManager { @VisibleForTesting /*package*/ void notifyDexLoadInternal(ApplicationInfo loadingAppInfo, Map classLoaderContextMap, String loaderIsa, - int loaderUserId) { + int loaderUserId, boolean loaderIsIsolatedProcess) { if (classLoaderContextMap == null) { return; } @@ -195,22 +202,36 @@ public class DexManager { return; } + // If this load is coming from an isolated process we need to be able to prevent profile + // based optimizations. This is because isolated processes are sandboxed and can only read + // world readable files, so they need world readable optimization files. An + // example of such a package is webview. + // + // In order to prevent profile optimization we pretend that the load is coming from a + // different package, and so we assign a artificial name to the loading package making it + // clear that it comes from an isolated process. This blends well with the entire + // usedByOthers logic without needing to special handle isolated process in all dexopt + // layers. + String loadingPackageAmendedName = loadingAppInfo.packageName; + if (loaderIsIsolatedProcess) { + loadingPackageAmendedName += ISOLATED_PROCESS_PACKAGE_SUFFIX; + } for (Map.Entry mapping : classLoaderContextMap.entrySet()) { String dexPath = mapping.getKey(); // Find the owning package name. DexSearchResult searchResult = getDexPackage(loadingAppInfo, dexPath, loaderUserId); if (DEBUG) { - Slog.i(TAG, loadingAppInfo.packageName - + " loads from " + searchResult + " : " + loaderUserId + " : " + dexPath); + Slog.i(TAG, loadingPackageAmendedName + + " loads from " + searchResult + " : " + loaderUserId + " : " + dexPath); } if (searchResult.mOutcome != DEX_SEARCH_NOT_FOUND) { // TODO(calin): extend isUsedByOtherApps check to detect the cases where // different apps share the same runtime. In that case we should not mark the dex // file as isUsedByOtherApps. Currently this is a safe approximation. - boolean isUsedByOtherApps = !loadingAppInfo.packageName.equals( - searchResult.mOwningPackageName); + boolean isUsedByOtherApps = + !loadingPackageAmendedName.equals(searchResult.mOwningPackageName); boolean primaryOrSplit = searchResult.mOutcome == DEX_SEARCH_FOUND_PRIMARY || searchResult.mOutcome == DEX_SEARCH_FOUND_SPLIT; @@ -249,7 +270,7 @@ public class DexManager { // async write to disk to make sure we don't loose the data in case of a reboot. if (mPackageDexUsage.record(searchResult.mOwningPackageName, dexPath, loaderUserId, loaderIsa, primaryOrSplit, - loadingAppInfo.packageName, classLoaderContext, overwriteCLC)) { + loadingPackageAmendedName, classLoaderContext, overwriteCLC)) { mPackageDexUsage.maybeWriteAsync(); } } @@ -749,7 +770,7 @@ public class DexManager { dexPath, userId, isa, /*primaryOrSplit*/ false, loadingPackage, PackageDexUsage.VARIABLE_CLASS_LOADER_CONTEXT, - /*overwriteCLC*/ false); + /*overwriteCLC=*/ false); update |= newUpdate; } if (update) { diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/dex/DexManagerTests.java b/services/tests/mockingservicestests/src/com/android/server/pm/dex/DexManagerTests.java index 2e0cadf264cf2..8abe46fab1d67 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/dex/DexManagerTests.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/dex/DexManagerTests.java @@ -409,6 +409,17 @@ public class DexManagerTests { assertIsUsedByOtherApps(mBarUser0, pui, false); } + @Test + public void testNotifyUsedByIsolatedProcess() { + // Bar loads its own apk but as isolatedProcess. + notifyDexLoad(mBarUser0, mBarUser0.getBaseAndSplitDexPaths(), mUser0, + /*isolatedProcess=*/ true); + + // Bar is used by an isolated process and should be marked as usedByOtherApps + PackageUseInfo pui = getPackageUseInfo(mBarUser0); + assertIsUsedByOtherApps(mBarUser0, pui, true); + } + @Test public void testNotifyPackageUpdatedCodeLocations() { // Simulate a split update. @@ -545,7 +556,7 @@ public class DexManagerTests { List classLoaders = Arrays.asList(PATH_CLASS_LOADER_NAME, UNSUPPORTED_CLASS_LOADER_NAME); List classPaths = Arrays.asList(classPath, classPath); - notifyDexLoad(mBarUser0, classLoaders, classPaths, mUser0); + notifyDexLoad(mBarUser0, classLoaders, classPaths, mUser0, /*isolatedProcess=*/ false); assertNoUseInfo(mBarUser0); @@ -664,7 +675,8 @@ public class DexManagerTests { expectedContexts[i] += contextSuffix; } - notifyDexLoad(mFooUser0, fooSecondaries, expectedContexts, mUser0); + notifyDexLoad(mFooUser0, fooSecondaries, expectedContexts, mUser0, + /*isolatedProcess=*/ false); PackageUseInfo pui = getPackageUseInfo(mFooUser0); assertIsUsedByOtherApps(mFooUser0, pui, false); @@ -838,26 +850,32 @@ public class DexManagerTests { assertEquals(codePath, isUsedByOtherApps, pui.isUsedByOtherApps(codePath)); } } + private void notifyDexLoad(TestData testData, List dexPaths, int loaderUserId) { + notifyDexLoad(testData, dexPaths, loaderUserId, /*isolatedProcess=*/ false); + } + + private void notifyDexLoad(TestData testData, List dexPaths, int loaderUserId, + boolean isolatedProcess) { // By default, assume a single class loader in the chain. // This makes writing tests much easier. List classLoaders = Arrays.asList(testData.mClassLoader); List classPaths = dexPaths != null ? Arrays.asList(String.join(File.pathSeparator, dexPaths)) : null; - notifyDexLoad(testData, classLoaders, classPaths, loaderUserId); + notifyDexLoad(testData, classLoaders, classPaths, loaderUserId, isolatedProcess); } private void notifyDexLoad(TestData testData, List classLoaders, - List classPaths, int loaderUserId) { + List classPaths, int loaderUserId, boolean isolatedProcess) { String[] classLoaderContexts = computeClassLoaderContexts(classLoaders, classPaths); // We call the internal function so any exceptions thrown cause test failures. List dexPaths = classPaths != null ? Arrays.asList(classPaths.get(0).split(File.pathSeparator)) : Arrays.asList(); - notifyDexLoad(testData, dexPaths, classLoaderContexts, loaderUserId); + notifyDexLoad(testData, dexPaths, classLoaderContexts, loaderUserId, isolatedProcess); } private void notifyDexLoad(TestData testData, List dexPaths, - String[] classLoaderContexts, int loaderUserId) { + String[] classLoaderContexts, int loaderUserId, boolean isolatedProcess) { assertTrue(dexPaths.size() == classLoaderContexts.length); HashMap dexPathMapping = new HashMap<>(dexPaths.size()); for (int i = 0; i < dexPaths.size(); i++) { @@ -865,7 +883,7 @@ public class DexManagerTests { ? classLoaderContexts[i] : PackageDexUsage.UNSUPPORTED_CLASS_LOADER_CONTEXT); } mDexManager.notifyDexLoadInternal(testData.mPackageInfo.applicationInfo, dexPathMapping, - testData.mLoaderIsa, loaderUserId); + testData.mLoaderIsa, loaderUserId, isolatedProcess); } private String[] computeClassLoaderContexts(List classLoaders,