set-verity-state: Harden logic of overlayfs setup/teardown
Refactor set_avb_verity_state() to return tri-state result:
{.success=false}
Failed to set verity state.
{.success=true, .want_reboot=false}
Success but verity already is requested state, so no need to reboot.
{.success=true, .want_reboot=true}
Successfully changed verity state, need reboot to take effect.
Setup overlayfs only if verity is going to be or already disabled.
Teardown overlayfs regardless of enable-verity success of not.
This ensures more robust behavior of setup / teardown overlayfs.
Adjust the log messages for consistent wording.
No point logging the errno of fs_mgr_overlayfs_[setup|teardown]
because the error must already be logged by callee.
Bug: 241688845
Test: adb [disable|enable]-verity
Change-Id: I3a77fe567757ca6173e8c3142e77fef483d9b849
diff --git a/set-verity-state/set-verity-state.cpp b/set-verity-state/set-verity-state.cpp
index 4b7bd5a..52757b6 100644
--- a/set-verity-state/set-verity-state.cpp
+++ b/set-verity-state/set-verity-state.cpp
@@ -59,44 +59,55 @@
bool overlayfs_setup(bool enable) {
auto change = false;
errno = 0;
- if (enable ? fs_mgr_overlayfs_teardown(nullptr, &change)
- : fs_mgr_overlayfs_setup(nullptr, &change)) {
+ if (enable ? fs_mgr_overlayfs_setup(nullptr, &change)
+ : fs_mgr_overlayfs_teardown(nullptr, &change)) {
if (change) {
- LOG(INFO) << (enable ? "disabling" : "using") << " overlayfs";
+ LOG(INFO) << (enable ? "Enabled" : "Disabled") << " overlayfs";
}
- } else if (errno) {
- PLOG(ERROR) << "Failed to " << (enable ? "teardown" : "setup") << " overlayfs";
+ } else {
+ LOG(ERROR) << "Failed to " << (enable ? "enable" : "disable") << " overlayfs";
}
return change;
}
+struct SetVerityStateResult {
+ bool success = false;
+ bool want_reboot = false;
+};
+
/* Use AVB to turn verity on/off */
-bool set_avb_verity_enabled_state(AvbOps* ops, bool enable_verity) {
+SetVerityStateResult SetVerityState(bool enable_verity) {
std::string ab_suffix = get_ab_suffix();
- bool verity_enabled;
+ bool verity_enabled = false;
if (is_avb_device_locked()) {
- LOG(ERROR) << "Device is locked. Please unlock the device first";
- return false;
+ LOG(ERROR) << "Device must be bootloader unlocked to change verity state";
+ return {};
}
- if (!avb_user_verity_get(ops, ab_suffix.c_str(), &verity_enabled)) {
+ std::unique_ptr<AvbOps, decltype(&avb_ops_user_free)> ops(avb_ops_user_new(), &avb_ops_user_free);
+ if (!ops) {
+ LOG(ERROR) << "Error getting AVB ops";
+ return {};
+ }
+
+ if (!avb_user_verity_get(ops.get(), ab_suffix.c_str(), &verity_enabled)) {
LOG(ERROR) << "Error getting verity state";
- return false;
+ return {};
}
if ((verity_enabled && enable_verity) || (!verity_enabled && !enable_verity)) {
- LOG(INFO) << "verity is already " << (verity_enabled ? "enabled" : "disabled");
- return false;
+ LOG(INFO) << "Verity is already " << (verity_enabled ? "enabled" : "disabled");
+ return {.success = true, .want_reboot = false};
}
- if (!avb_user_verity_set(ops, ab_suffix.c_str(), enable_verity)) {
+ if (!avb_user_verity_set(ops.get(), ab_suffix.c_str(), enable_verity)) {
LOG(ERROR) << "Error setting verity state";
- return false;
+ return {};
}
LOG(INFO) << "Successfully " << (enable_verity ? "enabled" : "disabled") << " verity";
- return true;
+ return {.success = true, .want_reboot = true};
}
void MyLogger(android::base::LogId id, android::base::LogSeverity severity, const char* tag,
@@ -118,14 +129,14 @@
LOG(FATAL) << "set-verity-state called with empty argv";
}
- bool enable = false;
+ bool enable_verity = false;
std::string procname = android::base::Basename(argv[0]);
if (procname == "enable-verity") {
- enable = true;
+ enable_verity = true;
} else if (procname == "disable-verity") {
- enable = false;
+ enable_verity = false;
} else if (argc == 2 && (argv[1] == "1"s || argv[1] == "0"s)) {
- enable = (argv[1] == "1"s);
+ enable_verity = (argv[1] == "1"s);
} else {
printf("usage: %s [1|0]\n", argv[0]);
return 1;
@@ -148,21 +159,28 @@
return 1;
}
- std::unique_ptr<AvbOps, decltype(&avb_ops_user_free)> ops(avb_ops_user_new(), &avb_ops_user_free);
- if (!ops) {
- LOG(ERROR) << "Error getting AVB ops";
- return 1;
+ int exit_code = 0;
+ bool want_reboot = false;
+
+ auto ret = SetVerityState(enable_verity);
+ if (ret.success) {
+ want_reboot |= ret.want_reboot;
+ } else {
+ exit_code = 1;
}
- // Start a threadpool to service waitForService() callbacks as
- // fs_mgr_overlayfs_* might call waitForService() to get the image service.
- android::ProcessState::self()->startThreadPool();
- bool any_changed = set_avb_verity_enabled_state(ops.get(), enable);
- any_changed |= overlayfs_setup(enable);
-
- if (any_changed) {
- printf("Now reboot your device for settings to take effect\n");
+ // Disable any overlayfs unconditionally if we want verity enabled.
+ // Enable overlayfs only if verity is successfully disabled or is already disabled.
+ if (enable_verity || ret.success) {
+ // 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);
}
- return 0;
+ if (want_reboot) {
+ printf("Reboot the device for new settings to take effect\n");
+ }
+
+ return exit_code;
}