Ensure the dex use database cannot grow unboundedly.

Its size can theoretically be

  O(<owning package> X <dex path> X <loading package>)

Owning and loading packages are limited by the valid entries in the
package database. Dex paths from primary dex'es are also limited by the
installed packages. However dex paths for secondary dex'es can
potentially be unbounded, so impose a limit on them.

The limit is fixed for any given owning package, which is simpler than
limiting based on loading package. That restricts a package from adding
an arbitrary number of secondary dex files in itself. Also check that
the dex file exists if the loading package is different from the owning
one, so that the former cannot consume entries up to the limit for the
latter.

The class loader context strings for secondary dex files are also not
guaranteed bounded, so impose a limit on them as well.

Test: atest DexUseManagerTest
Test: Install app_debug.apk from b/391895923#comment3 and verify that
      it can run until OOM repeatedly without growing the database to
      more than 650 KiB.
Bug: 391895923
Flag: EXEMPT bugfix
Ignore-AOSP-First: Security fix
Change-Id: Ic50bf22000730282d90a4f6aa1c49379357fe77a

3 files changed, 188 insertions, 21 deletions

diff --git a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
index 8d47cb6ba0..704dabe034 100644
--- a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
+++ b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java
@@ -113,6 +113,16 @@ public class DexUseManagerLocal {
      */
     @VisibleForTesting public static final long INTERVAL_MS = 15_000;
 
+    // Impose a limit on the input accepted by notifyDexContainersLoaded per owning package.
+    /** @hide */
+    @VisibleForTesting public static final int MAX_PATH_LENGTH = 4096;
+
+    /** @hide */
+    @VisibleForTesting public static final int MAX_CLASS_LOADER_CONTEXT_LENGTH = 10000;
+
+    /** @hide */
+    private static final int MAX_SECONDARY_DEX_FILES_PER_OWNER = 500;
+
     private static final Object sLock = new Object();
 
     // The static field is associated with the class and the class loader that loads it. In the
@@ -527,7 +537,7 @@ public class DexUseManagerLocal {
             }
 
             // Check remaining packages. Don't check for shared libraries because it might be too
-            // expansive to do so and the time complexity is O(n) no matter we do it or not.
+            // expensive to do so and the time complexity is O(n) no matter we do it or not.
             for (PackageState pkgState : packageStates.values()) {
                 if (visitedPackages.contains(pkgState.getPackageName())) {
                     continue;
@@ -657,16 +667,40 @@ public class DexUseManagerLocal {
     private void addSecondaryDexUse(@NonNull String owningPackageName, @NonNull String dexPath,
             @NonNull String loadingPackageName, boolean isolatedProcess,
             @NonNull String classLoaderContext, @NonNull String abiName, long lastUsedAtMs) {
+        DexLoader loader = DexLoader.create(loadingPackageName, isolatedProcess);
+        // This is to avoid a loading package from using up the SecondaryDexUse entries for another
+        // package (up to the MAX_SECONDARY_DEX_FILES_PER_OWNER limit). We don't care about the
+        // loading package messing up its own SecondaryDexUse entries.
+        // Note that we are using system_server's permission to check the existence. This is fine
+        // with the assumption that the file must be world readable to be used by other apps.
+        // We could use artd's permission to check the existence, and then there wouldn't be any
+        // permission issue, but that requires bringing up the artd service, which may be too
+        // expensive.
+        // TODO(jiakaiz): Check if the assumption is true.
+        if (isLoaderOtherApp(loader, owningPackageName) && !mInjector.pathExists(dexPath)) {
+            AsLog.w("Not recording non-existent secondary dex file '" + dexPath + "'");
+            return;
+        }
         synchronized (mLock) {
+            PackageDexUse packageDexUse = mDexUse.mPackageDexUseByOwningPackageName.computeIfAbsent(
+                    owningPackageName, k -> new PackageDexUse());
             SecondaryDexUse secondaryDexUse =
-                    mDexUse.mPackageDexUseByOwningPackageName
-                            .computeIfAbsent(owningPackageName, k -> new PackageDexUse())
-                            .mSecondaryDexUseByDexFile.computeIfAbsent(
-                                    dexPath, k -> new SecondaryDexUse());
+                    packageDexUse.mSecondaryDexUseByDexFile.computeIfAbsent(dexPath, k -> {
+                        if (packageDexUse.mSecondaryDexUseByDexFile.size()
+                                >= mInjector.getMaxSecondaryDexFilesPerOwner()) {
+                            AsLog.w("Not recording too many secondary dex use entries for "
+                                    + owningPackageName);
+                            return null;
+                        }
+                        return new SecondaryDexUse();
+                    });
+            if (secondaryDexUse == null) {
+                return;
+            }
             secondaryDexUse.mUserHandle = mInjector.getCallingUserHandle();
-            SecondaryDexUseRecord record = secondaryDexUse.mRecordByLoader.computeIfAbsent(
-                    DexLoader.create(loadingPackageName, isolatedProcess),
-                    k -> new SecondaryDexUseRecord());
+            SecondaryDexUseRecord record =
+                    secondaryDexUse.mRecordByLoader.computeIfAbsent(
+                            loader, k -> new SecondaryDexUseRecord());
             record.mClassLoaderContext = classLoaderContext;
             record.mAbiName = abiName;
             record.mLastUsedAtMs = lastUsedAtMs;
@@ -772,13 +806,23 @@ public class DexUseManagerLocal {
         }
 
         for (var entry : classLoaderContextByDexContainerFile.entrySet()) {
-            Utils.assertNonEmpty(entry.getKey());
-            String errorMsg = ArtJni.validateDexPath(entry.getKey());
+            String dexPath = entry.getKey();
+            String classLoaderContext = entry.getValue();
+            Utils.assertNonEmpty(dexPath);
+            if (dexPath.length() > MAX_PATH_LENGTH) {
+                throw new IllegalArgumentException(
+                        "Dex path too long - exceeds " + MAX_PATH_LENGTH + " chars");
+            }
+            String errorMsg = ArtJni.validateDexPath(dexPath);
             if (errorMsg != null) {
                 throw new IllegalArgumentException(errorMsg);
             }
-            Utils.assertNonEmpty(entry.getValue());
-            errorMsg = ArtJni.validateClassLoaderContext(entry.getKey(), entry.getValue());
+            Utils.assertNonEmpty(classLoaderContext);
+            if (classLoaderContext.length() > MAX_CLASS_LOADER_CONTEXT_LENGTH) {
+                throw new IllegalArgumentException("Class loader context too long - exceeds "
+                        + MAX_CLASS_LOADER_CONTEXT_LENGTH + " chars");
+            }
+            errorMsg = ArtJni.validateClassLoaderContext(dexPath, classLoaderContext);
             if (errorMsg != null) {
                 throw new IllegalArgumentException(errorMsg);
             }
@@ -1354,6 +1398,10 @@ public class DexUseManagerLocal {
             return System.currentTimeMillis();
         }
 
+        public boolean pathExists(String path) {
+            return new File(path).exists();
+        }
+
         @NonNull
         public String getFilename() {
             return FILENAME;
@@ -1404,5 +1452,9 @@ public class DexUseManagerLocal {
         public boolean isIsolatedUid(int uid) {
             return Process.isIsolatedUid(uid);
         }
+
+        public int getMaxSecondaryDexFilesPerOwner() {
+            return MAX_SECONDARY_DEX_FILES_PER_OWNER;
+        }
     }
 }
diff --git a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
index d404b4cbda..f0002b80d7 100644
--- a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
+++ b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java
@@ -28,6 +28,7 @@ import static org.mockito.Mockito.argThat;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
@@ -85,6 +86,9 @@ public class DexUseManagerTest {
     private static final String INVISIBLE_BASE_APK =
             "/somewhere/app/" + INVISIBLE_PKG_NAME + "/base.apk";
 
+    // A reduced limit to make the test run faster.
+    private static final int MAX_SECONDARY_DEX_FILES_PER_OWNER_FOR_TESTING = 50;
+
     @Rule
     public StaticMockitoRule mockitoRule = new StaticMockitoRule(
             SystemProperties.class, Constants.class, ArtJni.class);
@@ -173,6 +177,7 @@ public class DexUseManagerTest {
 
         lenient().when(mInjector.getArtd()).thenReturn(mArtd);
         lenient().when(mInjector.getCurrentTimeMillis()).thenReturn(0l);
+        lenient().when(mInjector.pathExists(any())).thenReturn(true);
         lenient().when(mInjector.getFilename()).thenReturn(mTempFile.getPath());
         lenient()
                 .when(mInjector.createScheduledExecutor())
@@ -185,6 +190,9 @@ public class DexUseManagerTest {
         lenient().when(mInjector.getCallingUserHandle()).thenReturn(mUserHandle);
         lenient().when(mInjector.getCallingUid()).thenReturn(110001);
         lenient().when(mInjector.isIsolatedUid(anyInt())).thenReturn(false);
+        lenient()
+                .when(mInjector.getMaxSecondaryDexFilesPerOwner())
+                .thenReturn(MAX_SECONDARY_DEX_FILES_PER_OWNER_FOR_TESTING);
 
         mDexUseManager = new DexUseManagerLocal(mInjector);
         mDexUseManager.systemReady();
@@ -639,11 +647,11 @@ public class DexUseManagerTest {
 
     @Test
     public void testCheckedSecondaryDexNotFound() throws Exception {
-        when(mArtd.getDexFileVisibility(mCeDir + "/foo.apk")).thenReturn(FileVisibility.NOT_FOUND);
-
         mDexUseManager.notifyDexContainersLoaded(
                 mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC"));
 
+        when(mArtd.getDexFileVisibility(mCeDir + "/foo.apk")).thenReturn(FileVisibility.NOT_FOUND);
+
         assertThat(mDexUseManager.getCheckedSecondaryDexInfo(
                            OWNING_PKG_NAME, true /* excludeObsoleteDexesAndLoaders */))
                 .isEmpty();
@@ -825,6 +833,18 @@ public class DexUseManagerTest {
     }
 
     @Test(expected = IllegalArgumentException.class)
+    public void testTooLongDexPath() throws Exception {
+        mDexUseManager.notifyDexContainersLoaded(mSnapshot, OWNING_PKG_NAME,
+                Map.of("/" + "X".repeat(DexUseManagerLocal.MAX_PATH_LENGTH), "CLC"));
+    }
+
+    @Test
+    public void testMaxLengthDexPath() throws Exception {
+        mDexUseManager.notifyDexContainersLoaded(mSnapshot, OWNING_PKG_NAME,
+                Map.of("/" + "X".repeat(DexUseManagerLocal.MAX_PATH_LENGTH - 1), "CLC"));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
     public void testInvalidDexPath() throws Exception {
         lenient().when(ArtJni.validateDexPath(any())).thenReturn("invalid");
         mDexUseManager.notifyDexContainersLoaded(
@@ -832,6 +852,20 @@ public class DexUseManagerTest {
     }
 
     @Test(expected = IllegalArgumentException.class)
+    public void testTooLongClassLoaderContext() throws Exception {
+        mDexUseManager.notifyDexContainersLoaded(mSnapshot, OWNING_PKG_NAME,
+                Map.of(mCeDir + "/foo.apk",
+                        "X".repeat(DexUseManagerLocal.MAX_CLASS_LOADER_CONTEXT_LENGTH + 1)));
+    }
+
+    @Test
+    public void testMaxLengthClassLoaderContext() throws Exception {
+        mDexUseManager.notifyDexContainersLoaded(mSnapshot, OWNING_PKG_NAME,
+                Map.of(mCeDir + "/foo.apk",
+                        "X".repeat(DexUseManagerLocal.MAX_CLASS_LOADER_CONTEXT_LENGTH)));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
     public void testInvalidClassLoaderContext() throws Exception {
         lenient().when(ArtJni.validateClassLoaderContext(any(), any())).thenReturn("invalid");
         mDexUseManager.notifyDexContainersLoaded(
@@ -874,6 +908,87 @@ public class DexUseManagerTest {
                 mSnapshot, OWNING_PKG_NAME, Map.of(BASE_APK, "CLC"));
     }
 
+    @Test
+    public void testExistingExternalSecondaryDexPath() throws Exception {
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        long oldFileSize = mTempFile.length();
+
+        String existingDexPath = mCeDir + "/foo.apk";
+        when(mInjector.pathExists(existingDexPath)).thenReturn(true);
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, LOADING_PKG_NAME, Map.of(existingDexPath, "PCL[]"));
+
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isGreaterThan(oldFileSize);
+    }
+
+    @Test
+    public void testNonexistingExternalSecondaryDexPath() throws Exception {
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        long oldFileSize = mTempFile.length();
+
+        String nonexistingDexPath = mCeDir + "/foo.apk";
+        when(mInjector.pathExists(nonexistingDexPath)).thenReturn(false);
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, LOADING_PKG_NAME, Map.of(nonexistingDexPath, "PCL[]"));
+
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isEqualTo(oldFileSize);
+    }
+
+    @Test
+    public void testInternalSecondaryDexPath() throws Exception {
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        long oldFileSize = mTempFile.length();
+
+        String nonexistingDexPath = mCeDir + "/foo.apk";
+        lenient().when(mInjector.pathExists(nonexistingDexPath)).thenReturn(false);
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, OWNING_PKG_NAME, Map.of(nonexistingDexPath, "PCL[]"));
+        verify(mArtd, never()).getDexFileVisibility(nonexistingDexPath);
+
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isGreaterThan(oldFileSize);
+    }
+
+    @Test
+    public void testLimitSecondaryDexFiles() throws Exception {
+        for (int n = 0; n < MAX_SECONDARY_DEX_FILES_PER_OWNER_FOR_TESTING - 1; ++n) {
+            mDexUseManager.notifyDexContainersLoaded(mSnapshot, LOADING_PKG_NAME,
+                    Map.of(String.format("%s/%04d/foo.apk", mCeDir, n), "CLC"));
+        }
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        long oldFileSize = mTempFile.length();
+
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, LOADING_PKG_NAME, Map.of(mCeDir + "/9998/foo.apk", "CLC"));
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isGreaterThan(oldFileSize);
+
+        oldFileSize = mTempFile.length();
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, LOADING_PKG_NAME, Map.of(mCeDir + "/9999/foo.apk", "CLC"));
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isEqualTo(oldFileSize);
+
+        // Can still add loading packages to existing entries after the limit is reached.
+        mDexUseManager.notifyDexContainersLoaded(
+                mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/9998/foo.apk", "CLC"));
+        mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); // Save.
+        assertThat(mTempFile.length()).isGreaterThan(oldFileSize);
+    }
+
+    @Test
+    public void testLimitSecondaryDexFilesSingleCall() throws Exception {
+        Map<String, String> clcByDexFile = new HashMap<>();
+        for (int n = 0; n < MAX_SECONDARY_DEX_FILES_PER_OWNER_FOR_TESTING + 1; ++n) {
+            clcByDexFile.put(String.format("%s/%04d/foo.apk", mCeDir, n), "CLC");
+        }
+        mDexUseManager.notifyDexContainersLoaded(mSnapshot, LOADING_PKG_NAME, clcByDexFile);
+        assertThat(mDexUseManager.getSecondaryDexInfo(OWNING_PKG_NAME))
+                .hasSize(MAX_SECONDARY_DEX_FILES_PER_OWNER_FOR_TESTING);
+    }
+
     private AndroidPackage createPackage(String packageName) {
         AndroidPackage pkg = mock(AndroidPackage.class);
         lenient().when(pkg.getStorageUuid()).thenReturn(StorageManager.UUID_DEFAULT);
diff --git a/libartservice/service/proto/dex_use.proto b/libartservice/service/proto/dex_use.proto
index 1dd962dbf4..1960882ad5 100644
--- a/libartservice/service/proto/dex_use.proto
+++ b/libartservice/service/proto/dex_use.proto
@@ -29,31 +29,31 @@ message DexUseProto {
 }
 
 message PackageDexUseProto {
-    string owning_package_name = 1;
+    string owning_package_name = 1;  // key
     repeated PrimaryDexUseProto primary_dex_use = 2;
     repeated SecondaryDexUseProto secondary_dex_use = 3;
 }
 
 message PrimaryDexUseProto {
-    string dex_file = 1;
+    string dex_file = 1;  // key
     repeated PrimaryDexUseRecordProto record = 2;
 }
 
 message PrimaryDexUseRecordProto {
-    string loading_package_name = 1;
-    bool isolated_process = 2;
+    string loading_package_name = 1;  // key
+    bool isolated_process = 2;        // key
     int64 last_used_at_ms = 3;
 }
 
 message SecondaryDexUseProto {
-    string dex_file = 1;
+    string dex_file = 1;     // key
     Int32Value user_id = 2;  // Must be explicitly set.
     repeated SecondaryDexUseRecordProto record = 3;
 }
 
 message SecondaryDexUseRecordProto {
-    string loading_package_name = 1;
-    bool isolated_process = 2;
+    string loading_package_name = 1;  // key
+    bool isolated_process = 2;        // key
     string class_loader_context = 3;
     string abi_name = 4;
     int64 last_used_at_ms = 5;