From 73e3010a252e22224282a7de6ba204487c543481 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Tue, 12 Jul 2022 08:11:02 +0000 Subject: [PATCH] Support bind mounting volumes into other volume's mountpoint. With the way the FUSE mount point are currently setup for emulated volumes, there can be multiple paths that serve the same files on the lower filesystem; eg * /mnt/user/0/emulated/0/Android * /mnt/user/10/emulated/0/Android both refer to the same file on the lower filesystem: * /data/media/0/Android this is normally not a problem, because cross-user file access is not allowed, and so the FUSE daemon won't serve files for other users. With clone profiles this is no longer true however, as their volumes are accessible by each other. So, it can happen that an app running in clone profile 10 accesses "/mnt/user/10/emulated/0/Android", which would be served by the FUSE daemon for the user 10 filesystem. At the same time, an app running in the owner profile 0 accesses "mnt/user/0/emulated/0/Android", which would be served by the FUSE daemon for the user 0 filesystem. This can cause page cache inconsistencies, because multiple FUSE daemons can be running on top of the same entries in the lower filesystem. To prevent this, use bind mounts to make sure that cross-profile accesses actually end up in the FUSE daemon to which the volume belongs: "/mnt/user/10/emulated/0" is bind-mounted to "/mnt/user/0/emulated/0", and vice-versa. Bug: 228271997 Test: manual Change-Id: Iefcbc813670628b329a1a5d408b6126b84991e09 --- VoldNativeService.cpp | 6 ++- VoldNativeService.h | 2 +- VolumeManager.cpp | 16 ++++++- VolumeManager.h | 6 ++- binder/android/os/IVold.aidl | 2 +- model/EmulatedVolume.cpp | 87 +++++++++++++++++++++++++++++++++++- model/EmulatedVolume.h | 7 ++- 7 files changed, 118 insertions(+), 8 deletions(-) diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp index ea2c98c..601323f 100644 --- a/VoldNativeService.cpp +++ b/VoldNativeService.cpp @@ -182,11 +182,13 @@ binder::Status VoldNativeService::abortFuse() { return translate(VolumeManager::Instance()->abortFuse()); } -binder::Status VoldNativeService::onUserAdded(int32_t userId, int32_t userSerial) { +binder::Status VoldNativeService::onUserAdded(int32_t userId, int32_t userSerial, + int32_t sharesStorageWithUserId) { ENFORCE_SYSTEM_OR_ROOT; ACQUIRE_LOCK; - return translate(VolumeManager::Instance()->onUserAdded(userId, userSerial)); + return translate( + VolumeManager::Instance()->onUserAdded(userId, userSerial, sharesStorageWithUserId)); } binder::Status VoldNativeService::onUserRemoved(int32_t userId) { diff --git a/VoldNativeService.h b/VoldNativeService.h index 37a988b..12a93f5 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -38,7 +38,7 @@ class VoldNativeService : public BinderService, public os::Bn binder::Status shutdown(); binder::Status abortFuse(); - binder::Status onUserAdded(int32_t userId, int32_t userSerial); + binder::Status onUserAdded(int32_t userId, int32_t userSerial, int32_t sharesStorageWithUserId); binder::Status onUserRemoved(int32_t userId); binder::Status onUserStarted(int32_t userId); binder::Status onUserStopped(int32_t userId); diff --git a/VolumeManager.cpp b/VolumeManager.cpp index 5cef239..e29b920 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -81,6 +81,7 @@ using android::vold::CreateDir; using android::vold::DeleteDirContents; using android::vold::DeleteDirContentsAndDir; using android::vold::EnsureDirExists; +using android::vold::GetFuseMountPathForUser; using android::vold::IsFilesystemSupported; using android::vold::IsSdcardfsUsed; using android::vold::IsVirtioBlkDevice; @@ -424,10 +425,21 @@ void VolumeManager::createEmulatedVolumesForUser(userid_t userId) { } } -int VolumeManager::onUserAdded(userid_t userId, int userSerialNumber) { +userid_t VolumeManager::getSharedStorageUser(userid_t userId) { + if (mSharedStorageUser.find(userId) == mSharedStorageUser.end()) { + return USER_UNKNOWN; + } + return mSharedStorageUser.at(userId); +} + +int VolumeManager::onUserAdded(userid_t userId, int userSerialNumber, + userid_t sharesStorageWithUserId) { LOG(INFO) << "onUserAdded: " << userId; mAddedUsers[userId] = userSerialNumber; + if (sharesStorageWithUserId != USER_UNKNOWN) { + mSharedStorageUser[userId] = sharesStorageWithUserId; + } return 0; } @@ -436,6 +448,7 @@ int VolumeManager::onUserRemoved(userid_t userId) { onUserStopped(userId); mAddedUsers.erase(userId); + mSharedStorageUser.erase(userId); return 0; } @@ -914,6 +927,7 @@ int VolumeManager::reset() { updateVirtualDisk(); mAddedUsers.clear(); mStartedUsers.clear(); + mSharedStorageUser.clear(); // Abort all FUSE connections to avoid deadlocks if the FUSE daemon was killed // with FUSE fds open. diff --git a/VolumeManager.h b/VolumeManager.h index a8117c9..943a144 100644 --- a/VolumeManager.h +++ b/VolumeManager.h @@ -104,9 +104,11 @@ class VolumeManager { const std::set& getStartedUsers() const { return mStartedUsers; } + userid_t getSharedStorageUser(userid_t userId); + int forgetPartition(const std::string& partGuid, const std::string& fsUuid); - int onUserAdded(userid_t userId, int userSerialNumber); + int onUserAdded(userid_t userId, int userSerialNumber, userid_t cloneParentUserId); int onUserRemoved(userid_t userId); int onUserStarted(userid_t userId); int onUserStopped(userid_t userId); @@ -225,6 +227,8 @@ class VolumeManager { std::list> mInternalEmulatedVolumes; std::unordered_map mAddedUsers; + // Map of users to a user with which they can share storage (eg clone profiles) + std::unordered_map mSharedStorageUser; // This needs to be a regular set because we care about the ordering here; // user 0 should always go first, because it is responsible for sdcardfs. std::set mStartedUsers; diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index 77478d9..c798959 100644 --- a/binder/android/os/IVold.aidl +++ b/binder/android/os/IVold.aidl @@ -30,7 +30,7 @@ interface IVold { void reset(); void shutdown(); - void onUserAdded(int userId, int userSerial); + void onUserAdded(int userId, int userSerial, int sharesStorageWithUserId); void onUserRemoved(int userId); void onUserStarted(int userId); void onUserStopped(int userId); diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp index 270d097..f99a369 100644 --- a/model/EmulatedVolume.cpp +++ b/model/EmulatedVolume.cpp @@ -18,6 +18,7 @@ #include "AppFuseUtil.h" #include "Utils.h" +#include "VolumeBase.h" #include "VolumeManager.h" #include @@ -68,7 +69,7 @@ EmulatedVolume::EmulatedVolume(const std::string& rawPath, dev_t device, const s EmulatedVolume::~EmulatedVolume() {} -std::string EmulatedVolume::getLabel() { +std::string EmulatedVolume::getLabel() const { // We could have migrated storage to an adopted private volume, so always // call primary storage "emulated" to avoid media rescans. if (getMountFlags() & MountFlags::kPrimary) { @@ -91,6 +92,29 @@ static status_t doFuseBindMount(const std::string& source, const std::string& ta return OK; } +// Bind mounts the volume 'volume' onto this volume. +status_t EmulatedVolume::bindMountVolume(const EmulatedVolume& volume, + std::list& pathsToUnmount) { + int myUserId = getMountUserId(); + int volumeUserId = volume.getMountUserId(); + std::string label = volume.getLabel(); + + // eg /mnt/user/10/emulated/10 + std::string srcUserPath = GetFuseMountPathForUser(volumeUserId, label); + std::string srcPath = StringPrintf("%s/%d", srcUserPath.c_str(), volumeUserId); + // eg /mnt/user/0/emulated/10 + std::string dstUserPath = GetFuseMountPathForUser(myUserId, label); + std::string dstPath = StringPrintf("%s/%d", dstUserPath.c_str(), volumeUserId); + + auto status = doFuseBindMount(srcPath, dstPath, pathsToUnmount); + if (status == OK) { + // Store the mount path, so we can unmount it when this volume goes away + mSharedStorageMountPath = dstPath; + } + + return status; +} + status_t EmulatedVolume::mountFuseBindMounts() { std::string androidSource; std::string label = getLabel(); @@ -152,6 +176,59 @@ status_t EmulatedVolume::mountFuseBindMounts() { } } + // For users that share their volume with another user (eg a clone + // profile), the current mount setup can cause page cache inconsistency + // issues. Let's say this is user 10, and the user it shares storage with + // is user 0. + // Then: + // * The FUSE daemon for user 0 serves /mnt/user/0 + // * The FUSE daemon for user 10 serves /mnt/user/10 + // The emulated volume for user 10 would be located at two paths: + // /mnt/user/0/emulated/10 + // /mnt/user/10/emulated/10 + // Since these paths refer to the same files but are served by different FUSE + // daemons, this can result in page cache inconsistency issues. To prevent this, + // bind mount the relevant paths for the involved users: + // 1. /mnt/user/10/emulated/10 =B=> /mnt/user/0/emulated/10 + // 2. /mnt/user/0/emulated/0 =B=> /mnt/user/10/emulated/0 + // + // This will ensure that any access to the volume for a specific user always + // goes through a single FUSE daemon. + userid_t sharedStorageUserId = VolumeManager::Instance()->getSharedStorageUser(userId); + if (sharedStorageUserId != USER_UNKNOWN) { + auto filter_fn = [&](const VolumeBase& vol) { + if (vol.getState() != VolumeBase::State::kMounted) { + // The volume must be mounted + return false; + } + if (vol.getType() != VolumeBase::Type::kEmulated) { + return false; + } + if (vol.getMountUserId() != sharedStorageUserId) { + return false; + } + if ((vol.getMountFlags() & MountFlags::kPrimary) == 0) { + // We only care about the primary emulated volume, so not a private + // volume with an emulated volume stacked on top. + return false; + } + return true; + }; + auto vol = VolumeManager::Instance()->findVolumeWithFilter(filter_fn); + if (vol != nullptr) { + auto sharedVol = static_cast(vol.get()); + // Bind mount this volume in the other user's primary volume + status = sharedVol->bindMountVolume(*this, pathsToUnmount); + if (status != OK) { + return status; + } + // And vice-versa + status = bindMountVolume(*sharedVol, pathsToUnmount); + if (status != OK) { + return status; + } + } + } unmount_guard.Disable(); return OK; } @@ -160,6 +237,14 @@ status_t EmulatedVolume::unmountFuseBindMounts() { std::string label = getLabel(); int userId = getMountUserId(); + if (!mSharedStorageMountPath.empty()) { + LOG(INFO) << "Unmounting " << mSharedStorageMountPath; + auto status = UnmountTree(mSharedStorageMountPath); + if (status != OK) { + LOG(ERROR) << "Failed to unmount " << mSharedStorageMountPath; + } + mSharedStorageMountPath = ""; + } if (mUseSdcardFs || mAppDataIsolationEnabled) { std::string installerTarget( StringPrintf("/mnt/installer/%d/%s/%d/Android/obb", userId, label.c_str(), userId)); diff --git a/model/EmulatedVolume.h b/model/EmulatedVolume.h index 0f39fbd..50fda14 100644 --- a/model/EmulatedVolume.h +++ b/model/EmulatedVolume.h @@ -52,7 +52,9 @@ class EmulatedVolume : public VolumeBase { status_t mountFuseBindMounts(); status_t unmountFuseBindMounts(); - std::string getLabel(); + status_t bindMountVolume(const EmulatedVolume& vol, std::list& pathsToUnmount); + + std::string getLabel() const; std::string mRawPath; std::string mLabel; @@ -73,6 +75,9 @@ class EmulatedVolume : public VolumeBase { /* Whether to use app data isolation is enabled tor this volume */ bool mAppDataIsolationEnabled; + /* Location of bind mount for another profile that shares storage with us */ + std::string mSharedStorageMountPath = ""; + DISALLOW_COPY_AND_ASSIGN(EmulatedVolume); };