Make storage dirs remount fork() safe am: 6b12257702

Change-Id: I589afb12062914cc3da9ff8f7c8dd43014ac6ce9

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<std::string>& 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);

VoldNativeService.h

@@ -64,6 +64,8 @@ class VoldNativeService : public BinderService<VoldNativeService>, public os::Bn
                                const android::sp<android::os::IVoldTaskListener>& listener);
 
     binder::Status remountUid(int32_t uid, int32_t remountMode);
+    binder::Status remountAppStorageDirs(int uid, int pid,
+                               const std::vector<std::string>& packageNames);
 
     binder::Status setupAppDir(const std::string& path, int32_t appUid);
     binder::Status fixupAppDir(const std::string& path, int32_t appUid);

VolumeManager.cpp

@@ -738,228 +738,144 @@ 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));
+    for (int i = 0; i < size; i++) {
-    // We search both .../obb and .../obb/$PKG paths here.
+        if (TEMP_FAILURE_RETRY(mount(sources[i], targets[i], NULL, MS_BIND | MS_REC, NULL)) == -1) {
-    std::string obb_path(StringPrintf("/storage/emulated/%d/Android/obb", userId));
+            async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Failed to mount %s to %s :%s",
-    int profiles_path_len = profiles_path.length();
+                                  sources[i], targets[i], strerror(errno));
-    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<FILE, int (*)(FILE*)>(
-            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<std::string> 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;
-        }
-    }
-
-    // Obb mounted in zygote already, so skip it
-    if (obb_mounted) {
-        return true;
-    }
-
-    std::string obbSource, dataSource;
-    if (IsFilesystemSupported("sdcardfs")) {
-        obbSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/obb", userId);
-        dataSource = StringPrintf("/mnt/runtime/default/emulated/%d/Android/data", userId);
-    } 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);
-    }
-    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));
-        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;
-    }
-
-    // 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));
             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));
-            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());
-        } 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());
-        }
-        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;
-        }
-        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 (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;
-        }
     }
     return true;
 }
 
-int VolumeManager::remountAppStorageDirs(userid_t userId) {
+static std::string getStorageDirSrc(userid_t userId, const std::string& dirName,
-    if (!GetBoolProperty(android::vold::kPropFuse, false)) {
+        const std::string& packageName) {
-        return 0;
+    if (IsFilesystemSupported("sdcardfs")) {
+        return StringPrintf("/mnt/runtime/default/emulated/%d/%s/%s",
+                userId, dirName.c_str(), packageName.c_str());
+    } else {
+        return StringPrintf("/mnt/pass_through/%d/emulated/%d/%s/%s",
+                userId, userId, dirName.c_str(), packageName.c_str());
     }
-    LOG(INFO) << "Start remounting app obb and data";
+}
-    pid_t child;
+
-    if (!(child = fork())) {
+static std::string getStorageDirTarget(userid_t userId, std::string dirName,
-        // Child process
+        std::string packageName) {
-        if (daemon(0, 0) == -1) {
+    return StringPrintf("/storage/emulated/%d/%s/%s",
-            PLOG(FATAL) << "Cannot create daemon";
+            userId, dirName.c_str(), packageName.c_str());
+}
+
+// Fork the process and remount storage
+static bool forkAndRemountStorage(int uid, int pid, const std::vector<std::string>& 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;
+    }
+    // Storing both Android/obb and Android/data paths.
+    int size = packageNames.size() * 2;
+
+    std::unique_ptr<std::string[]> sources(new std::string[size]);
+    std::unique_ptr<std::string[]> targets(new std::string[size]);
+    std::unique_ptr<const char*[]> sources_uptr(new const char*[size]);
+    std::unique_ptr<const char*[]> 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();
+    }
+
+    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;
         }
-        // TODO(149548518): Refactor the code so minimize the work after fork to prevent deadlock.
+        status = EnsureDirExists(targets_cstr[i], 0771, AID_MEDIA_RW, AID_MEDIA_RW);
-        if (scanProcProcesses(0, userId, bindMountAppDataObbDir, nullptr)) {
+        if (status != OK) {
-            // As some forked zygote processes may not setuid and recognized as an app yet, sleep
+            PLOG(ERROR) << "Failed to create dir: " << targets_cstr[i];
-            // 3s and try again to catch 'em all.
+            return false;
-            usleep(3 * 1000 * 1000);  // 3s
+        }
-            async_safe_format_log(ANDROID_LOG_ERROR, "vold", "Retry remounting app obb");
+    }
-            scanProcProcesses(0, userId, bindMountAppDataObbDir, nullptr);
+
+    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 {
             _exit(1);
         }
     }
+
     if (child == -1) {
         PLOG(ERROR) << "Failed to fork";
-        return -1;
+        return false;
-    } else if (child == 0) {
+    } else {
-        // Parent
+        int status;
-        int stat_loc;
+        if (TEMP_FAILURE_RETRY(waitpid(child, &status, 0)) == -1) {
-        for (;;) {
+            PLOG(ERROR) << "Failed to waitpid: " << child;
-            if (waitpid(child, &stat_loc, 0) != -1 || errno != EINTR) {
+            return false;
-                break;
+        }
-            }
+        if (!WIFEXITED(status)) {
+            PLOG(ERROR) << "Process did not exit normally, status: " << status;
+            return false;
+        }
+        if (WEXITSTATUS(status)) {
+            PLOG(ERROR) << "Process exited with code: " << WEXITSTATUS(status);
+            return false;
         }
     }
+    return true;
+}
+
+int VolumeManager::remountAppStorageDirs(int uid, int pid,
+        const std::vector<std::string>& packageNames) {
+    if (!GetBoolProperty(android::vold::kPropFuse, false)) {
+        return 0;
+    }
+    // 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<android::vold::EmulatedVolume*>(vol.get());
+            if (emulatedVol) {
+                fuseMounted = emulatedVol->isFuseMounted();
+            }
+            break;
+        }
+    }
+    if (fuseMounted) {
+        forkAndRemountStorage(uid, pid, packageNames);
+    }
     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 = ", ";
-    }
-    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();
-}
-
 int VolumeManager::reset() {
     // Tear down all existing disks/volumes and start from a blank slate so
     // newly connected framework hears all events.
@@ -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;
 }

VolumeManager.h

@@ -118,10 +118,7 @@ class VolumeManager {
     int setPrimary(const std::shared_ptr<android::vold::VolumeBase>& vol);
 
     int remountUid(uid_t uid, int32_t remountMode);
-    int remountAppStorageDirs(userid_t userId);
+    int remountAppStorageDirs(int uid, int pid, const std::vector<std::string>& packageNames);
-
-    bool addFuseMountedUser(userid_t userId);
-    bool removeFuseMountedUser(userid_t userId);
 
     /* 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<userid_t> mFuseMountedUsers;
 };
 
 #endif

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);

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();

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;