Snap for 13219462 from fedaddb9bd975308f1300da77b6d2c5eb63af4d3 to 25Q2-release
Change-Id: I636f918d30ef1bbba3f01ab916d2195178f48f44
diff --git a/Android.bp b/Android.bp
index f1221eb..bef6471 100644
--- a/Android.bp
+++ b/Android.bp
@@ -158,6 +158,12 @@
"model/VolumeBase.cpp",
"model/VolumeEncryption.cpp",
],
+ shared_libs: [
+ "server_configurable_flags",
+ ],
+ static_libs: [
+ "vold_flags_c_lib",
+ ],
product_variables: {
arc: {
exclude_srcs: [
@@ -271,3 +277,15 @@
],
path: "binder",
}
+
+aconfig_declarations {
+ name: "vold_flags",
+ package: "android.vold.flags",
+ srcs: ["aconfig/flags.aconfig"],
+ container: "system",
+}
+
+cc_aconfig_library {
+ name: "vold_flags_c_lib",
+ aconfig_declarations: "vold_flags",
+}
\ No newline at end of file
diff --git a/Process.cpp b/Process.cpp
index 426a425..0115fb1 100644
--- a/Process.cpp
+++ b/Process.cpp
@@ -46,7 +46,7 @@
namespace android {
namespace vold {
-static bool checkMaps(const std::string& path, const std::string& prefix) {
+static bool checkMaps(const std::string& path, const std::vector<std::string>& prefixes) {
bool found = false;
auto file = std::unique_ptr<FILE, decltype(&fclose)>{fopen(path.c_str(), "re"), fclose};
if (!file) {
@@ -60,9 +60,14 @@
std::string::size_type pos = line.find('/');
if (pos != std::string::npos) {
line = line.substr(pos);
- if (android::base::StartsWith(line, prefix)) {
- LOG(WARNING) << "Found map " << path << " referencing " << line;
- found = true;
+ for (const auto& prefix : prefixes) {
+ if (android::base::StartsWith(line, prefix)) {
+ LOG(WARNING) << "Found map " << path << " referencing " << line;
+ found = true;
+ break;
+ }
+ }
+ if (found) {
break;
}
}
@@ -72,12 +77,14 @@
return found;
}
-static bool checkSymlink(const std::string& path, const std::string& prefix) {
+static bool checkSymlink(const std::string& path, const std::vector<std::string>& prefixes) {
std::string res;
if (android::base::Readlink(path, &res)) {
- if (android::base::StartsWith(res, prefix)) {
- LOG(WARNING) << "Found symlink " << path << " referencing " << res;
- return true;
+ for (const auto& prefix : prefixes) {
+ if (android::base::StartsWith(res, prefix)) {
+ LOG(WARNING) << "Found symlink " << path << " referencing " << res;
+ return true;
+ }
}
}
return false;
@@ -129,7 +136,8 @@
return pids.size();
}
-int KillProcessesWithOpenFiles(const std::string& prefix, int signal, bool killFuseDaemon) {
+int KillProcessesWithOpenFiles(const std::vector<std::string>& prefixes, int signal,
+ bool killFuseDaemon) {
std::unordered_set<pid_t> pids;
auto proc_d = std::unique_ptr<DIR, int (*)(DIR*)>(opendir("/proc"), closedir);
@@ -148,10 +156,10 @@
// Look for references to prefix
bool found = false;
auto path = StringPrintf("/proc/%d", pid);
- found |= checkMaps(path + "/maps", prefix);
- found |= checkSymlink(path + "/cwd", prefix);
- found |= checkSymlink(path + "/root", prefix);
- found |= checkSymlink(path + "/exe", prefix);
+ found |= checkMaps(path + "/maps", prefixes);
+ found |= checkSymlink(path + "/cwd", prefixes);
+ found |= checkSymlink(path + "/root", prefixes);
+ found |= checkSymlink(path + "/exe", prefixes);
auto fd_path = path + "/fd";
auto fd_d = std::unique_ptr<DIR, int (*)(DIR*)>(opendir(fd_path.c_str()), closedir);
@@ -161,7 +169,7 @@
struct dirent* fd_de;
while ((fd_de = readdir(fd_d.get())) != nullptr) {
if (fd_de->d_type != DT_LNK) continue;
- found |= checkSymlink(fd_path + "/" + fd_de->d_name, prefix);
+ found |= checkSymlink(fd_path + "/" + fd_de->d_name, prefixes);
}
}
@@ -198,5 +206,10 @@
return totalKilledPids;
}
+int KillProcessesWithOpenFiles(const std::string& prefix, int signal, bool killFuseDaemon) {
+ return KillProcessesWithOpenFiles(std::vector<std::string>(1, prefix), signal,
+ killFuseDaemon);
+}
+
} // namespace vold
} // namespace android
diff --git a/Process.h b/Process.h
index f3728b5..8a20d1c 100644
--- a/Process.h
+++ b/Process.h
@@ -20,6 +20,8 @@
namespace android {
namespace vold {
+int KillProcessesWithOpenFiles(const std::vector<std::string>& paths, int signal,
+ bool killFuseDaemon = true);
int KillProcessesWithOpenFiles(const std::string& path, int signal, bool killFuseDaemon = true);
int KillProcessesWithTmpfsMounts(const std::string& path, int signal);
diff --git a/Utils.cpp b/Utils.cpp
index c4070d1..9ad828c 100644
--- a/Utils.cpp
+++ b/Utils.cpp
@@ -1724,6 +1724,139 @@
return result;
}
+/* returns list of non unmounted paths */
+std::vector<std::string> UnmountFusePaths(const std::vector<std::string>& paths_to_unmount) {
+ std::vector<std::string> non_unmounted_paths;
+ for (const auto& path : paths_to_unmount) {
+ LOG(INFO) << "Unmounting fuse path " << path;
+ const char* cpath = path.c_str();
+ if (!umount2(cpath, UMOUNT_NOFOLLOW) || errno == EINVAL || errno == ENOENT) {
+ rmdir(cpath);
+ continue;
+ }
+ non_unmounted_paths.push_back(path);
+ }
+ return non_unmounted_paths;
+}
+
+/* returns list of non unmounted paths */
+std::vector<std::string> UnmountFusePathsWithSleepAndPolling(
+ const std::vector<std::string>& paths_to_unmount) {
+ std::vector<std::string> non_unmounted_paths = paths_to_unmount;
+
+ int count = 10;
+ while (count-- > 0) {
+ usleep(500 * 1000);
+ non_unmounted_paths = UnmountFusePaths(non_unmounted_paths);
+ if (non_unmounted_paths.empty()) {
+ return non_unmounted_paths;
+ }
+ }
+ return non_unmounted_paths;
+}
+
+/* returns list of non unmounted paths */
+std::vector<std::string> KillProcessesWithFuseOpenFilesAndUnmount(
+ const std::vector<std::string>& paths_to_unmount, const std::string& absolute_upper_path,
+ int signal, bool kill_fuse_daemon, bool force_sleep_if_no_processes_killed) {
+
+ // In addition to killing apps using paths to unmount, we need to kill aps using
+ // the upper path (e.g storage/emulated) As they would prevent unmounting fuse
+ std::vector<std::string> paths_to_kill(paths_to_unmount);
+ paths_to_kill.push_back(absolute_upper_path);
+
+ int total_killed_pids = KillProcessesWithOpenFiles(paths_to_kill, signal, kill_fuse_daemon);
+
+ if (sSleepOnUnmount && (force_sleep_if_no_processes_killed || total_killed_pids)) {
+ return UnmountFusePathsWithSleepAndPolling(paths_to_unmount);
+ }
+ return UnmountFusePaths(paths_to_unmount);
+}
+
+status_t UnmountUserFuseEnhanced(userid_t user_id, const std::string& absolute_lower_path,
+ const std::string& relative_upper_path,
+ const std::string& absolute_upper_path,
+ const std::vector<std::string>& bind_mount_paths) {
+ std::vector<std::string> paths_to_unmount(bind_mount_paths);
+
+ std::string fuse_path(StringPrintf("/mnt/user/%d/%s", user_id, relative_upper_path.c_str()));
+ paths_to_unmount.push_back(fuse_path);
+
+ std::string pass_through_path(
+ StringPrintf("/mnt/pass_through/%d/%s", user_id, relative_upper_path.c_str()));
+ paths_to_unmount.push_back(pass_through_path);
+
+ auto start_time = std::chrono::steady_clock::now();
+ LOG(INFO) << "Unmounting fuse paths";
+
+ // Try unmounting without killing any processes
+ paths_to_unmount = UnmountFusePaths(paths_to_unmount);
+ if (paths_to_unmount.empty()) {
+ return android::OK;
+ }
+
+ // Kill processes except for FuseDaemon holding references to fuse paths with SIGINT
+ // And try to unmount afterwards with sleep and polling mechanism
+ paths_to_unmount = KillProcessesWithFuseOpenFilesAndUnmount(
+ paths_to_unmount, absolute_upper_path, SIGINT, /*kill_fuse_daemon*/ false,
+ /*force_sleep_if_no_processes_killed*/false);
+ if (paths_to_unmount.empty()) {
+ return android::OK;
+ }
+
+ // Kill processes except for FuseDaemon holding references to fuse paths with SIGTERM
+ // And try to unmount afterwards with sleep and polling mechanism
+ paths_to_unmount = KillProcessesWithFuseOpenFilesAndUnmount(
+ paths_to_unmount, absolute_upper_path, SIGTERM, /*kill_fuse_daemon*/ false,
+ /*force_sleep_if_no_processes_killed*/false);
+
+ if (paths_to_unmount.empty()) {
+ return android::OK;
+ }
+
+ auto now = std::chrono::steady_clock::now();
+ auto time_elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(now - start_time);
+ bool force_sleep_if_no_process_killed = time_elapsed < std::chrono::milliseconds(5000);
+ // Kill processes except for FuseDaemon holding references to fuse paths with SIGKILL
+ // And try to unmount afterwards with sleep and polling mechanism
+ // intentionally force sleep if sSleepOnUnmount isn't set to false
+ // and if we haven't slept in previous retries so that we give MediaProvider time
+ // to release FDs prior to try killing it in the next step
+ paths_to_unmount = KillProcessesWithFuseOpenFilesAndUnmount(
+ paths_to_unmount, absolute_upper_path, SIGKILL, /*kill_fuse_daemon*/ false,
+ force_sleep_if_no_process_killed);
+
+ if (paths_to_unmount.empty()) {
+ return android::OK;
+ }
+
+ // Kill processes including FuseDaemon holding references to fuse paths with SIGKILL
+ // And try to unmount afterwards with sleep and polling mechanism
+ paths_to_unmount = KillProcessesWithFuseOpenFilesAndUnmount(
+ paths_to_unmount, absolute_upper_path, SIGKILL, /*kill_fuse_daemon*/ true,
+ /*force_sleep_if_no_processes_killed*/false);
+ if (paths_to_unmount.empty()) {
+ return android::OK;
+ }
+
+ // If we reached here, then it means that previous kill and unmount retries didn't succeed
+ // Try to unmount with MNT_DETACH so we try lazily unmount
+ android::status_t result = android::OK;
+ for (const auto& path : paths_to_unmount) {
+ LOG(ERROR) << "Failed to unmount. Trying MNT_DETACH " << path;
+ const char* cpath = path.c_str();
+ if (!umount2(cpath, UMOUNT_NOFOLLOW | MNT_DETACH) || errno == EINVAL || errno == ENOENT) {
+ rmdir(cpath);
+ continue;
+ }
+ PLOG(ERROR) << "Failed to unmount with MNT_DETACH " << path;
+ if (path == fuse_path) {
+ result = -errno;
+ }
+ }
+ return result;
+}
+
status_t PrepareAndroidDirs(const std::string& volumeRoot) {
std::string androidDir = volumeRoot + kAndroidDir;
std::string androidDataDir = volumeRoot + kAppDataDir;
diff --git a/Utils.h b/Utils.h
index 0eca902..8296ef8 100644
--- a/Utils.h
+++ b/Utils.h
@@ -205,6 +205,10 @@
status_t UnmountUserFuse(userid_t userId, const std::string& absolute_lower_path,
const std::string& relative_upper_path);
+status_t UnmountUserFuseEnhanced(userid_t userId, const std::string& absolute_lower_path,
+ const std::string& relative_upper_path,
+ const std::string& absolute_upper_path,
+ const std::vector<std::string>& bind_mount_paths = {});
status_t PrepareAndroidDirs(const std::string& volumeRoot);
diff --git a/aconfig/flags.aconfig b/aconfig/flags.aconfig
new file mode 100644
index 0000000..d9c8fe2
--- /dev/null
+++ b/aconfig/flags.aconfig
@@ -0,0 +1,10 @@
+package: "android.vold.flags"
+container: "system"
+
+flag {
+ name: "enhance_fuse_unmount"
+ namespace: "mediaprovider"
+ description: "This flag controls whether enhancements to unmounting is enabled"
+ bug: "402367661"
+ is_fixed_read_only: true
+}
diff --git a/model/EmulatedVolume.cpp b/model/EmulatedVolume.cpp
index 270dcd4..519453d 100644
--- a/model/EmulatedVolume.cpp
+++ b/model/EmulatedVolume.cpp
@@ -36,8 +36,11 @@
#include <sys/sysmacros.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <android_vold_flags.h>
using android::base::StringPrintf;
+namespace flags = android::vold::flags;
+
namespace android {
namespace vold {
@@ -428,9 +431,17 @@
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";
+ if (flags::enhance_fuse_unmount()) {
+ std::string user_path(StringPrintf("%s/%d", getPath().c_str(), getMountUserId()));
+ if (UnmountUserFuseEnhanced(user_id, getInternalPath(), label, user_path) != OK) {
+ PLOG(INFO) << "UnmountUserFuseEnhanced failed on emulated fuse volume";
+ }
+ } else {
+ 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);
@@ -486,21 +497,22 @@
status_t EmulatedVolume::doUnmount() {
int userId = getMountUserId();
- // Kill all processes using the filesystem before we unmount it. If we
- // unmount the filesystem first, most file system operations will return
- // ENOTCONN until the unmount completes. This is an exotic and unusual
- // error code and might cause broken behaviour in applications.
if (mFuseMounted) {
- // For FUSE specifically, we have an emulated volume per user, so only kill
- // processes using files from this particular user.
std::string user_path(StringPrintf("%s/%d", getPath().c_str(), getMountUserId()));
- LOG(INFO) << "Killing all processes referencing " << user_path;
- KillProcessesUsingPath(user_path);
- } else {
- KillProcessesUsingPath(getPath());
- }
- if (mFuseMounted) {
+ // We don't kill processes before trying to unmount in case enhance_fuse_unmount enabled
+ // As we make sure to kill processes if needed if unmounting failed
+ if (!flags::enhance_fuse_unmount()) {
+ // Kill all processes using the filesystem before we unmount it. If we
+ // unmount the filesystem first, most file system operations will return
+ // ENOTCONN until the unmount completes. This is an exotic and unusual
+ // error code and might cause broken behaviour in applications.
+ // For FUSE specifically, we have an emulated volume per user, so only kill
+ // processes using files from this particular user.
+ LOG(INFO) << "Killing all processes referencing " << user_path;
+ KillProcessesUsingPath(user_path);
+ }
+
std::string label = getLabel();
if (!IsFuseBpfEnabled()) {
@@ -509,12 +521,24 @@
unmountFuseBindMounts();
}
- if (UnmountUserFuse(userId, getInternalPath(), label) != OK) {
- PLOG(INFO) << "UnmountUserFuse failed on emulated fuse volume";
- return -errno;
+ if (flags::enhance_fuse_unmount()) {
+ status_t result = UnmountUserFuseEnhanced(userId, getInternalPath(), label, user_path);
+ if (result != OK) {
+ PLOG(INFO) << "UnmountUserFuseEnhanced failed on emulated fuse volume";
+ return result;
+ }
+ } else {
+ if (UnmountUserFuse(userId, getInternalPath(), label) != OK) {
+ PLOG(INFO) << "UnmountUserFuse failed on emulated fuse volume";
+ return -errno;
+ }
}
mFuseMounted = false;
+ } else {
+ // This branch is needed to help with unmounting private volumes that aren't set to primary
+ // and don't have fuse mounted but have stacked emulated volumes
+ KillProcessesUsingPath(getPath());
}
return unmountSdcardFs();
diff --git a/model/PublicVolume.cpp b/model/PublicVolume.cpp
index 747598a..5a30fca 100644
--- a/model/PublicVolume.cpp
+++ b/model/PublicVolume.cpp
@@ -36,9 +36,11 @@
#include <sys/sysmacros.h>
#include <sys/types.h>
#include <sys/wait.h>
+#include <android_vold_flags.h>
using android::base::GetBoolProperty;
using android::base::StringPrintf;
+namespace flags = android::vold::flags;
namespace android {
namespace vold {
@@ -309,30 +311,57 @@
// the FUSE process first, most file system operations will return
// ENOTCONN until the unmount completes. This is an exotic and unusual
// error code and might cause broken behaviour in applications.
- KillProcessesUsingPath(getPath());
+
+ // We don't kill processes here if enhance_fuse_unmount as we make sure that we kill processes
+ // only if unmounting failed
+ if (!mFuseMounted || !flags::enhance_fuse_unmount()) {
+ KillProcessesUsingPath(getPath());
+ }
if (mFuseMounted) {
std::string stableName = getStableName();
// Unmount bind mounts for running users
auto vol_manager = VolumeManager::Instance();
- int user_id = getMountUserId();
- for (int started_user : vol_manager->getStartedUsers()) {
+ userid_t user_id = getMountUserId();
+ std::vector<std::string> bind_mount_paths;
+ for (userid_t started_user : vol_manager->getStartedUsers()) {
if (started_user == user_id) {
// No need to remove bind mount for the user that owns the mount
continue;
}
- LOG(INFO) << "Removing Public Volume Bind Mount for: " << started_user;
+ if (user_id != VolumeManager::Instance()->getSharedStorageUser(started_user)) {
+ // No need to remove bind mount
+ // if the user does not share storage with the mount owner
+ continue;
+ }
+
auto mountPath = GetFuseMountPathForUser(started_user, stableName);
- ForceUnmount(mountPath);
- rmdir(mountPath.c_str());
+ if (flags::enhance_fuse_unmount()) {
+ // Add it to list so that we unmount it as part of UnmountUserFuseEnhanced
+ bind_mount_paths.push_back(mountPath);
+ } else {
+ LOG(INFO) << "Removing Public Volume Bind Mount for: " << started_user;
+ ForceUnmount(mountPath);
+ rmdir(mountPath.c_str());
+ }
}
- if (UnmountUserFuse(getMountUserId(), getInternalPath(), stableName) != OK) {
- PLOG(INFO) << "UnmountUserFuse failed on public fuse volume";
- return -errno;
+ if (flags::enhance_fuse_unmount()) {
+ status_t result = UnmountUserFuseEnhanced(getMountUserId(), getInternalPath(),
+ stableName, getPath(), bind_mount_paths);
+ if (result != OK) {
+ PLOG(INFO) << "UnmountUserFuseEnhanced failed on public fuse volume";
+ return result;
+ }
+ } else {
+ if (UnmountUserFuse(getMountUserId(), getInternalPath(), stableName) != OK) {
+ PLOG(INFO) << "UnmountUserFuse failed on public fuse volume";
+ return -errno;
+ }
}
+
mFuseMounted = false;
}