From 79a4f575d671231987c8fb85743d03f556707a41 Mon Sep 17 00:00:00 2001
From: Alan Stokes <alanstokes@google.com>
Date: Wed, 21 Feb 2024 15:32:27 +0000
Subject: [PATCH] Ignore access from a different user

This shouldn't happen, but if it does we should just ignore it rather
than crashing system server.

Modify the tests to match.

Bug: 323665257
Bug: 326190135
Test: atest PackageDynamicCodeLoadingTests
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1aea0e915748d661e2bdf251e34bdc32d22ce0e9)
Merged-In: I2ae7f506b15133f45f00c24e909f49d56fc23e66
Change-Id: I2ae7f506b15133f45f00c24e909f49d56fc23e66
---
 .../android/server/pm/dex/PackageDynamicCodeLoading.java   | 7 ++++---
 .../server/pm/dex/PackageDynamicCodeLoadingTests.java      | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/services/core/java/com/android/server/pm/dex/PackageDynamicCodeLoading.java b/services/core/java/com/android/server/pm/dex/PackageDynamicCodeLoading.java
index cc26c9b5f76c..9159851a5470 100644
--- a/services/core/java/com/android/server/pm/dex/PackageDynamicCodeLoading.java
+++ b/services/core/java/com/android/server/pm/dex/PackageDynamicCodeLoading.java
@@ -538,9 +538,10 @@ class PackageDynamicCodeLoading extends AbstractStatsBase<Void> {
             } else {
                 if (fileInfo.mUserId != userId) {
                     // This should be impossible: private app files are always user-specific and
-                    // can't be accessed from different users.
-                    throw new IllegalArgumentException("Cannot change userId for '" + path
-                            + "' from " + fileInfo.mUserId + " to " + userId);
+                    // can't be accessed from different users. But it does very occasionally happen
+                    // (b/323665257). Ignore such cases - we shouldn't record data from a different
+                    // user.
+                    return false;
                 }
                 // Changing file type (i.e. loading the same file in different ways is possible if
                 // unlikely. We allow it but ignore it.
diff --git a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDynamicCodeLoadingTests.java b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDynamicCodeLoadingTests.java
index e075379284f0..c0ea1571d8c7 100644
--- a/services/tests/servicestests/src/com/android/server/pm/dex/PackageDynamicCodeLoadingTests.java
+++ b/services/tests/servicestests/src/com/android/server/pm/dex/PackageDynamicCodeLoadingTests.java
@@ -106,13 +106,13 @@ public class PackageDynamicCodeLoadingTests {
     }
 
     @Test
-    public void testRecord_changeUserForFile_throws() {
+    public void testRecord_changeUserForFile_ignored() {
         Entry entry1 = new Entry("owning.package1", "/path/file1", 'D', 10, "loading.package1");
         Entry entry2 = new Entry("owning.package1", "/path/file1", 'D', 20, "loading.package1");
 
         PackageDynamicCodeLoading info = makePackageDcl(entry1);
 
-        assertThrows(() -> record(info, entry2));
+        assertThat(record(info, entry2)).isFalse();
         assertHasEntries(info, entry1);
     }
 
-- 
GitLab