From 6b1225770261cbad5cbf6509a32743691fe40783 Mon Sep 17 00:00:00 2001 From: Ricky Wai Date: Fri, 28 Feb 2020 16:30:47 +0000 Subject: [PATCH] Make storage dirs remount fork() safe Also, use the pids provided by system server to remount all existing processes, so we don't need to do the heavy and unreliable scanning in /proc anymore. Bug: 149548518 Test: atest AdoptableHostTest Change-Id: Ifb5b79a3bc5438f36e0d61ec8aec96bdbc60ca13 --- VoldNativeService.cpp | 8 + VoldNativeService.h | 2 + VolumeManager.cpp | 282 ++++++++++++----------------------- VolumeManager.h | 8 +- binder/android/os/IVold.aidl | 1 + model/EmulatedVolume.cpp | 19 --- model/EmulatedVolume.h | 1 + 7 files changed, 110 insertions(+), 211 deletions(-) diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp index 08b4661..57cee23 100644 --- a/VoldNativeService.cpp +++ b/VoldNativeService.cpp @@ -458,6 +458,14 @@ binder::Status VoldNativeService::remountUid(int32_t uid, int32_t remountMode) { return translate(VolumeManager::Instance()->remountUid(uid, remountMode)); } +binder::Status VoldNativeService::remountAppStorageDirs(int uid, int pid, + const std::vector& packageNames) { + ENFORCE_SYSTEM_OR_ROOT; + ACQUIRE_LOCK; + + return translate(VolumeManager::Instance()->remountAppStorageDirs(uid, pid, packageNames)); +} + binder::Status VoldNativeService::setupAppDir(const std::string& path, int32_t appUid) { ENFORCE_SYSTEM_OR_ROOT; CHECK_ARGUMENT_PATH(path); diff --git a/VoldNativeService.h b/VoldNativeService.h index e04c259..2f4b6eb 100644 --- a/VoldNativeService.h +++ b/VoldNativeService.h @@ -64,6 +64,8 @@ class VoldNativeService : public BinderService, public os::Bn const android::sp& listener); binder::Status remountUid(int32_t uid, int32_t remountMode); + binder::Status remountAppStorageDirs(int uid, int pid, + const std::vector& packageNames); binder::Status setupAppDir(const std::string& path, int32_t appUid); binder::Status fixupAppDir(const std::string& path, int32_t appUid); diff --git a/VolumeManager.cpp b/VolumeManager.cpp index fce977e..6a40a52 100644 --- a/VolumeManager.cpp +++ b/VolumeManager.cpp @@ -738,226 +738,142 @@ int VolumeManager::remountUid(uid_t uid, int32_t mountMode) { forkAndRemountChild, &mountMode) ? 0 : -1; } -// Bind mount obb & data dir for an app if necessary. -// How it works: -// 1). Check if a pid is an app uid and not the FuseDaemon, if not then return. -// 2). Get the mounts for that pid. -// 3). If obb is already mounted then return, otherwise we need to mount obb for this pid. -// 4). Get all packages and uid mounted for jit profile. These packages are all packages with -// same uid or whitelisted apps. -// 5a). If there's no package, it means it's not a process running app data isolation, so -// just bind mount Android/obb & Android/data dir. -// 5b). Otherwise, for each package, create obb dir if it's not created and bind mount it. -// TODO: Should we get some reliable data from system server instead of scanning /proc ? -static bool bindMountAppDataObbDir(uid_t uid, pid_t pid, int nsFd, const char* name, void* params) { - if (uid < AID_APP_START || uid > AID_APP_END) { - return true; - } - if (android::vold::IsFuseDaemon(pid)) { - return true; - } - async_safe_format_log(ANDROID_LOG_ERROR, "vold", - "Start mounting obb and data for uid:%d, pid:%d", uid, pid); - userid_t userId = multiuser_get_user_id(uid); +// Set the namespace the app process and remount its storage directories. +static bool remountStorageDirs(int nsFd, const char* sources[], const char* targets[], int size) { + // This code is executed after a fork so it's very important that the set of + // methods we call here is strictly limited. if (setns(nsFd, CLONE_NEWNS) != 0) { async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to setns %s", strerror(errno)); return false; } - std::string profiles_path(StringPrintf("/data/misc/profiles/cur/%d/", userId)); - // We search both .../obb and .../obb/$PKG paths here. - std::string obb_path(StringPrintf("/storage/emulated/%d/Android/obb", userId)); - int profiles_path_len = profiles_path.length(); - int obb_path_len = obb_path.length(); - - // TODO: Refactor the code as a util function so we can reuse the mount parsing code. - std::string mounts_file(StringPrintf("/proc/%d/mounts", pid)); - auto fp = std::unique_ptr( - setmntent(mounts_file.c_str(), "r"), endmntent); - if (!fp) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Error opening %s: %s", - mounts_file.c_str(), strerror(errno)); - return false; - } - - // Check if obb directory is mounted, and get all packages of mounted app data directory. - // We only need to check obb directory and assume if obb is mounted, data is mounted also. - bool obb_mounted = false; - std::vector pkg_name_list; - mntent* mentry; - while ((mentry = getmntent(fp.get())) != nullptr) { - if (strncmp(mentry->mnt_dir, profiles_path.c_str(), profiles_path_len) == 0) { - pkg_name_list.push_back(std::string(mentry->mnt_dir + profiles_path_len)); - } - if (strncmp(mentry->mnt_dir, obb_path.c_str(), obb_path_len) == 0) { - obb_mounted = true; + for (int i = 0; i < size; i++) { + if (TEMP_FAILURE_RETRY(mount(sources[i], targets[i], NULL, MS_BIND | MS_REC, NULL)) == -1) { + async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s to %s :%s", + sources[i], targets[i], strerror(errno)); + return false; } } + return true; +} - // Obb mounted in zygote already, so skip it - if (obb_mounted) { - return true; - } - - std::string obbSource, dataSource; +static std::string getStorageDirSrc(userid_t userId, const std::string& dirName, + const std::string& packageName) { if (IsFilesystemSupported("sdcardfs")) { - obbSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/obb", userId); - dataSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/data", userId); + return StringPrintf("/mnt/runtime/default/emulated/%d/%s/%s", + userId, dirName.c_str(), packageName.c_str()); } else { - obbSource = StringPrintf("/mnt/pass_through/%d/emulated/%d/Android/obb", userId, userId); - dataSource = StringPrintf("/mnt/pass_through/%d/emulated/%d/Android/data", userId, userId); + return StringPrintf("/mnt/pass_through/%d/emulated/%d/%s/%s", + userId, userId, dirName.c_str(), packageName.c_str()); } - std::string obbTarget(StringPrintf("/storage/emulated/%d/Android/obb", userId)); - std::string dataTarget(StringPrintf("/storage/emulated/%d/Android/data", userId)); - - // TODO: Review if these checks are still necessary - auto status = EnsureDirExists(obbSource, 0771, AID_MEDIA_RW, AID_MEDIA_RW); - if (status != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to create dir %s %s", - obbSource.c_str(), strerror(-status)); +} + +static std::string getStorageDirTarget(userid_t userId, std::string dirName, + std::string packageName) { + return StringPrintf("/storage/emulated/%d/%s/%s", + userId, dirName.c_str(), packageName.c_str()); +} + +// Fork the process and remount storage +static bool forkAndRemountStorage(int uid, int pid, const std::vector& packageNames) { + userid_t userId = multiuser_get_user_id(uid); + std::string mnt_path = StringPrintf("/proc/%d/ns/mnt", pid); + android::base::unique_fd nsFd( + TEMP_FAILURE_RETRY(open(mnt_path.c_str(), O_RDONLY | O_CLOEXEC))); + if (nsFd == -1) { + PLOG(ERROR) << "Unable to open " << mnt_path.c_str(); return false; } - status = EnsureDirExists(dataSource, 0771, AID_MEDIA_RW, AID_MEDIA_RW); - if (status != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to create dir %s %s", - dataSource.c_str(), strerror(-status)); - return false; + // Storing both Android/obb and Android/data paths. + int size = packageNames.size() * 2; + + std::unique_ptr sources(new std::string[size]); + std::unique_ptr targets(new std::string[size]); + std::unique_ptr sources_uptr(new const char*[size]); + std::unique_ptr targets_uptr(new const char*[size]); + const char** sources_cstr = sources_uptr.get(); + const char** targets_cstr = targets_uptr.get(); + + for (int i = 0; i < size; i += 2) { + std::string const& packageName = packageNames[i/2]; + sources[i] = getStorageDirSrc(userId, "Android/data", packageName); + targets[i] = getStorageDirTarget(userId, "Android/data", packageName); + sources[i+1] = getStorageDirSrc(userId, "Android/obb", packageName); + targets[i+1] = getStorageDirTarget(userId, "Android/obb", packageName); + + sources_cstr[i] = sources[i].c_str(); + targets_cstr[i] = targets[i].c_str(); + sources_cstr[i+1] = sources[i+1].c_str(); + targets_cstr[i+1] = targets[i+1].c_str(); } - // It means app data isolation is not applied to this, so we can just bind the whole obb - // directory instead. - if (pkg_name_list.empty()) { - async_safe_format_log(ANDROID_LOG_INFO, "vold", - "Bind mounting whole obb and data directory for pid %d", pid); - auto status1 = BindMount(obbSource, obbTarget); - // Still bind mount data even obb fails, just slower to access obb dir - auto status2 = BindMount(dataSource, dataTarget); - if (status1 != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s %s %s", - obbSource.c_str(), obbTarget.c_str(), strerror(-status)); + for (int i = 0; i < size; i++) { + auto status = EnsureDirExists(sources_cstr[i], 0771, AID_MEDIA_RW, AID_MEDIA_RW); + if (status != OK) { + PLOG(ERROR) << "Failed to create dir: " << sources_cstr[i]; return false; } - if (status2 != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s %s %s", - dataSource.c_str(), dataTarget.c_str(), strerror(-status)); + status = EnsureDirExists(targets_cstr[i], 0771, AID_MEDIA_RW, AID_MEDIA_RW); + if (status != OK) { + PLOG(ERROR) << "Failed to create dir: " << targets_cstr[i]; return false; } - return true; } - // Bind mount each app's obb directory - for (const auto& pkg_name : pkg_name_list) { - std::string appObbSource, appDataSource; - if (IsFilesystemSupported("sdcardfs")) { - appObbSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/obb/%s", - userId, pkg_name.c_str()); - appDataSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/data/%s", - userId, pkg_name.c_str()); + pid_t child; + // Fork a child to mount Android/obb android Android/data dirs, as we don't want it to affect + // original vold process mount namespace. + if (!(child = fork())) { + if (remountStorageDirs(nsFd, sources_cstr, targets_cstr, size)) { + _exit(0); } else { - appObbSource = StringPrintf("/mnt/pass_through/%d/emulated/%d/Android/obb/%s", - userId, userId, pkg_name.c_str()); - appDataSource = StringPrintf("/mnt/pass_through/%d/emulated/%d/Android/data/%s", - userId, userId, pkg_name.c_str()); + _exit(1); } - std::string appObbTarget(StringPrintf("/storage/emulated/%d/Android/obb/%s", - userId, pkg_name.c_str())); - std::string appDataTarget(StringPrintf("/storage/emulated/%d/Android/data/%s", - userId, pkg_name.c_str())); + } - status = EnsureDirExists(appObbSource, 0770, uid, AID_MEDIA_RW); - if (status != OK) { - async_safe_format_log(ANDROID_LOG_INFO, "vold", "Failed to ensure dir %s exists", - appObbSource.c_str()); - continue; - } - status = EnsureDirExists(appDataSource, 0770, uid, AID_MEDIA_RW); - if (status != OK) { - async_safe_format_log(ANDROID_LOG_INFO, "vold", "Failed to ensure dir %s exists", - appDataSource.c_str()); - continue; + if (child == -1) { + PLOG(ERROR) << "Failed to fork"; + return false; + } else { + int status; + if (TEMP_FAILURE_RETRY(waitpid(child, &status, 0)) == -1) { + PLOG(ERROR) << "Failed to waitpid: " << child; + return false; } - async_safe_format_log(ANDROID_LOG_INFO, "vold", - "Bind mounting app obb and data directory(%s) for pid %d", - pkg_name.c_str(), pid); - auto status1 = BindMount(appObbSource, appObbTarget); - // Still bind mount data even obb fails, just slower to access obb dir - auto status2 = BindMount(appDataSource, appDataTarget); - if (status1 != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s %s %s", - obbSource.c_str(), obbTarget.c_str(), strerror(-status)); - continue; + if (!WIFEXITED(status)) { + PLOG(ERROR) << "Process did not exit normally, status: " << status; + return false; } - if (status2 != OK) { - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s %s %s", - appDataSource.c_str(), appDataTarget.c_str(), strerror(-status)); - continue; + if (WEXITSTATUS(status)) { + PLOG(ERROR) << "Process exited with code: " << WEXITSTATUS(status); + return false; } } return true; } -int VolumeManager::remountAppStorageDirs(userid_t userId) { +int VolumeManager::remountAppStorageDirs(int uid, int pid, + const std::vector& packageNames) { if (!GetBoolProperty(android::vold::kPropFuse, false)) { return 0; } - LOG(INFO) << "Start remounting app obb and data"; - pid_t child; - if (!(child = fork())) { - // Child process - if (daemon(0, 0) == -1) { - PLOG(FATAL) << "Cannot create daemon"; - } - // TODO(149548518): Refactor the code so minimize the work after fork to prevent deadlock. - if (scanProcProcesses(0, userId, bindMountAppDataObbDir, nullptr)) { - // As some forked zygote processes may not setuid and recognized as an app yet, sleep - // 3s and try again to catch 'em all. - usleep(3 * 1000 * 1000); // 3s - async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Retry remounting app obb"); - scanProcProcesses(0, userId, bindMountAppDataObbDir, nullptr); - _exit(0); - } else { - _exit(1); - } - } - if (child == -1) { - PLOG(ERROR) << "Failed to fork"; - return -1; - } else if (child == 0) { - // Parent - int stat_loc; - for (;;) { - if (waitpid(child, &stat_loc, 0) != -1 || errno != EINTR) { - break; + // Only run the remount if fuse is mounted for that user. + userid_t userId = multiuser_get_user_id(uid); + bool fuseMounted = false; + for (auto& vol : mInternalEmulatedVolumes) { + if (vol->getMountUserId() == userId && vol->getState() == VolumeBase::State::kMounted) { + auto* emulatedVol = static_cast(vol.get()); + if (emulatedVol) { + fuseMounted = emulatedVol->isFuseMounted(); } + break; } } - return 0; -} - -bool VolumeManager::updateFuseMountedProperty() { - if (mFuseMountedUsers.size() == 0) { - android::base::SetProperty("vold.fuse_running_users", ""); - return true; - } - std::stringstream stream; - char const * sep = ""; - for (const auto& userId : mFuseMountedUsers) { - stream << sep; - stream << userId; - sep = ", "; + if (fuseMounted) { + forkAndRemountStorage(uid, pid, packageNames); } - return android::base::SetProperty("vold.fuse_running_users", stream.str()); -} - -bool VolumeManager::addFuseMountedUser(userid_t userId) { - mFuseMountedUsers.insert(userId); - return updateFuseMountedProperty(); -} - -bool VolumeManager::removeFuseMountedUser(userid_t userId) { - mFuseMountedUsers.erase(userId); - return updateFuseMountedProperty(); + return 0; } int VolumeManager::reset() { @@ -975,8 +891,6 @@ int VolumeManager::reset() { updateVirtualDisk(); mAddedUsers.clear(); mStartedUsers.clear(); - mFuseMountedUsers.clear(); - updateFuseMountedProperty(); return 0; } @@ -996,8 +910,6 @@ int VolumeManager::shutdown() { mInternalEmulatedVolumes.clear(); mDisks.clear(); mPendingDisks.clear(); - mFuseMountedUsers.clear(); - updateFuseMountedProperty(); android::vold::sSleepOnUnmount = true; return 0; } diff --git a/VolumeManager.h b/VolumeManager.h index bf05dcf..b83871e 100644 --- a/VolumeManager.h +++ b/VolumeManager.h @@ -118,10 +118,7 @@ class VolumeManager { int setPrimary(const std::shared_ptr& vol); int remountUid(uid_t uid, int32_t remountMode); - int remountAppStorageDirs(userid_t userId); - - bool addFuseMountedUser(userid_t userId); - bool removeFuseMountedUser(userid_t userId); + int remountAppStorageDirs(int uid, int pid, const std::vector& packageNames); /* Reset all internal state, typically during framework boot */ int reset(); @@ -230,9 +227,6 @@ class VolumeManager { int mNextObbId; int mNextStubId; bool mSecureKeyguardShowing; - - // Set of all user id that fuse is ready to use. - std::unordered_set mFuseMountedUsers; }; #endif diff --git a/binder/android/os/IVold.aidl b/binder/android/os/IVold.aidl index f1ada6c..1d5657f 100644 --- a/binder/android/os/IVold.aidl +++ b/binder/android/os/IVold.aidl @@ -53,6 +53,7 @@ interface IVold { IVoldTaskListener listener); void remountUid(int uid, int remountMode); + void remountAppStorageDirs(int uid, int pid, in @utf8InCpp String[] packageNames); void setupAppDir(@utf8InCpp String path, int appUid); void fixupAppDir(@utf8InCpp String path, int appUid); diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp index 1391685..02d5c37 100644 --- a/model/EmulatedVolume.cpp +++ b/model/EmulatedVolume.cpp @@ -125,19 +125,6 @@ status_t EmulatedVolume::mountFuseBindMounts() { if (status != OK) { return status; } - - if (mAppDataIsolationEnabled) { - // Starting from now, fuse is running, and zygote will bind app obb & data directory - if (!VolumeManager::Instance()->addFuseMountedUser(userId)) { - return UNKNOWN_ERROR; - } - - // As all new processes created by zygote will bind app obb data directory, we just need - // to have a snapshot of all existing processes and see if any existing process needs to - // remount obb data directory. - VolumeManager::Instance()->remountAppStorageDirs(userId); - } - return OK; } @@ -308,12 +295,6 @@ status_t EmulatedVolume::doUnmount() { if (mFuseMounted) { std::string label = getLabel(); - // Update fuse mounted record - if (mAppDataIsolationEnabled && - !VolumeManager::Instance()->removeFuseMountedUser(userId)) { - return UNKNOWN_ERROR; - } - // Ignoring unmount return status because we do want to try to unmount // the rest cleanly. unmountFuseBindMounts(); diff --git a/model/EmulatedVolume.h b/model/EmulatedVolume.h index 12d01ec..b25fb7c 100644 --- a/model/EmulatedVolume.h +++ b/model/EmulatedVolume.h @@ -41,6 +41,7 @@ class EmulatedVolume : public VolumeBase { EmulatedVolume(const std::string& rawPath, dev_t device, const std::string& fsUuid, int userId); virtual ~EmulatedVolume(); std::string getRootPath() const override; + bool isFuseMounted() const { return mFuseMounted; } protected: status_t doMount() override;