Merge "async read of mtectrl state" am: 020047609e am: 091eaef52b am: 6efcfd16fa am: e15a85bdbd

Original change: https://android-review.googlesource.com/c/platform/system/extras/+/2645427

Change-Id: I4aabfcbcb365fc44cd8aa3b222c3cf05be3b7fb2
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 57 insertions, 29 deletions

diff --git a/mtectrl/mtectrl.cc b/mtectrl/mtectrl.cc
index bea12a6c..0d862511 100644
--- a/mtectrl/mtectrl.cc
+++ b/mtectrl/mtectrl.cc
@@ -47,6 +47,7 @@ bool UpdateProp(const char* prop_name, const misc_memtag_message& m) {
   if (CheckAndUnset(mode, MISC_MEMTAG_MODE_MEMTAG_KERNEL_ONCE))
     AddItem(&prop_str, "memtag-kernel-once");
   if (CheckAndUnset(mode, MISC_MEMTAG_MODE_MEMTAG_OFF)) AddItem(&prop_str, "memtag-off");
+  if (prop_str.empty()) prop_str = "none";
   if (android::base::GetProperty(prop_name, "") != prop_str)
     android::base::SetProperty(prop_name, prop_str);
   if (mode) {
@@ -68,6 +69,7 @@ void PrintUsage(const char* progname) {
       << "USAGE: " << progname
       << "\n"
          "      [-s PROPERTY_NAME]\n"
+         "      [-f PROPERTY_NAME]\n"
          "      [none,][memtag,][memtag-once,][memtag-kernel,][memtag-kernel-once,][memtag-off,]\n"
          "      [default|force_on|force_off]\n"
          "      [-t PATH_TO_FAKE_MISC_PARTITION]\n"
@@ -76,6 +78,9 @@ void PrintUsage(const char* progname) {
          "  -s PROPERTY_NAME\n"
          "      Sets the system property 'PROPERTY_NAME' to the new MTE mode (if provided), or to\n"
          "      the current value from the /misc partition.\n"
+         "  -f PROPERTY_NAME\n"
+         "      Used in combination with -s without a new MTE mode and sets the system property\n"
+         "      'PROPERTY_NAME' to 1 after reading the current value from the /misc partition\n"
          "  [none,][memtag,][memtag-once,][memtag-kernel,][memtag-kernel-once,][memtag-off,]\n"
          "      A set of MTE options to be applied, if provided. Multiple options may be\n"
          "      specified as a ','-delimited list, e.g. 'memtag,memtag-kernel'.\n"
@@ -138,8 +143,30 @@ bool HandleOverride(const std::string& override_value, misc_memtag_message* m) {
   return true;
 }
 
+int DoSetProp(const std::function<bool(misc_memtag_message*, std::string*)>& read_memtag_message,
+              const char* set_prop) {
+  // -s <property> is given on its own. This means we want to read the state
+  // of the misc partition into the property.
+  std::string err;
+  misc_memtag_message m = {};
+  if (!read_memtag_message(&m, &err)) {
+    LOG(ERROR) << "Failed to read memtag message: " << err;
+    return 1;
+  }
+  if (m.magic != MISC_MEMTAG_MAGIC_HEADER || m.version != MISC_MEMTAG_MESSAGE_VERSION) {
+    // This should not fail by construction.
+    CHECK(UpdateProp(set_prop, {}));
+    // This is an expected case, as the partition gets initialized to all zero.
+    return 0;
+  }
+  // Unlike above, setting the system property here can fail if the misc partition
+  // was corrupted by another program (e.g. the bootloader).
+  return UpdateProp(set_prop, m) ? 0 : 1;
+}
+
 int main(int argc, char** argv) {
   const char* set_prop = nullptr;
+  const char* flag_prop = nullptr;
   int opt;
   std::function<bool(misc_memtag_message*, std::string*)> read_memtag_message =
       ReadMiscMemtagMessage;
@@ -147,7 +174,7 @@ int main(int argc, char** argv) {
       WriteMiscMemtagMessage;
 
   android::base::unique_fd fake_partition_fd;
-  while ((opt = getopt(argc, argv, "s:t:")) != -1) {
+  while ((opt = getopt(argc, argv, "s:t:f:")) != -1) {
     switch (opt) {
       case 's':
         // Set property in argument to state of misc partition. If given by
@@ -158,6 +185,9 @@ int main(int argc, char** argv) {
         // state.
         set_prop = optarg;
         break;
+      case 'f':
+        flag_prop = optarg;
+        break;
       case 't': {
         // Use different fake misc partition for testing.
         const char* filename = optarg;
@@ -184,35 +214,22 @@ int main(int argc, char** argv) {
   const char* value = optind < argc ? argv[optind++] : nullptr;
   const char* override_value = optind < argc ? argv[optind++] : nullptr;
 
-  if (optind != argc) {  // Unknown argument.
+  if ((optind != argc) ||       // Unknown argument.
+      (value && flag_prop) ||   // -f is only valid when no value given
+      (!value && !set_prop)) {  // value must be given if -s is not
     PrintUsage(argv[0]);
     return 1;
   }
 
   if (!value && set_prop) {
-    // -s <property> is given on its own. This means we want to read the state
-    // of the misc partition into the property.
-    std::string err;
-    misc_memtag_message m = {};
-    if (!read_memtag_message(&m, &err)) {
-      LOG(ERROR) << "Failed to read memtag message: " << err;
-      return 1;
-    }
-    if (m.magic != MISC_MEMTAG_MAGIC_HEADER || m.version != MISC_MEMTAG_MESSAGE_VERSION) {
-      // This should not fail by construction.
-      CHECK(UpdateProp(set_prop, {}));
-      // This is an expected case, as the partition gets initialized to all zero.
-      return 0;
+    int ret = DoSetProp(read_memtag_message, set_prop);
+    if (flag_prop) {
+      android::base::SetProperty(flag_prop, "1");
     }
-    // Unlike above, setting the system property here can fail if the misc partition
-    // was corrupted by another program (e.g. the bootloader).
-    return UpdateProp(set_prop, m) ? 0 : 1;
+    return ret;
   }
 
-  if (!value) {
-    PrintUsage(argv[0]);
-    return 1;
-  }
+  CHECK(value);
 
   misc_memtag_message m = {.version = MISC_MEMTAG_MESSAGE_VERSION,
                            .magic = MISC_MEMTAG_MAGIC_HEADER};
diff --git a/mtectrl/mtectrl.rc b/mtectrl/mtectrl.rc
index 050993d1..38c58f7a 100644
--- a/mtectrl/mtectrl.rc
+++ b/mtectrl/mtectrl.rc
@@ -13,17 +13,20 @@
 # limitations under the License.
 
 on property:arm64.memtag.bootctl=*
+  wait_for_prop arm64.memtag.bootctl_loaded 1
   exec -- /system/bin/mtectrl ${arm64.memtag.bootctl:-none} ${persist.device_config.runtime_native_boot.bootloader_override:-default}
 
 on property:persist.device_config.runtime_native_boot.bootloader_override=*
+  wait_for_prop arm64.memtag.bootctl_loaded 1
   exec -- /system/bin/mtectrl ${arm64.memtag.bootctl:-none} ${persist.device_config.runtime_native_boot.bootloader_override:-default}
 
 # adbd gets initialized in init, so run before that. this makes sure that the
 # user does not change the value before we initialize it
 on early-init && property:ro.arm64.memtag.bootctl_supported=1
-  exec -- /system/bin/mtectrl -s arm64.memtag.bootctl
+  exec_background -- /system/bin/mtectrl -s arm64.memtag.bootctl -f arm64.memtag.bootctl_loaded
 
 on shutdown && property:ro.arm64.memtag.bootctl_supported=1
+  # This doesn't use wait_for_prop to not stall the shutdown.
   exec -- /system/bin/mtectrl ${arm64.memtag.bootctl:-none} ${persist.device_config.runtime_native_boot.bootloader_override:-default}
 
 on property:persist.device_config.runtime_native_boot.bootloader_override=force_on
diff --git a/mtectrl/mtectrl_test.cc b/mtectrl/mtectrl_test.cc
index 5fe77f87..d9f3c370 100644
--- a/mtectrl/mtectrl_test.cc
+++ b/mtectrl/mtectrl_test.cc
@@ -42,6 +42,9 @@ std::string GetMisc() {
 std::string TestProperty() {
   return android::base::GetProperty("arm64.memtag.test_bootctl", "");
 }
+std::string TestFlag() {
+  return android::base::GetProperty("arm64.memtag.test_bootctl_loaded", "");
+}
 }  // namespace
 
 class MteCtrlTest : public ::testing::Test {
@@ -52,6 +55,7 @@ class MteCtrlTest : public ::testing::Test {
     CHECK(ftruncate(fd, sizeof(misc_memtag_message)) != -1);
     close(fd);
     android::base::SetProperty("arm64.memtag.test_bootctl", "INVALID");
+    android::base::SetProperty("arm64.memtag.test_bootctl_loaded", "0");
   }
   void TearDown() override {
     CHECK(unlink("/data/local/tmp/misc_memtag") == 0);
@@ -85,16 +89,18 @@ TEST_F(MteCtrlTest, set_memtag_force_off) {
 
 TEST_F(MteCtrlTest, read_memtag) {
   ASSERT_EQ(mtectrl("memtag"), 0);
-  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl"), 0);
+  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
   EXPECT_EQ(TestProperty(), "memtag");
+  EXPECT_EQ(TestFlag(), "1");
 }
 
 TEST_F(MteCtrlTest, read_invalid_memtag_message) {
   misc_memtag_message m = {.version = 1, .magic = 0xffff, .memtag_mode = MISC_MEMTAG_MODE_MEMTAG};
   std::string m_str(reinterpret_cast<char*>(&m), sizeof(m));
   android::base::WriteStringToFile(m_str, "/data/local/tmp/misc_memtag");
-  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl"), 0);
-  EXPECT_EQ(TestProperty(), "");
+  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
+  EXPECT_EQ(TestProperty(), "none");
+  EXPECT_EQ(TestFlag(), "1");
 }
 
 TEST_F(MteCtrlTest, read_invalid_memtag_mode) {
@@ -103,8 +109,9 @@ TEST_F(MteCtrlTest, read_invalid_memtag_mode) {
                            .memtag_mode = MISC_MEMTAG_MODE_MEMTAG | 1u << 31};
   std::string m_str(reinterpret_cast<char*>(&m), sizeof(m));
   android::base::WriteStringToFile(m_str, "/data/local/tmp/misc_memtag");
-  ASSERT_NE(mtectrl("-s arm64.memtag.test_bootctl"), 0);
+  ASSERT_NE(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
   EXPECT_EQ(TestProperty(), "memtag");
+  EXPECT_EQ(TestFlag(), "1");
 }
 
 TEST_F(MteCtrlTest, set_read_memtag) {
@@ -124,8 +131,9 @@ TEST_F(MteCtrlTest, override) {
 }
 
 TEST_F(MteCtrlTest, read_empty) {
-  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl"), 0);
-  EXPECT_EQ(TestProperty(), "");
+  ASSERT_EQ(mtectrl("-s arm64.memtag.test_bootctl -f arm64.memtag.test_bootctl_loaded"), 0);
+  EXPECT_EQ(TestProperty(), "none");
+  EXPECT_EQ(TestFlag(), "1");
 }
 
 TEST_F(MteCtrlTest, force_off_invalid_mode) {