From a1c15b70f88254b498e1e3ddbd121a8144e15905 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Wed, 4 May 2022 02:57:14 +0000
Subject: [PATCH] UserDataPreparer: fix volume preparation order

Internal storage must be prepared before adoptable storage, since the
user's volume keys are stored in their internal storage.  Previously,
the order depended on the Java hashcodes of the volume IDs.  It seems
that in practice things sort of worked out anyway even with the wrong
order, via a bizarre sequence of events involving the user's storage
being deleted and re-created several times.  Regardless, let's fix it.

destroyUserData() sort of had the reverse bug, although I don't think it
actually mattered.  My concern there was that destroying the internal
storage first would bypass vold's destroyKey() for the volume keys.
However, that's okay since the volume keys don't actually need a special
destruction procedure in this case.  Also, internal storage has already
been locked when destroyUserData() runs anyway.

Test: On Cuttlefish:
      $ sm partition disk:253,32 private
      $ pm create-user 10
      $ pm remove-user 10
      Checked logcat for expected messages.
Bug: 231387956
Change-Id: I146d8c786c6b923aed7f8c748db8a7e90c96687f
(cherry picked from commit a8a53c2615f4f6fe9ba14c920bfd8d0b27b8233f)
Merged-In: I146d8c786c6b923aed7f8c748db8a7e90c96687f
---
 .../android/server/pm/UserDataPreparer.java    | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/services/core/java/com/android/server/pm/UserDataPreparer.java b/services/core/java/com/android/server/pm/UserDataPreparer.java
index 95482d7c7f1aa..974a1e1cd59d5 100644
--- a/services/core/java/com/android/server/pm/UserDataPreparer.java
+++ b/services/core/java/com/android/server/pm/UserDataPreparer.java
@@ -70,9 +70,16 @@ class UserDataPreparer {
     void prepareUserData(int userId, int userSerial, int flags) {
         synchronized (mInstallLock) {
             final StorageManager storage = mContext.getSystemService(StorageManager.class);
+            /*
+             * Internal storage must be prepared before adoptable storage, since the user's volume
+             * keys are stored in their internal storage.
+             */
+            prepareUserDataLI(null /* internal storage */, userId, userSerial, flags, true);
             for (VolumeInfo vol : storage.getWritablePrivateVolumes()) {
                 final String volumeUuid = vol.getFsUuid();
-                prepareUserDataLI(volumeUuid, userId, userSerial, flags, true);
+                if (volumeUuid != null) {
+                    prepareUserDataLI(volumeUuid, userId, userSerial, flags, true);
+                }
             }
         }
     }
@@ -136,10 +143,17 @@ class UserDataPreparer {
     void destroyUserData(int userId, int flags) {
         synchronized (mInstallLock) {
             final StorageManager storage = mContext.getSystemService(StorageManager.class);
+            /*
+             * Volume destruction order isn't really important, but to avoid any weird issues we
+             * process internal storage last, the opposite of prepareUserData.
+             */
             for (VolumeInfo vol : storage.getWritablePrivateVolumes()) {
                 final String volumeUuid = vol.getFsUuid();
-                destroyUserDataLI(volumeUuid, userId, flags);
+                if (volumeUuid != null) {
+                    destroyUserDataLI(volumeUuid, userId, flags);
+                }
             }
+            destroyUserDataLI(null /* internal storage */, userId, flags);
         }
     }