remount: Simplify fs_mgr_overlayfs_setup.
The use of errno in this function is very difficult to reason about, and
leads to a lot of complexity (eg saving and restoring errno on a case by
case basis).
This CL adds explicit logging in error paths and simplifies the return
state to "succeeded" or "failed".
In addition, the "change" outparam has been simplified as well.
Previously it indicated that *anything* in the filesystem changed. This
is not super useful since the only thing callers care about is whether
or not overlayfs went from "disabled" to "enabled". The outparam now
reflects that.
Bug: 241179247
Test: remount
Change-Id: I5a2b4dcc942e6807c9965cd484de152b47022c4e
diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp
index 354d02a..57762e6 100644
--- a/fs_mgr/fs_mgr_overlayfs.cpp
+++ b/fs_mgr/fs_mgr_overlayfs.cpp
@@ -100,13 +100,12 @@
return false;
}
-bool fs_mgr_overlayfs_setup(const char*, bool* change, bool) {
- if (change) *change = false;
+bool fs_mgr_overlayfs_setup(const char*, bool*, bool) {
+ LOG(ERROR) << "Overlayfs remounts can only be used in debuggable builds";
return false;
}
-bool fs_mgr_overlayfs_teardown(const char*, bool* change) {
- if (change) *change = false;
+bool fs_mgr_overlayfs_teardown(const char*, bool*) {
return false;
}
@@ -372,77 +371,97 @@
constexpr char kOverlayfsFileContext[] = "u:object_r:overlayfs_file:s0";
-bool fs_mgr_overlayfs_setup_dir(const std::string& dir, std::string* overlay, bool* change) {
- auto ret = true;
- auto top = dir + kOverlayTopDir;
- if (setfscreatecon(kOverlayfsFileContext)) {
- ret = false;
- PERROR << "setfscreatecon " << kOverlayfsFileContext;
- }
- auto save_errno = errno;
- if (!mkdir(top.c_str(), 0755)) {
- if (change) *change = true;
- } else if (errno != EEXIST) {
- ret = false;
- PERROR << "mkdir " << top;
- } else {
- errno = save_errno;
- }
- setfscreatecon(nullptr);
+class AutoSetFsCreateCon final {
+ public:
+ AutoSetFsCreateCon() {}
+ AutoSetFsCreateCon(const std::string& context) { Set(context); }
+ ~AutoSetFsCreateCon() { Restore(); }
- if (overlay) *overlay = std::move(top);
- return ret;
+ bool Ok() const { return ok_; }
+ bool Set(const std::string& context) {
+ if (setfscreatecon(context.c_str())) {
+ PLOG(ERROR) << "setfscreatecon " << context;
+ return false;
+ }
+ ok_ = true;
+ return true;
+ }
+ bool Restore() {
+ if (restored_ || !ok_) {
+ return true;
+ }
+ if (setfscreatecon(nullptr)) {
+ PLOG(ERROR) << "setfscreatecon null";
+ return false;
+ }
+ restored_ = true;
+ return true;
+ }
+
+ private:
+ bool ok_ = false;
+ bool restored_ = false;
+};
+
+std::string fs_mgr_overlayfs_setup_dir(const std::string& dir) {
+ auto top = dir + kOverlayTopDir;
+
+ AutoSetFsCreateCon createcon(kOverlayfsFileContext);
+ if (!createcon.Ok()) {
+ return {};
+ }
+ if (mkdir(top.c_str(), 0755) != 0 && errno != EEXIST) {
+ PERROR << "mkdir " << top;
+ return {};
+ }
+ if (!createcon.Restore()) {
+ return {};
+ }
+ return top;
}
bool fs_mgr_overlayfs_setup_one(const std::string& overlay, const std::string& mount_point,
- bool* change) {
- auto ret = true;
- if (fs_mgr_overlayfs_already_mounted(mount_point)) return ret;
+ bool* want_reboot) {
+ if (fs_mgr_overlayfs_already_mounted(mount_point)) {
+ return true;
+ }
auto fsrec_mount_point = overlay + "/" + android::base::Basename(mount_point) + "/";
- if (setfscreatecon(kOverlayfsFileContext)) {
- ret = false;
- PERROR << "setfscreatecon " << kOverlayfsFileContext;
+ AutoSetFsCreateCon createcon(kOverlayfsFileContext);
+ if (!createcon.Ok()) {
+ return false;
}
- auto save_errno = errno;
- if (!mkdir(fsrec_mount_point.c_str(), 0755)) {
- if (change) *change = true;
- } else if (errno != EEXIST) {
- ret = false;
+ if (mkdir(fsrec_mount_point.c_str(), 0755) != 0 && errno != EEXIST) {
PERROR << "mkdir " << fsrec_mount_point;
- } else {
- errno = save_errno;
+ return false;
+ }
+ if (mkdir((fsrec_mount_point + kWorkName).c_str(), 0755) != 0 && errno != EEXIST) {
+ PERROR << "mkdir " << fsrec_mount_point << kWorkName;
+ return false;
+ }
+ if (!createcon.Restore()) {
+ return false;
}
- save_errno = errno;
- if (!mkdir((fsrec_mount_point + kWorkName).c_str(), 0755)) {
- if (change) *change = true;
- } else if (errno != EEXIST) {
- ret = false;
- PERROR << "mkdir " << fsrec_mount_point << kWorkName;
- } else {
- errno = save_errno;
- }
- setfscreatecon(nullptr);
+ createcon = {};
auto new_context = fs_mgr_get_context(mount_point);
- if (!new_context.empty() && setfscreatecon(new_context.c_str())) {
- ret = false;
- PERROR << "setfscreatecon " << new_context;
+ if (new_context.empty() || !createcon.Set(new_context)) {
+ return false;
}
- auto upper = fsrec_mount_point + kUpperName;
- save_errno = errno;
- if (!mkdir(upper.c_str(), 0755)) {
- if (change) *change = true;
- } else if (errno != EEXIST) {
- ret = false;
- PERROR << "mkdir " << upper;
- } else {
- errno = save_errno;
- }
- if (!new_context.empty()) setfscreatecon(nullptr);
- return ret;
+ auto upper = fsrec_mount_point + kUpperName;
+ if (mkdir(upper.c_str(), 0755) != 0 && errno != EEXIST) {
+ PERROR << "mkdir " << upper;
+ return false;
+ }
+ if (!createcon.Restore()) {
+ return false;
+ }
+
+ if (want_reboot) *want_reboot = true;
+
+ return true;
}
uint32_t fs_mgr_overlayfs_slot_number() {
@@ -729,21 +748,23 @@
}
// use as the bound directory in /dev.
+ AutoSetFsCreateCon createcon;
auto new_context = fs_mgr_get_context(entry.mount_point);
- if (!new_context.empty() && setfscreatecon(new_context.c_str())) {
- PERROR << "setfscreatecon " << new_context;
+ if (new_context.empty() || !createcon.Set(new_context)) {
+ continue;
}
move_entry new_entry = {std::move(entry.mount_point), "/dev/TemporaryDir-XXXXXX",
entry.shared_flag};
const auto target = mkdtemp(new_entry.dir.data());
+ if (!createcon.Restore()) {
+ return false;
+ }
if (!target) {
retval = false;
save_errno = errno;
PERROR << "temporary directory for MS_BIND";
- setfscreatecon(nullptr);
continue;
}
- setfscreatecon(nullptr);
if (!parent_private && !parent_made_private) {
parent_made_private = fs_mgr_overlayfs_set_shared_mount(mount_point, false);
@@ -814,20 +835,29 @@
bool fs_mgr_overlayfs_mount_scratch(const std::string& device_path, const std::string mnt_type,
bool readonly = false) {
if (readonly) {
- if (!fs_mgr_access(device_path)) return false;
- } else {
- if (!fs_mgr_rw_access(device_path)) return false;
+ if (!fs_mgr_access(device_path)) {
+ LOG(ERROR) << "Path does not exist: " << device_path;
+ return false;
+ }
+ } else if (!fs_mgr_rw_access(device_path)) {
+ LOG(ERROR) << "Path does not exist or is not readwrite: " << device_path;
+ return false;
}
auto f2fs = fs_mgr_is_f2fs(device_path);
auto ext4 = fs_mgr_is_ext4(device_path);
- if (!f2fs && !ext4) return false;
+ if (!f2fs && !ext4) {
+ LOG(ERROR) << "Scratch partition is not f2fs or ext4";
+ return false;
+ }
- if (setfscreatecon(kOverlayfsFileContext)) {
- PERROR << "setfscreatecon " << kOverlayfsFileContext;
+ AutoSetFsCreateCon createcon(kOverlayfsFileContext);
+ if (!createcon.Ok()) {
+ return false;
}
if (mkdir(kScratchMountPoint.c_str(), 0755) && (errno != EEXIST)) {
PERROR << "create " << kScratchMountPoint;
+ return false;
}
FstabEntry entry;
@@ -859,7 +889,6 @@
if (fs_mgr_overlayfs_already_mounted("/data", false)) {
entry.fs_mgr_flags.check = true;
}
- auto save_errno = errno;
if (mounted) mounted = fs_mgr_do_mount_one(entry) == 0;
if (!mounted) {
if ((entry.fs_type == "f2fs") && ext4) {
@@ -869,12 +898,15 @@
entry.fs_type = "f2fs";
mounted = fs_mgr_do_mount_one(entry) == 0;
}
- if (!mounted) save_errno = errno;
}
- setfscreatecon(nullptr);
- if (!mounted) rmdir(kScratchMountPoint.c_str());
- errno = save_errno;
- return mounted;
+ if (!createcon.Restore()) {
+ return false;
+ }
+ if (!mounted) {
+ rmdir(kScratchMountPoint.c_str());
+ return false;
+ }
+ return true;
}
const std::string kMkF2fs("/system/bin/make_f2fs");
@@ -962,7 +994,6 @@
} else if (mnt_type == "ext4") {
command = kMkExt4 + " -F -b 4096 -t ext4 -m 0 -O has_journal -M " + kScratchMountPoint;
} else {
- errno = ESRCH;
LERROR << mnt_type << " has no mkfs cookbook";
return false;
}
@@ -995,8 +1026,7 @@
}
// Create or update a scratch partition within super.
-static bool CreateDynamicScratch(std::string* scratch_device, bool* partition_exists,
- bool* change) {
+static bool CreateDynamicScratch(std::string* scratch_device, bool* partition_exists) {
const auto partition_name = android::base::Basename(kScratchMountPoint);
auto& dm = DeviceMapper::Instance();
@@ -1069,8 +1099,6 @@
LERROR << "add partition " << partition_name;
return false;
}
-
- if (change) *change = true;
}
if (changed || partition_create) {
@@ -1084,8 +1112,6 @@
if (!CreateLogicalPartition(params, scratch_device)) {
return false;
}
-
- if (change) *change = true;
} else if (scratch_device->empty()) {
*scratch_device = GetBootScratchDevice();
}
@@ -1115,9 +1141,8 @@
return ideal_size;
}
-static bool CreateScratchOnData(std::string* scratch_device, bool* partition_exists, bool* change) {
+static bool CreateScratchOnData(std::string* scratch_device, bool* partition_exists) {
*partition_exists = false;
- if (change) *change = false;
auto images = IImageManager::Open("remount", 10s);
if (!images) {
@@ -1130,8 +1155,6 @@
return true;
}
- if (change) *change = true;
-
// Note: calling RemoveDisabledImages here ensures that we do not race with
// clean_scratch_files and accidentally try to map an image that will be
// deleted.
@@ -1173,12 +1196,11 @@
}
bool fs_mgr_overlayfs_create_scratch(const Fstab& fstab, std::string* scratch_device,
- bool* partition_exists, bool* change) {
+ bool* partition_exists) {
// Use the DSU scratch device managed by gsid if within a DSU system.
if (fs_mgr_is_dsu_running()) {
*scratch_device = GetDsuScratchDevice();
*partition_exists = !scratch_device->empty();
- *change = false;
return *partition_exists;
}
@@ -1194,22 +1216,24 @@
if (CanUseSuperPartition(fstab, &is_virtual_ab)) {
bool can_use_data = false;
if (is_virtual_ab && FilesystemHasReliablePinning("/data", &can_use_data) && can_use_data) {
- return CreateScratchOnData(scratch_device, partition_exists, change);
+ return CreateScratchOnData(scratch_device, partition_exists);
}
- return CreateDynamicScratch(scratch_device, partition_exists, change);
+ return CreateDynamicScratch(scratch_device, partition_exists);
}
- errno = ENXIO;
return false;
}
// Create and mount kScratchMountPoint storage if we have logical partitions
-bool fs_mgr_overlayfs_setup_scratch(const Fstab& fstab, bool* change) {
- if (fs_mgr_overlayfs_already_mounted(kScratchMountPoint, false)) return true;
+bool fs_mgr_overlayfs_setup_scratch(const Fstab& fstab) {
+ if (fs_mgr_overlayfs_already_mounted(kScratchMountPoint, false)) {
+ return true;
+ }
std::string scratch_device;
bool partition_exists;
- if (!fs_mgr_overlayfs_create_scratch(fstab, &scratch_device, &partition_exists, change)) {
+ if (!fs_mgr_overlayfs_create_scratch(fstab, &scratch_device, &partition_exists)) {
+ LOG(ERROR) << "Failed to create scratch partition";
return false;
}
@@ -1217,22 +1241,19 @@
auto mnt_type = fs_mgr_overlayfs_scratch_mount_type();
if (partition_exists) {
if (fs_mgr_overlayfs_mount_scratch(scratch_device, mnt_type)) {
- if (!fs_mgr_access(kScratchMountPoint + kOverlayTopDir) &&
- !fs_mgr_filesystem_has_space(kScratchMountPoint)) {
- // declare it useless, no overrides and no free space
- fs_mgr_overlayfs_umount_scratch();
- } else {
- if (change) *change = true;
+ if (fs_mgr_access(kScratchMountPoint + kOverlayTopDir) ||
+ fs_mgr_filesystem_has_space(kScratchMountPoint)) {
return true;
}
+ // declare it useless, no overrides and no free space
+ fs_mgr_overlayfs_umount_scratch();
}
- // partition existed, but was not initialized; fall through to make it.
- errno = 0;
}
- if (!fs_mgr_overlayfs_make_scratch(scratch_device, mnt_type)) return false;
-
- if (change) *change = true;
+ if (!fs_mgr_overlayfs_make_scratch(scratch_device, mnt_type)) {
+ LOG(ERROR) << "Failed to format scratch partition";
+ return false;
+ }
return fs_mgr_overlayfs_mount_scratch(scratch_device, mnt_type);
}
@@ -1355,24 +1376,23 @@
return ret;
}
-// Returns false if setup not permitted, errno set to last error.
-// If something is altered, set *change.
-bool fs_mgr_overlayfs_setup(const char* mount_point, bool* change, bool force) {
- if (change) *change = false;
- auto ret = false;
- if (fs_mgr_overlayfs_valid() == OverlayfsValidResult::kNotSupported) return ret;
- if (!fs_mgr_boot_completed()) {
- errno = EBUSY;
- PERROR << "setup";
- return ret;
- }
-
- auto save_errno = errno;
- Fstab fstab;
- if (!ReadDefaultFstab(&fstab)) {
+bool fs_mgr_overlayfs_setup(const char* mount_point, bool* want_reboot, bool just_disabled_verity) {
+ if (fs_mgr_overlayfs_valid() == OverlayfsValidResult::kNotSupported) {
+ LOG(ERROR) << "Overlayfs is not supported";
return false;
}
- errno = save_errno;
+
+ if (!fs_mgr_boot_completed()) {
+ LOG(ERROR) << "Cannot setup overlayfs before persistent properties are ready";
+ return false;
+ }
+
+ Fstab fstab;
+ if (!ReadDefaultFstab(&fstab)) {
+ LOG(ERROR) << "Could not read fstab";
+ return false;
+ }
+
auto candidates = fs_mgr_overlayfs_candidate_list(fstab);
for (auto it = candidates.begin(); it != candidates.end();) {
if (mount_point &&
@@ -1380,9 +1400,8 @@
it = candidates.erase(it);
continue;
}
- save_errno = errno;
- auto verity_enabled = !force && fs_mgr_is_verity_enabled(*it);
- if (errno == ENOENT || errno == ENXIO) errno = save_errno;
+
+ auto verity_enabled = !just_disabled_verity && fs_mgr_is_verity_enabled(*it);
if (verity_enabled) {
it = candidates.erase(it);
continue;
@@ -1390,12 +1409,20 @@
++it;
}
- if (candidates.empty()) return ret;
+ if (candidates.empty()) {
+ if (mount_point) {
+ LOG(ERROR) << "No overlayfs candidate was found for " << mount_point;
+ return false;
+ }
+ return true;
+ }
std::string dir;
for (const auto& overlay_mount_point : OverlayMountPoints()) {
if (overlay_mount_point == kScratchMountPoint) {
- if (!fs_mgr_overlayfs_setup_scratch(fstab, change)) continue;
+ if (!fs_mgr_overlayfs_setup_scratch(fstab)) {
+ continue;
+ }
} else {
if (GetEntryForMountPoint(&fstab, overlay_mount_point) == nullptr) {
continue;
@@ -1405,17 +1432,21 @@
break;
}
if (dir.empty()) {
- if (change && *change) errno = ESRCH;
- if (errno == EPERM) errno = save_errno;
- return ret;
+ LOG(ERROR) << "Could not allocate backing storage for overlays";
+ return false;
}
- std::string overlay;
- ret |= fs_mgr_overlayfs_setup_dir(dir, &overlay, change);
- for (const auto& entry : candidates) {
- ret |= fs_mgr_overlayfs_setup_one(overlay, fs_mgr_mount_point(entry.mount_point), change);
+ const auto overlay = fs_mgr_overlayfs_setup_dir(dir);
+ if (overlay.empty()) {
+ return false;
}
- return ret;
+
+ bool ok = true;
+ for (const auto& entry : candidates) {
+ auto fstab_mount_point = fs_mgr_mount_point(entry.mount_point);
+ ok &= fs_mgr_overlayfs_setup_one(overlay, fstab_mount_point, want_reboot);
+ }
+ return ok;
}
struct MapInfo {
@@ -1736,6 +1767,7 @@
std::string fs_mgr_get_context(const std::string& mount_point) {
char* ctx = nullptr;
if (getfilecon(mount_point.c_str(), &ctx) == -1) {
+ PLOG(ERROR) << "getfilecon " << mount_point;
return "";
}
diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp
index 4a927d0..2202fda 100644
--- a/fs_mgr/fs_mgr_remount.cpp
+++ b/fs_mgr/fs_mgr_remount.cpp
@@ -317,15 +317,15 @@
}
if (fs_mgr_wants_overlayfs(&entry)) {
- bool change = false;
+ bool want_reboot = false;
bool force = result->disabled_verity;
- if (!fs_mgr_overlayfs_setup(mount_point.c_str(), &change, force)) {
+ if (!fs_mgr_overlayfs_setup(mount_point.c_str(), &want_reboot, force)) {
LOG(ERROR) << "Overlayfs setup for " << mount_point << " failed, skipping";
status = BAD_OVERLAY;
it = partitions->erase(it);
continue;
}
- if (change) {
+ if (want_reboot) {
LOG(INFO) << "Using overlayfs for " << mount_point;
result->reboot_later = true;
result->setup_overlayfs = true;
diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h
index ec1d78f..590f66b 100644
--- a/fs_mgr/include/fs_mgr_overlayfs.h
+++ b/fs_mgr/include/fs_mgr_overlayfs.h
@@ -28,14 +28,20 @@
bool fs_mgr_wants_overlayfs(android::fs_mgr::FstabEntry* entry);
bool fs_mgr_overlayfs_mount_all(android::fs_mgr::Fstab* fstab);
-bool fs_mgr_overlayfs_setup(const char* mount_point = nullptr, bool* change = nullptr,
- bool force = true);
bool fs_mgr_overlayfs_teardown(const char* mount_point = nullptr, bool* change = nullptr);
bool fs_mgr_overlayfs_is_setup();
bool fs_mgr_has_shared_blocks(const std::string& mount_point, const std::string& dev);
bool fs_mgr_overlayfs_already_mounted(const std::string& mount_point, bool overlay_only = true);
std::string fs_mgr_get_context(const std::string& mount_point);
+// If "mount_point" is non-null, set up exactly one overlay.
+// If "mount_point" is null, setup any overlays.
+//
+// If |want_reboot| is non-null, and a reboot is needed to apply overlays, then
+// it will be true on return. The caller is responsible for initializing it.
+bool fs_mgr_overlayfs_setup(const char* mount_point = nullptr, bool* want_reboot = nullptr,
+ bool just_disabled_verity = true);
+
enum class OverlayfsValidResult {
kNotSupported = 0,
kOk,
diff --git a/set-verity-state/set-verity-state.cpp b/set-verity-state/set-verity-state.cpp
index de9a452..3c0df79 100644
--- a/set-verity-state/set-verity-state.cpp
+++ b/set-verity-state/set-verity-state.cpp
@@ -80,17 +80,17 @@
}
bool overlayfs_setup(bool enable) {
- auto change = false;
+ auto want_reboot = false;
errno = 0;
- if (enable ? fs_mgr_overlayfs_setup(nullptr, &change)
- : fs_mgr_overlayfs_teardown(nullptr, &change)) {
- if (change) {
+ if (enable ? fs_mgr_overlayfs_setup(nullptr, &want_reboot)
+ : fs_mgr_overlayfs_teardown(nullptr, &want_reboot)) {
+ if (want_reboot) {
LOG(INFO) << (enable ? "Enabled" : "Disabled") << " overlayfs";
}
} else {
LOG(ERROR) << "Failed to " << (enable ? "enable" : "disable") << " overlayfs";
}
- return change;
+ return want_reboot;
}
struct SetVerityStateResult {