Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 094917de authored by Florian Mayer's avatar Florian Mayer
Browse files

Do not use GetBoolProperty in signal handler 

This uses an std::string, which causes a heap allocation, which is not
async-safe.

Test: atest --no-bazel-mode permissive_mte_test
Change-Id: I4bd53d42d9a6a659abe62a964f14c81d9ec059d0
parent 61c82abe

debuggerd/handler/debuggerd_handler.cpp

+17 −2
Original line number Diff line number Diff line
@@ -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);

}

debuggerd/test_permissive_mte/src/com/android/tests/debuggerd/PermissiveMteTest.java

+29 −0
Original line number Diff line number Diff line
@@ -97,4 +97,33 @@ public class PermissiveMteTest extends BaseHostJUnit4Test { 
    }

    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();

  }

}