From ab4dd01395faaff0cd8095cd03e26eb8b520a5bf Mon Sep 17 00:00:00 2001
From: Yi-Yo Chiang <yochiang@google.com>
Date: Thu, 3 Nov 2022 21:20:08 +0800
Subject: [PATCH] set-verity-state: Use ro.boot.veritymode to determine current
 verity state

On a device that don't use overlayfs remount (e.g. no EXT4 dup blocks;
this can be simulated by patching fs_mgr_wants_overlayfs()), if we run
disable-verity or enable-verity twice in a row then the second
invocation would not suggest a reboot:

  adb disable-verity
  > Successfully disabled verity
  > Reboot to take effect...
  adb disable-verity
  > Verity is already disabled
              ^^^ this is WRONG! verity is disabled only after a reboot

It behaves like this because it suggest a reboot only if the vbmeta
verity (HASHTREE) flag is changed.
Read the ro.boot.veritymode property instead to determine the current
dm-verity state and suggest a reboot by comparing current and future
verity state:
* If AVB verification is disabled, then ro.boot.veritymode is undefined
  (probably empty), don't suggest reboot in this case as it's pointless.
* Otherwise suggest a reboot if the new state (which would take effect
  after reboot) differs from the current verity state.
* Reference:
  https://android.googlesource.com/platform/external/avb/+/master/README.md#handling-dm_verity-errors

Bug: 241688845
Test: adb-remount-test
Test: Run "adb enable-verity" & "adb disable-verity" multiple times
Change-Id: If1df5bee6e5dcbda580b3dff6c32da93d08bbb46
---
 fs_mgr/fs_mgr_remount.cpp | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp
index eac3ff298..1040a1802 100644
--- a/fs_mgr/fs_mgr_remount.cpp
+++ b/fs_mgr/fs_mgr_remount.cpp
@@ -445,30 +445,33 @@ struct SetVerityStateResult {
 
 SetVerityStateResult SetVerityState(bool enable_verity) {
     const auto ab_suffix = android::base::GetProperty("ro.boot.slot_suffix", "");
-    bool verity_enabled = false;
-
     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 {};
-    }
-
-    if ((verity_enabled && enable_verity) || (!verity_enabled && !enable_verity)) {
-        LOG(INFO) << "Verity is already " << (verity_enabled ? "enabled" : "disabled");
-        return {.success = true, .want_reboot = false};
-    }
-
     if (!avb_user_verity_set(ops.get(), ab_suffix.c_str(), enable_verity)) {
         LOG(ERROR) << "Error setting verity state";
         return {};
     }
-
+    bool verification_enabled = false;
+    if (!avb_user_verification_get(ops.get(), ab_suffix.c_str(), &verification_enabled)) {
+        LOG(ERROR) << "Error getting verification state";
+        return {};
+    }
+    if (!verification_enabled) {
+        LOG(WARNING) << "AVB verification is disabled, "
+                     << (enable_verity ? "enabling" : "disabling")
+                     << " verity state may have no effect";
+        return {.success = true, .want_reboot = false};
+    }
+    const auto verity_mode = android::base::GetProperty("ro.boot.veritymode", "");
+    const bool was_enabled = (verity_mode != "disabled");
+    if ((was_enabled && enable_verity) || (!was_enabled && !enable_verity)) {
+        LOG(INFO) << "Verity is already " << (enable_verity ? "enabled" : "disabled");
+        return {.success = true, .want_reboot = false};
+    }
     LOG(INFO) << "Successfully " << (enable_verity ? "enabled" : "disabled") << " verity";
     return {.success = true, .want_reboot = true};
 }