Merge "Do not use GetBoolProperty in signal handler"

StyxProject-Next · Sep 16, 2022 · 1ee1567 · 1ee1567
2 parents a67092a + 094917d
commit 1ee1567
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 2 deletions.
diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
@@ -24,6 +24,7 @@
 #include <sched.h>
 #include <signal.h>
 #include <stddef.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -51,7 +52,6 @@
 
 #include "handler/fallback.h"
 
-using ::android::base::GetBoolProperty;
 using ::android::base::ParseBool;
 using ::android::base::ParseBoolResult;
 using ::android::base::Pipe;
@@ -87,10 +87,25 @@ static pid_t __gettid() {
   return syscall(__NR_gettid);
 }
 
+static bool property_parse_bool(const char* name) {
+  const prop_info* pi = __system_property_find(name);
+  if (!pi) return false;
+  bool cookie = false;
+  __system_property_read_callback(
+      pi,
+      [](void* cookie, const char*, const char* value, uint32_t) {
+        *reinterpret_cast<bool*>(cookie) = ParseBool(value) == ParseBoolResult::kTrue;
+      },
+      &cookie);
+  return cookie;
+}
+
 static bool is_permissive_mte() {
   // Environment variable for testing or local use from shell.
   char* permissive_env = getenv("MTE_PERMISSIVE");
-  return GetBoolProperty("persist.sys.mte.permissive", false) ||
+  // DO NOT REPLACE this with GetBoolProperty. That uses std::string which allocates, so it is
+  // not async-safe (and this functiong gets used in a signal handler).
+  return property_parse_bool("persist.sys.mte.permissive") ||
          (permissive_env && ParseBool(permissive_env) == ParseBoolResult::kTrue);
 }
 

diff --git a/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java b/debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java
@@ -97,4 +97,33 @@ public void testCrash() throws Exception {
     }
     assertThat(numberTombstones).isEqualTo(1);
   }
+  @Test
+  public void testCrashProperty() throws Exception {
+    String prevValue = getDevice().getProperty("persist.sys.mte.permissive");
+    if (prevValue == null) {
+      prevValue = "";
+    }
+    assertThat(getDevice().setProperty("persist.sys.mte.permissive", "1")).isTrue();
+    CommandResult result =
+        getDevice().executeShellV2Command("/data/local/tmp/mte_crash testCrash " + mUUID);
+    assertThat(result.getExitCode()).isEqualTo(0);
+    int numberTombstones = 0;
+    String[] tombstones = getDevice().getChildren("/data/tombstones");
+    for (String tombstone : tombstones) {
+      if (!tombstone.endsWith(".pb")) {
+        continue;
+      }
+      String tombstonePath = "/data/tombstones/" + tombstone;
+      Tombstone tombstoneProto = parseTombstone(tombstonePath);
+      if (!tombstoneProto.getCommandLineList().stream().anyMatch(x -> x.contains(mUUID))) {
+        continue;
+      }
+      if (!tombstoneProto.getCommandLineList().stream().anyMatch(x -> x.contains("testCrash"))) {
+        continue;
+      }
+      numberTombstones++;
+    }
+    assertThat(numberTombstones).isEqualTo(1);
+    assertThat(getDevice().setProperty("persist.sys.mte.permissive", prevValue)).isTrue();
+  }
 }