remount: Remove errno interaction from fs_mgr_overlayfs_teardown.
This patch eliminates errno as part of the return contract for
fs_mgr_overlayfs_teardown().
The non-standard use of implicit errno makes it extremely difficult to
reason about how these functions can fail. As it turns out,
fs_mgr_overlayfs_teardown has been consistently failing for a long time,
but in a place where errno isn't set, which meant "enable-verity" never
saw the failure.
The failure was originating from umount2(MNT_DETACH) which guaranteed
that DeleteBackingImage would fail with EBUSY, and DeleteBackingImage is
a binder call that doesn't set errno.
This patch switches to umount() and returns a "busy" status if the
unmount fails with EBUSY. In this case it will also disable the scratch
partition. There is a long-standing existing bug where, for non-VAB
devices, it will delete the underlying scratch partition off super. This
is pretty risky with MNT_DETACH, but that path is left unchanged here.
Some duplicated code in set-verity-state was refactored as well, since
the return value of fs_mgr_overlayfs_teardown is now more complex.
Bug: 241179247
Test: adb-remount-test.sh
Change-Id: I2ca75332b75a302622ba9b86d122a6f2accdda3e
diff --git a/set-verity-state/set-verity-state.cpp b/set-verity-state/set-verity-state.cpp
index 3c0df79..2b9c0ca 100644
--- a/set-verity-state/set-verity-state.cpp
+++ b/set-verity-state/set-verity-state.cpp
@@ -47,6 +47,33 @@
const bool kAllowDisableVerity = false;
#endif
+static bool SetupOrTeardownOverlayfs(bool enable) {
+ bool want_reboot = false;
+ if (enable) {
+ if (!fs_mgr_overlayfs_setup(nullptr, &want_reboot)) {
+ LOG(ERROR) << "Overlayfs setup failed.";
+ return want_reboot;
+ }
+ if (want_reboot) {
+ printf("enabling overlayfs\n");
+ }
+ } else {
+ auto rv = fs_mgr_overlayfs_teardown(nullptr, &want_reboot);
+ if (rv == OverlayfsTeardownResult::Error) {
+ LOG(ERROR) << "Overlayfs teardown failed.";
+ return want_reboot;
+ }
+ if (rv == OverlayfsTeardownResult::Busy) {
+ LOG(ERROR) << "Overlayfs is still active until reboot.";
+ return true;
+ }
+ if (want_reboot) {
+ printf("disabling overlayfs\n");
+ }
+ }
+ return want_reboot;
+}
+
/* Helper function to get A/B suffix, if any. If the device isn't
* using A/B the empty string is returned. Otherwise either "_a",
* "_b", ... is returned.
@@ -79,20 +106,6 @@
::exit(1);
}
-bool overlayfs_setup(bool enable) {
- auto want_reboot = false;
- errno = 0;
- 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 want_reboot;
-}
-
struct SetVerityStateResult {
bool success = false;
bool want_reboot = false;
@@ -229,7 +242,7 @@
// Start a threadpool to service waitForService() callbacks as
// fs_mgr_overlayfs_* might call waitForService() to get the image service.
android::ProcessState::self()->startThreadPool();
- want_reboot |= overlayfs_setup(!enable_verity);
+ want_reboot |= SetupOrTeardownOverlayfs(!enable_verity);
}
if (want_reboot) {