From 449a7d8ae07787dbed5292e77979e8754978c804 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Mon, 16 Mar 2020 14:37:33 +0100 Subject: [PATCH] Bind mount Android/data and Android/obb individually. Because we want all other paths (in particular Android/media) to go through FUSE. Also use scope_guard to make unwinding some failures easier. Bug: 151272568 Test: atest AdoptableHostTest Change-Id: Ib487b9071b5b212c7bb12ce54f80c96d98acaef5 --- model/EmulatedVolume.cpp | 168 +++++++++++++++++++++++++-------------- model/EmulatedVolume.h | 4 +- 2 files changed, 110 insertions(+), 62 deletions(-) diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp index b212c0e..e411b33 100644 --- a/model/EmulatedVolume.cpp +++ b/model/EmulatedVolume.cpp @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -48,7 +49,6 @@ EmulatedVolume::EmulatedVolume(const std::string& rawPath, int userId) mRawPath = rawPath; mLabel = "emulated"; mFuseMounted = false; - mAndroidMounted = false; mUseSdcardFs = IsFilesystemSupported("sdcardfs"); mAppDataIsolationEnabled = base::GetBoolProperty(kVoldAppDataIsolationEnabled, false); } @@ -60,7 +60,6 @@ EmulatedVolume::EmulatedVolume(const std::string& rawPath, dev_t device, const s mRawPath = rawPath; mLabel = fsUuid; mFuseMounted = false; - mAndroidMounted = false; mUseSdcardFs = IsFilesystemSupported("sdcardfs"); mAppDataIsolationEnabled = base::GetBoolProperty(kVoldAppDataIsolationEnabled, false); } @@ -78,22 +77,37 @@ std::string EmulatedVolume::getLabel() { } // Creates a bind mount from source to target -static status_t doFuseBindMount(const std::string& source, const std::string& target) { +static status_t doFuseBindMount(const std::string& source, const std::string& target, + std::list& pathsToUnmount) { LOG(INFO) << "Bind mounting " << source << " on " << target; auto status = BindMount(source, target); if (status != OK) { return status; } LOG(INFO) << "Bind mounted " << source << " on " << target; + pathsToUnmount.push_front(target); return OK; } status_t EmulatedVolume::mountFuseBindMounts() { - CHECK(!mAndroidMounted); - std::string androidSource; std::string label = getLabel(); int userId = getMountUserId(); + std::list pathsToUnmount; + + auto unmounter = [&]() { + LOG(INFO) << "mountFuseBindMounts() unmount scope_guard running"; + for (const auto& path : pathsToUnmount) { + LOG(INFO) << "Unmounting " << path; + auto status = UnmountTree(path); + if (status != OK) { + LOG(INFO) << "Failed to unmount " << path; + } else { + LOG(INFO) << "Unmounted " << path; + } + } + }; + auto unmount_guard = android::base::make_scope_guard(unmounter); if (mUseSdcardFs) { androidSource = StringPrintf("/mnt/runtime/default/%s/%d/Android", label.c_str(), userId); @@ -105,38 +119,43 @@ status_t EmulatedVolume::mountFuseBindMounts() { // When app data isolation is enabled, obb/ will be mounted per app, otherwise we should // bind mount the whole Android/ to speed up reading. if (!mAppDataIsolationEnabled) { - std::string androidTarget( - StringPrintf("/mnt/user/%d/%s/%d/Android", userId, label.c_str(), userId)); - status = doFuseBindMount(androidSource, androidTarget); - } + std::string androidDataSource = StringPrintf("%s/data", androidSource.c_str()); + std::string androidDataTarget( + StringPrintf("/mnt/user/%d/%s/%d/Android/data", userId, label.c_str(), userId)); + status = doFuseBindMount(androidDataSource, androidDataTarget, pathsToUnmount); + if (status != OK) { + return status; + } - if (status != OK) { - return status; + std::string androidObbSource = StringPrintf("%s/obb", androidSource.c_str()); + std::string androidObbTarget( + StringPrintf("/mnt/user/%d/%s/%d/Android/obb", userId, label.c_str(), userId)); + status = doFuseBindMount(androidObbSource, androidObbTarget, pathsToUnmount); + if (status != OK) { + return status; + } } - mAndroidMounted = true; // Installers get the same view as all other apps, with the sole exception that the // OBB dirs (Android/obb) are writable to them. On sdcardfs devices, this requires // a special bind mount, since app-private and OBB dirs share the same GID, but we // only want to give access to the latter. - if (!mUseSdcardFs) { - return OK; - } - std::string installerSource( - StringPrintf("/mnt/runtime/write/%s/%d/Android/obb", label.c_str(), userId)); - std::string installerTarget( - StringPrintf("/mnt/installer/%d/%s/%d/Android/obb", userId, label.c_str(), userId)); + if (mUseSdcardFs) { + std::string installerSource( + StringPrintf("/mnt/runtime/write/%s/%d/Android/obb", label.c_str(), userId)); + std::string installerTarget( + StringPrintf("/mnt/installer/%d/%s/%d/Android/obb", userId, label.c_str(), userId)); - status = doFuseBindMount(installerSource, installerTarget); - if (status != OK) { - return status; + status = doFuseBindMount(installerSource, installerTarget, pathsToUnmount); + if (status != OK) { + return status; + } } + unmount_guard.Disable(); return OK; } status_t EmulatedVolume::unmountFuseBindMounts() { - CHECK(mAndroidMounted); - std::string label = getLabel(); int userId = getMountUserId(); @@ -156,19 +175,54 @@ status_t EmulatedVolume::unmountFuseBindMounts() { std::string appObbDir(StringPrintf("%s/%d/Android/obb", getPath().c_str(), userId)); KillProcessesWithMountPrefix(appObbDir); } else { - std::string androidTarget( - StringPrintf("/mnt/user/%d/%s/%d/Android", userId, label.c_str(), userId)); + std::string androidDataTarget( + StringPrintf("/mnt/user/%d/%s/%d/Android/data", userId, label.c_str(), userId)); + + LOG(INFO) << "Unmounting " << androidDataTarget; + auto status = UnmountTree(androidDataTarget); + if (status != OK) { + return status; + } + LOG(INFO) << "Unmounted " << androidDataTarget; + + std::string androidObbTarget( + StringPrintf("/mnt/user/%d/%s/%d/Android/obb", userId, label.c_str(), userId)); - LOG(INFO) << "Unmounting " << androidTarget; - auto status = UnmountTree(androidTarget); + LOG(INFO) << "Unmounting " << androidObbTarget; + status = UnmountTree(androidObbTarget); if (status != OK) { return status; } - LOG(INFO) << "Unmounted " << androidTarget; + LOG(INFO) << "Unmounted " << androidObbTarget; } return OK; } +status_t EmulatedVolume::unmountSdcardFs() { + if (!mUseSdcardFs || getMountUserId() != 0) { + // For sdcardfs, only unmount for user 0, since user 0 will always be running + // and the paths don't change for different users. + return OK; + } + + ForceUnmount(mSdcardFsDefault); + ForceUnmount(mSdcardFsRead); + ForceUnmount(mSdcardFsWrite); + ForceUnmount(mSdcardFsFull); + + rmdir(mSdcardFsDefault.c_str()); + rmdir(mSdcardFsRead.c_str()); + rmdir(mSdcardFsWrite.c_str()); + rmdir(mSdcardFsFull.c_str()); + + mSdcardFsDefault.clear(); + mSdcardFsRead.clear(); + mSdcardFsWrite.clear(); + mSdcardFsFull.clear(); + + return OK; +} + status_t EmulatedVolume::doMount() { std::string label = getLabel(); bool isVisible = getMountFlags() & MountFlags::kVisible; @@ -239,7 +293,15 @@ status_t EmulatedVolume::doMount() { TEMP_FAILURE_RETRY(waitpid(sdcardFsPid, nullptr, 0)); sdcardFsPid = 0; } + if (isFuse && isVisible) { + // Make sure we unmount sdcardfs if we bail out with an error below + auto sdcardfs_unmounter = [&]() { + LOG(INFO) << "sdcardfs_unmounter scope_guard running"; + unmountSdcardFs(); + }; + auto sdcardfs_guard = android::base::make_scope_guard(sdcardfs_unmounter); + LOG(INFO) << "Mounting emulated fuse volume"; android::base::unique_fd fd; int user_id = getMountUserId(); @@ -259,13 +321,21 @@ status_t EmulatedVolume::doMount() { } mFuseMounted = true; + auto fuse_unmounter = [&]() { + LOG(INFO) << "fuse_unmounter scope_guard running"; + fd.reset(); + if (UnmountUserFuse(user_id, getInternalPath(), label) != OK) { + PLOG(INFO) << "UnmountUserFuse failed on emulated fuse volume"; + } + mFuseMounted = false; + }; + auto fuse_guard = android::base::make_scope_guard(fuse_unmounter); + auto callback = getMountCallback(); if (callback) { bool is_ready = false; callback->onVolumeChecking(std::move(fd), getPath(), getInternalPath(), &is_ready); if (!is_ready) { - fd.reset(); - doUnmount(); return -EIO; } } @@ -273,10 +343,12 @@ status_t EmulatedVolume::doMount() { // Only do the bind-mounts when we know for sure the FUSE daemon can resolve the path. res = mountFuseBindMounts(); if (res != OK) { - fd.reset(); - doUnmount(); + return res; } - return res; + + // All mounts where successful, disable scope guards + sdcardfs_guard.Disable(); + fuse_guard.Disable(); } return OK; @@ -304,10 +376,8 @@ status_t EmulatedVolume::doUnmount() { // Ignoring unmount return status because we do want to try to unmount // the rest cleanly. - if (mAndroidMounted) { - unmountFuseBindMounts(); - mAndroidMounted = false; - } + unmountFuseBindMounts(); + if (UnmountUserFuse(userId, getInternalPath(), label) != OK) { PLOG(INFO) << "UnmountUserFuse failed on emulated fuse volume"; return -errno; @@ -315,28 +385,8 @@ status_t EmulatedVolume::doUnmount() { mFuseMounted = false; } - if (getMountUserId() != 0 || !mUseSdcardFs) { - // For sdcardfs, only unmount for user 0, since user 0 will always be running - // and the paths don't change for different users. - return OK; - } - - ForceUnmount(mSdcardFsDefault); - ForceUnmount(mSdcardFsRead); - ForceUnmount(mSdcardFsWrite); - ForceUnmount(mSdcardFsFull); - - rmdir(mSdcardFsDefault.c_str()); - rmdir(mSdcardFsRead.c_str()); - rmdir(mSdcardFsWrite.c_str()); - rmdir(mSdcardFsFull.c_str()); - - mSdcardFsDefault.clear(); - mSdcardFsRead.clear(); - mSdcardFsWrite.clear(); - mSdcardFsFull.clear(); - return OK; + return unmountSdcardFs(); } std::string EmulatedVolume::getRootPath() const { diff --git a/model/EmulatedVolume.h b/model/EmulatedVolume.h index 9bff0ca..1d2385d 100644 --- a/model/EmulatedVolume.h +++ b/model/EmulatedVolume.h @@ -48,6 +48,7 @@ class EmulatedVolume : public VolumeBase { status_t doUnmount() override; private: + status_t unmountSdcardFs(); status_t mountFuseBindMounts(); status_t unmountFuseBindMounts(); @@ -63,9 +64,6 @@ class EmulatedVolume : public VolumeBase { /* Whether we mounted FUSE for this volume */ bool mFuseMounted; - /* Whether we mounted Android/ for this volume */ - bool mAndroidMounted; - /* Whether to use sdcardfs for this volume */ bool mUseSdcardFs;