commit ce86e24d233a26b68ae3655ebc3f5730322d78a4
author Eric Biggers <ebiggers@google.com> Tue Aug 01 22:36:55 2023 +0000
committer Eric Biggers <ebiggers@google.com> Tue Aug 01 22:41:44 2023 +0000
tree fb01169e67ff8d59b05e362b62d868118c56fda4
parent 92428b247f86807f96d60d4fa99dae350a7a5237

Call fscrypt_destroy_volume_keys() under mCryptLock

Everything in FsCrypt.cpp seems to run under VolumeManager::mCryptLock,
except for fscrypt_destroy_volume_keys() which uses mLock instead.

This was sort of okay because fscrypt_destroy_volume_keys() didn't
operate on any in-memory data structures.  However, that is going to be
changed.  Therefore, rework VoldNativeService::forgetPartition() to call
fscrypt_destroy_volume_keys() under mCryptLock.

Ignore-AOSP-First: Conflicts. Will cherry-pick after Android 14 push...
Test: see I7f11a135d8550618cd96013f834cebd54be5ef84
Change-Id: Ia27a61faf2fdd546cdbddb2a3985c7c6696f6aa6

FsCrypt.cpp

diff --git a/FsCrypt.cpp b/FsCrypt.cpp
index 7ba3162..b60747a 100644
--- a/FsCrypt.cpp
+++ b/FsCrypt.cpp
@@ -1137,7 +1137,10 @@
     return res;
 }
 
+// Destroys all CE and DE keys for an adoptable storage volume that is permanently going away.
+// Requires VolumeManager::mCryptLock.
 bool fscrypt_destroy_volume_keys(const std::string& volume_uuid) {
+    if (!IsFbeEnabled()) return true;
     bool res = true;
     LOG(DEBUG) << "fscrypt_destroy_volume_keys for volume " << escape_empty(volume_uuid);
     auto secdiscardable_path = volume_secdiscardable_path(volume_uuid);

VoldNativeService.cpp

diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index 5a89ea7..6e02ea2 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -256,9 +256,19 @@
     ENFORCE_SYSTEM_OR_ROOT;
     CHECK_ARGUMENT_HEX(partGuid);
     CHECK_ARGUMENT_HEX(fsUuid);
-    ACQUIRE_LOCK;
+    bool success = true;
 
-    return translate(VolumeManager::Instance()->forgetPartition(partGuid, fsUuid));
+    {
+        ACQUIRE_LOCK;
+        success &= VolumeManager::Instance()->forgetPartition(partGuid, fsUuid);
+    }
+
+    {
+        ACQUIRE_CRYPT_LOCK;
+        success &= fscrypt_destroy_volume_keys(fsUuid);
+    }
+
+    return translateBool(success);
 }
 
 binder::Status VoldNativeService::mount(

VolumeManager.cpp

diff --git a/VolumeManager.cpp b/VolumeManager.cpp
index e29b920..e615172 100644
--- a/VolumeManager.cpp
+++ b/VolumeManager.cpp
@@ -346,25 +346,19 @@
     }
 }
 
-int VolumeManager::forgetPartition(const std::string& partGuid, const std::string& fsUuid) {
+bool VolumeManager::forgetPartition(const std::string& partGuid, const std::string& fsUuid) {
     std::string normalizedGuid;
     if (android::vold::NormalizeHex(partGuid, normalizedGuid)) {
         LOG(WARNING) << "Invalid GUID " << partGuid;
-        return -1;
+        return false;
     }
 
-    bool success = true;
     std::string keyPath = android::vold::BuildKeyPath(normalizedGuid);
     if (unlink(keyPath.c_str()) != 0) {
         LOG(ERROR) << "Failed to unlink " << keyPath;
-        success = false;
+        return false;
     }
-    if (IsFbeEnabled()) {
-        if (!fscrypt_destroy_volume_keys(fsUuid)) {
-            success = false;
-        }
-    }
-    return success ? 0 : -1;
+    return true;
 }
 
 void VolumeManager::destroyEmulatedVolumesForUser(userid_t userId) {

VolumeManager.h

diff --git a/VolumeManager.h b/VolumeManager.h
index 943a144..1a7b510 100644
--- a/VolumeManager.h
+++ b/VolumeManager.h
@@ -106,7 +106,7 @@
 
     userid_t getSharedStorageUser(userid_t userId);
 
-    int forgetPartition(const std::string& partGuid, const std::string& fsUuid);
+    bool forgetPartition(const std::string& partGuid, const std::string& fsUuid);
 
     int onUserAdded(userid_t userId, int userSerialNumber, userid_t cloneParentUserId);
     int onUserRemoved(userid_t userId);