From cf5916f3fa9e5eabc143dfb3669198e9ba3c3634 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Fri, 3 Jan 2020 14:36:45 +0100 Subject: [PATCH 1/2] Also delay creating found disks until user 0 is started. Public and private volumes can be discovered before user 0 is up and running; when using FUSE however, we can't mount these disks yet, because we depend on the user to become unlocked before we can start the FUSE daemon (which is the MediaProvider application process). So besides waiting for any secure keyguard to be dismissed, also wait for user 0 to be started. Bug: 146419093 Test: Boot cuttlefish with a fake public volume; is available after repeated boots. Change-Id: I06fe4d336d1baec3a49886c3cf12d844a1d0eb26 --- VolumeManager.cpp | 24 +++++++++++++++++++----- VolumeManager.h | 1 + 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/VolumeManager.cpp b/VolumeManager.cpp index bc843b4..4d850fb 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -264,10 +264,17 @@ void VolumeManager::handleBlockEvent(NetlinkEvent* evt) { void VolumeManager::handleDiskAdded(const std::shared_ptr& disk) { // For security reasons, if secure keyguard is showing, wait // until the user unlocks the device to actually touch it + // Additionally, wait until user 0 is actually started, since we need + // the user to be up before we can mount a FUSE daemon to handle the disk. + bool userZeroStarted = mStartedUsers.find(0) != mStartedUsers.end(); if (mSecureKeyguardShowing) { LOG(INFO) << "Found disk at " << disk->getEventPath() << " but delaying scan due to secure keyguard"; mPendingDisks.push_back(disk); + } else if (!userZeroStarted) { + LOG(INFO) << "Found disk at " << disk->getEventPath() + << " but delaying scan due to user zero not having started"; + mPendingDisks.push_back(disk); } else { disk->create(); mDisks.push_back(disk); @@ -482,6 +489,8 @@ int VolumeManager::onUserStarted(userid_t userId) { } mStartedUsers.insert(userId); + + createPendingDisksIfNeeded(); return 0; } @@ -496,17 +505,22 @@ int VolumeManager::onUserStopped(userid_t userId) { return 0; } -int VolumeManager::onSecureKeyguardStateChanged(bool isShowing) { - mSecureKeyguardShowing = isShowing; - if (!mSecureKeyguardShowing) { - // Now that secure keyguard has been dismissed, process - // any pending disks +void VolumeManager::createPendingDisksIfNeeded() { + bool userZeroStarted = mStartedUsers.find(0) != mStartedUsers.end(); + if (!mSecureKeyguardShowing && userZeroStarted) { + // Now that secure keyguard has been dismissed and user 0 has + // started, process any pending disks for (const auto& disk : mPendingDisks) { disk->create(); mDisks.push_back(disk); } mPendingDisks.clear(); } +} + +int VolumeManager::onSecureKeyguardStateChanged(bool isShowing) { + mSecureKeyguardShowing = isShowing; + createPendingDisksIfNeeded(); return 0; } diff --git a/VolumeManager.h b/VolumeManager.h index db32ecd..cacab85 100644 --- a/VolumeManager.h +++ b/VolumeManager.h @@ -94,6 +94,7 @@ class VolumeManager { int onUserStarted(userid_t userId); int onUserStopped(userid_t userId); + void createPendingDisksIfNeeded(); int onSecureKeyguardStateChanged(bool isShowing); int setPrimary(const std::shared_ptr& vol); From 86f21a2211d23bcf87953e7129bec601a6dce2c9 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Mon, 6 Jan 2020 09:48:14 +0100 Subject: [PATCH 2/2] Conditionally use sdcardfs. In preparation of sdcardfs going away on devices launching with R, conditionally use it. Bug: 146419093 Test: cuttlefish with sdcardfs, cuttlefish without sdcardfs but with FUSE Change-Id: I2c1d4b428dcb43c3fd274dde84d5088984161993 --- Utils.cpp | 15 ++-- model/EmulatedVolume.cpp | 32 ++++++--- model/EmulatedVolume.h | 6 ++ model/PublicVolume.cpp | 143 ++++++++++++++++++++------------------- model/PublicVolume.h | 3 + 5 files changed, 114 insertions(+), 85 deletions(-) diff --git a/Utils.cpp b/Utils.cpp index 67c48ad..0375765 100644 --- a/Utils.cpp +++ b/Utils.cpp @@ -1017,9 +1017,6 @@ status_t MountUserFuse(userid_t user_id, const std::string& absolute_lower_path, std::string pass_through_path( StringPrintf("%s/%s", pre_pass_through_path.c_str(), relative_upper_path.c_str())); - std::string sdcardfs_path( - StringPrintf("/mnt/runtime/full/%s", relative_upper_path.c_str())); - // Create directories. auto result = PrepareDir(pre_fuse_path, 0700, AID_ROOT, AID_ROOT); if (result != android::OK) { @@ -1081,8 +1078,16 @@ status_t MountUserFuse(userid_t user_id, const std::string& absolute_lower_path, return -errno; } - LOG(INFO) << "Bind mounting " << sdcardfs_path << " to " << pass_through_path; - return BindMount(sdcardfs_path, pass_through_path); + if (IsFilesystemSupported("sdcardfs")) { + std::string sdcardfs_path( + StringPrintf("/mnt/runtime/full/%s", relative_upper_path.c_str())); + + LOG(INFO) << "Bind mounting " << sdcardfs_path << " to " << pass_through_path; + return BindMount(sdcardfs_path, pass_through_path); + } else { + LOG(INFO) << "Bind mounting " << absolute_lower_path << " to " << pass_through_path; + return BindMount(absolute_lower_path, pass_through_path); + } } status_t UnmountUserFuse(userid_t user_id, const std::string& absolute_lower_path, diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp index aef7b77..d7b22e3 100644 --- a/model/EmulatedVolume.cpp +++ b/model/EmulatedVolume.cpp @@ -48,6 +48,7 @@ EmulatedVolume::EmulatedVolume(const std::string& rawPath, int userId) mRawPath = rawPath; mLabel = "emulated"; mFuseMounted = false; + mUseSdcardFs = IsFilesystemSupported("sdcardfs"); } EmulatedVolume::EmulatedVolume(const std::string& rawPath, dev_t device, const std::string& fsUuid, @@ -57,6 +58,7 @@ EmulatedVolume::EmulatedVolume(const std::string& rawPath, dev_t device, const s mRawPath = rawPath; mLabel = fsUuid; mFuseMounted = false; + mUseSdcardFs = IsFilesystemSupported("sdcardfs"); } EmulatedVolume::~EmulatedVolume() {} @@ -71,18 +73,23 @@ std::string EmulatedVolume::getLabel() { } } -static status_t mountFuseBindMounts(int userId, const std::string& label) { - // TODO(b/134706060) we don't actually want to mount the "write" view by - // default, since it gives write access to all OBB dirs. - std::string androidSource( - StringPrintf("/mnt/runtime/default/%s/%d/Android", label.c_str(), userId)); +status_t EmulatedVolume::mountFuseBindMounts() { + std::string androidSource; + std::string label = getLabel(); + int userId = getMountUserId(); + + if (mUseSdcardFs) { + androidSource = StringPrintf("/mnt/runtime/default/%s/%d/Android", label.c_str(), userId); + } else { + androidSource = StringPrintf("/%s/%d/Android", mRawPath.c_str(), userId); + } std::string androidTarget( StringPrintf("/mnt/user/%d/%s/%d/Android", userId, label.c_str(), userId)); if (access(androidSource.c_str(), F_OK) != 0) { // Android path may not exist yet if users has just been created; create it on // the lower fs. - if (fs_prepare_dir(androidSource.c_str(), 0771, AID_ROOT, AID_ROOT) != 0) { + if (fs_prepare_dir(androidSource.c_str(), 0771, AID_MEDIA_RW, AID_MEDIA_RW) != 0) { PLOG(ERROR) << "Failed to create " << androidSource; return -errno; } @@ -97,7 +104,10 @@ static status_t mountFuseBindMounts(int userId, const std::string& label) { return OK; } -static status_t unmountFuseBindMounts(int userId, const std::string& label) { +status_t EmulatedVolume::unmountFuseBindMounts() { + std::string label = getLabel(); + int userId = getMountUserId(); + std::string androidTarget( StringPrintf("/mnt/user/%d/%s/%d/Android", userId, label.c_str(), userId)); @@ -137,7 +147,7 @@ status_t EmulatedVolume::doMount() { // Mount sdcardfs regardless of FUSE, since we need it to bind-mount on top of the // FUSE volume for various reasons. - if (getMountUserId() == 0) { + if (mUseSdcardFs && getMountUserId() == 0) { LOG(INFO) << "Executing sdcardfs"; int sdcardFsPid; if (!(sdcardFsPid = fork())) { @@ -203,7 +213,7 @@ status_t EmulatedVolume::doMount() { } // Only do the bind-mounts when we know for sure the FUSE daemon can resolve the path. - return mountFuseBindMounts(user_id, label); + return mountFuseBindMounts(); } return OK; @@ -231,7 +241,7 @@ status_t EmulatedVolume::doUnmount() { // Ignoring unmount return status because we do want to try to unmount // the rest cleanly. - unmountFuseBindMounts(userId, label); + unmountFuseBindMounts(); if (UnmountUserFuse(userId, getInternalPath(), label) != OK) { PLOG(INFO) << "UnmountUserFuse failed on emulated fuse volume"; return -errno; @@ -239,7 +249,7 @@ status_t EmulatedVolume::doUnmount() { mFuseMounted = false; } - if (getMountUserId() != 0) { + 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; diff --git a/model/EmulatedVolume.h b/model/EmulatedVolume.h index 131761c..4f76a60 100644 --- a/model/EmulatedVolume.h +++ b/model/EmulatedVolume.h @@ -46,6 +46,9 @@ class EmulatedVolume : public VolumeBase { status_t doUnmount() override; private: + status_t mountFuseBindMounts(); + status_t unmountFuseBindMounts(); + std::string getLabel(); std::string mRawPath; std::string mLabel; @@ -58,6 +61,9 @@ class EmulatedVolume : public VolumeBase { /* Whether we mounted FUSE for this volume */ bool mFuseMounted; + /* Whether to use sdcardfs for this volume */ + bool mUseSdcardFs; + DISALLOW_COPY_AND_ASSIGN(EmulatedVolume); }; diff --git a/model/PublicVolume.cpp b/model/PublicVolume.cpp index 78f150d..b246c95 100644 --- a/model/PublicVolume.cpp +++ b/model/PublicVolume.cpp @@ -51,6 +51,7 @@ PublicVolume::PublicVolume(dev_t device) : VolumeBase(Type::kPublic), mDevice(de setId(StringPrintf("public:%u,%u", major(device), minor(device))); mDevPath = StringPrintf("/dev/block/vold/%s", getId().c_str()); mFuseMounted = false; + mUseSdcardFs = IsFilesystemSupported("sdcardfs"); } PublicVolume::~PublicVolume() {} @@ -161,67 +162,69 @@ status_t PublicVolume::doMount() { return OK; } - if (fs_prepare_dir(mSdcardFsDefault.c_str(), 0700, AID_ROOT, AID_ROOT) || - fs_prepare_dir(mSdcardFsRead.c_str(), 0700, AID_ROOT, AID_ROOT) || - fs_prepare_dir(mSdcardFsWrite.c_str(), 0700, AID_ROOT, AID_ROOT) || - fs_prepare_dir(mSdcardFsFull.c_str(), 0700, AID_ROOT, AID_ROOT)) { - PLOG(ERROR) << getId() << " failed to create sdcardfs mount points"; - return -errno; - } - - dev_t before = GetDevice(mSdcardFsFull); - - int sdcardFsPid; - if (!(sdcardFsPid = fork())) { - if (getMountFlags() & MountFlags::kPrimary) { - // clang-format off - if (execl(kSdcardFsPath, kSdcardFsPath, - "-u", "1023", // AID_MEDIA_RW - "-g", "1023", // AID_MEDIA_RW - "-U", std::to_string(getMountUserId()).c_str(), - "-w", - mRawPath.c_str(), - stableName.c_str(), - NULL)) { - // clang-format on - PLOG(ERROR) << "Failed to exec"; - } - } else { - // clang-format off - if (execl(kSdcardFsPath, kSdcardFsPath, - "-u", "1023", // AID_MEDIA_RW - "-g", "1023", // AID_MEDIA_RW - "-U", std::to_string(getMountUserId()).c_str(), - mRawPath.c_str(), - stableName.c_str(), - NULL)) { - // clang-format on - PLOG(ERROR) << "Failed to exec"; - } + if (mUseSdcardFs) { + if (fs_prepare_dir(mSdcardFsDefault.c_str(), 0700, AID_ROOT, AID_ROOT) || + fs_prepare_dir(mSdcardFsRead.c_str(), 0700, AID_ROOT, AID_ROOT) || + fs_prepare_dir(mSdcardFsWrite.c_str(), 0700, AID_ROOT, AID_ROOT) || + fs_prepare_dir(mSdcardFsFull.c_str(), 0700, AID_ROOT, AID_ROOT)) { + PLOG(ERROR) << getId() << " failed to create sdcardfs mount points"; + return -errno; } - LOG(ERROR) << "sdcardfs exiting"; - _exit(1); - } + dev_t before = GetDevice(mSdcardFsFull); - if (sdcardFsPid == -1) { - PLOG(ERROR) << getId() << " failed to fork"; - return -errno; - } + int sdcardFsPid; + if (!(sdcardFsPid = fork())) { + if (getMountFlags() & MountFlags::kPrimary) { + // clang-format off + if (execl(kSdcardFsPath, kSdcardFsPath, + "-u", "1023", // AID_MEDIA_RW + "-g", "1023", // AID_MEDIA_RW + "-U", std::to_string(getMountUserId()).c_str(), + "-w", + mRawPath.c_str(), + stableName.c_str(), + NULL)) { + // clang-format on + PLOG(ERROR) << "Failed to exec"; + } + } else { + // clang-format off + if (execl(kSdcardFsPath, kSdcardFsPath, + "-u", "1023", // AID_MEDIA_RW + "-g", "1023", // AID_MEDIA_RW + "-U", std::to_string(getMountUserId()).c_str(), + mRawPath.c_str(), + stableName.c_str(), + NULL)) { + // clang-format on + PLOG(ERROR) << "Failed to exec"; + } + } - nsecs_t start = systemTime(SYSTEM_TIME_BOOTTIME); - while (before == GetDevice(mSdcardFsFull)) { - LOG(DEBUG) << "Waiting for sdcardfs to spin up..."; - usleep(50000); // 50ms - - nsecs_t now = systemTime(SYSTEM_TIME_BOOTTIME); - if (nanoseconds_to_milliseconds(now - start) > 5000) { - LOG(WARNING) << "Timed out while waiting for sdcardfs to spin up"; - return -ETIMEDOUT; + LOG(ERROR) << "sdcardfs exiting"; + _exit(1); } + + if (sdcardFsPid == -1) { + PLOG(ERROR) << getId() << " failed to fork"; + return -errno; + } + + nsecs_t start = systemTime(SYSTEM_TIME_BOOTTIME); + while (before == GetDevice(mSdcardFsFull)) { + LOG(DEBUG) << "Waiting for sdcardfs to spin up..."; + usleep(50000); // 50ms + + nsecs_t now = systemTime(SYSTEM_TIME_BOOTTIME); + if (nanoseconds_to_milliseconds(now - start) > 5000) { + LOG(WARNING) << "Timed out while waiting for sdcardfs to spin up"; + return -ETIMEDOUT; + } + } + /* sdcardfs will have exited already. The filesystem will still be running */ + TEMP_FAILURE_RETRY(waitpid(sdcardFsPid, nullptr, 0)); } - /* sdcardfs will have exited already. The filesystem will still be running */ - TEMP_FAILURE_RETRY(waitpid(sdcardFsPid, nullptr, 0)); bool isFuse = base::GetBoolProperty(kPropFuse, false); if (isFuse) { @@ -275,22 +278,24 @@ status_t PublicVolume::doUnmount() { ForceUnmount(kAsecPath); - ForceUnmount(mSdcardFsDefault); - ForceUnmount(mSdcardFsRead); - ForceUnmount(mSdcardFsWrite); - ForceUnmount(mSdcardFsFull); + if (mUseSdcardFs) { + 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(); + } ForceUnmount(mRawPath); - - rmdir(mSdcardFsDefault.c_str()); - rmdir(mSdcardFsRead.c_str()); - rmdir(mSdcardFsWrite.c_str()); - rmdir(mSdcardFsFull.c_str()); rmdir(mRawPath.c_str()); - - mSdcardFsDefault.clear(); - mSdcardFsRead.clear(); - mSdcardFsWrite.clear(); - mSdcardFsFull.clear(); mRawPath.clear(); return OK; diff --git a/model/PublicVolume.h b/model/PublicVolume.h index dd76373..3156b53 100644 --- a/model/PublicVolume.h +++ b/model/PublicVolume.h @@ -68,6 +68,9 @@ class PublicVolume : public VolumeBase { /* Whether we mounted FUSE for this volume */ bool mFuseMounted; + /* Whether to use sdcardfs for this volume */ + bool mUseSdcardFs; + /* Filesystem type */ std::string mFsType; /* Filesystem UUID */