From 9908b1e3c8708aba09afa9f76f05ed4a1a4f4844 Mon Sep 17 00:00:00 2001 From: Florian Mayer Date: Wed, 17 May 2023 20:59:14 +0000 Subject: [PATCH] Revert "Add debug logging for memtag level" This reverts commit b3f3e868785a525692944033ad044f589168ead5. Reason for revert: This created spam in tombstones. This is a cherry-pick of dd44378da7a6912ce3b7ae171c70cc4eda53f2d Bug: 280493417 Merged-In: Ia7b8285a6abd6f89d49859a981be1c9cfa104f81 Change-Id: Ia7b8285a6abd6f89d49859a981be1c9cfa104f81 --- libc/bionic/gwp_asan_wrappers.cpp | 4 +--- libc/bionic/libc_init_static.cpp | 33 +++++++++++-------------------- libc/bionic/sysprop_helpers.cpp | 10 +++------- libc/bionic/sysprop_helpers.h | 9 +++------ 4 files changed, 19 insertions(+), 37 deletions(-) diff --git a/libc/bionic/gwp_asan_wrappers.cpp b/libc/bionic/gwp_asan_wrappers.cpp index fab29ef11..251633d78 100644 --- a/libc/bionic/gwp_asan_wrappers.cpp +++ b/libc/bionic/gwp_asan_wrappers.cpp @@ -307,10 +307,8 @@ bool GetGwpAsanOptionImpl(char* value_out, sysprop_names[3] = persist_default_sysprop; } - // TODO(mitchp): Log overrides using this. - const char* source; return get_config_from_env_or_sysprops(env_var, sysprop_names, arraysize(sysprop_names), - value_out, PROP_VALUE_MAX, &source); + value_out, PROP_VALUE_MAX); } bool GetGwpAsanIntegerOption(unsigned long long* result, diff --git a/libc/bionic/libc_init_static.cpp b/libc/bionic/libc_init_static.cpp index 0935cd623..d64d402ab 100644 --- a/libc/bionic/libc_init_static.cpp +++ b/libc/bionic/libc_init_static.cpp @@ -217,13 +217,16 @@ static unsigned __get_memtag_note(const ElfW(Phdr)* phdr_start, size_t phdr_ct, // Returns true if there's an environment setting (either sysprop or env var) // that should overwrite the ELF note, and places the equivalent heap tagging // level into *level. -static bool get_environment_memtag_setting(const char* basename, HeapTaggingLevel* level) { +static bool get_environment_memtag_setting(HeapTaggingLevel* level) { static const char kMemtagPrognameSyspropPrefix[] = "arm64.memtag.process."; static const char kMemtagGlobalSysprop[] = "persist.arm64.memtag.default"; static const char kMemtagOverrideSyspropPrefix[] = "persist.device_config.memory_safety_native.mode_override.process."; - if (basename == nullptr) return false; + const char* progname = __libc_shared_globals()->init_progname; + if (progname == nullptr) return false; + + const char* basename = __gnu_basename(progname); char options_str[PROP_VALUE_MAX]; char sysprop_name[512]; @@ -234,9 +237,8 @@ static bool get_environment_memtag_setting(const char* basename, HeapTaggingLeve kMemtagOverrideSyspropPrefix, basename); const char* sys_prop_names[] = {sysprop_name, remote_sysprop_name, kMemtagGlobalSysprop}; - const char* source = nullptr; if (!get_config_from_env_or_sysprops("MEMTAG_OPTIONS", sys_prop_names, arraysize(sys_prop_names), - options_str, sizeof(options_str), &source)) { + options_str, sizeof(options_str))) { return false; } @@ -247,20 +249,14 @@ static bool get_environment_memtag_setting(const char* basename, HeapTaggingLeve } else if (strcmp("off", options_str) == 0) { *level = M_HEAP_TAGGING_LEVEL_TBI; } else { - async_safe_format_log(ANDROID_LOG_ERROR, "libc", - "%s: unrecognized memtag level in %s: \"%s\" (options are \"sync\", " - "\"async\", or \"off\").", - basename, source, options_str); + async_safe_format_log( + ANDROID_LOG_ERROR, "libc", + "unrecognized memtag level: \"%s\" (options are \"sync\", \"async\", or \"off\").", + options_str); return false; } - async_safe_format_log(ANDROID_LOG_DEBUG, "libc", "%s: chose memtag level \"%s\" from %s.", - basename, options_str, source); - return true; -} -static void log_elf_memtag_level(const char* basename, const char* level) { - async_safe_format_log(ANDROID_LOG_DEBUG, "libc", "%s: chose memtag level \"%s\" from ELF note", - basename ?: "", level); + return true; } // Returns the initial heap tagging level. Note: This function will never return @@ -268,14 +264,12 @@ static void log_elf_memtag_level(const char* basename, const char* level) { // M_HEAP_TAGGING_LEVEL_TBI. static HeapTaggingLevel __get_heap_tagging_level(const void* phdr_start, size_t phdr_ct, uintptr_t load_bias, bool* stack) { - const char* progname = __libc_shared_globals()->init_progname; - const char* basename = progname ? __gnu_basename(progname) : nullptr; unsigned note_val = __get_memtag_note(reinterpret_cast(phdr_start), phdr_ct, load_bias); *stack = note_val & NT_MEMTAG_STACK; HeapTaggingLevel level; - if (get_environment_memtag_setting(basename, &level)) return level; + if (get_environment_memtag_setting(&level)) return level; // Note, previously (in Android 12), any value outside of bits [0..3] resulted // in a check-fail. In order to be permissive of further extensions, we @@ -290,17 +284,14 @@ static HeapTaggingLevel __get_heap_tagging_level(const void* phdr_start, size_t // by anyone, but we note it (heh) here for posterity, in case the zero // level becomes meaningful, and binaries with this note can be executed // on Android 12 devices. - log_elf_memtag_level(basename, "off"); return M_HEAP_TAGGING_LEVEL_TBI; case NT_MEMTAG_LEVEL_ASYNC: - log_elf_memtag_level(basename, "async"); return M_HEAP_TAGGING_LEVEL_ASYNC; case NT_MEMTAG_LEVEL_SYNC: default: // We allow future extensions to specify mode 3 (currently unused), with // the idea that it might be used for ASYMM mode or something else. On // this version of Android, it falls back to SYNC mode. - log_elf_memtag_level(basename, "sync"); return M_HEAP_TAGGING_LEVEL_SYNC; } } diff --git a/libc/bionic/sysprop_helpers.cpp b/libc/bionic/sysprop_helpers.cpp index 20513306c..56270343f 100644 --- a/libc/bionic/sysprop_helpers.cpp +++ b/libc/bionic/sysprop_helpers.cpp @@ -57,22 +57,18 @@ static bool get_property_value(const char* property_name, char* dest, size_t des } bool get_config_from_env_or_sysprops(const char* env_var_name, const char* const* sys_prop_names, - size_t sys_prop_names_size, char* options, size_t options_size, - const char** chosen_source) { + size_t sys_prop_names_size, char* options, + size_t options_size) { const char* env = getenv(env_var_name); if (env && *env != '\0') { strncpy(options, env, options_size); options[options_size - 1] = '\0'; // Ensure null-termination. - *chosen_source = env_var_name; return true; } for (size_t i = 0; i < sys_prop_names_size; ++i) { if (sys_prop_names[i] == nullptr) continue; - if (get_property_value(sys_prop_names[i], options, options_size)) { - *chosen_source = sys_prop_names[i]; - return true; - } + if (get_property_value(sys_prop_names[i], options, options_size)) return true; } return false; } diff --git a/libc/bionic/sysprop_helpers.h b/libc/bionic/sysprop_helpers.h index 84e7af197..a02c2dc41 100644 --- a/libc/bionic/sysprop_helpers.h +++ b/libc/bionic/sysprop_helpers.h @@ -36,13 +36,10 @@ // 2. System properties, in the order they're specified in sys_prop_names. // If neither of these options are specified (or they're both an empty string), // this function returns false. Otherwise, it returns true, and the presiding -// options string is written to the `options` buffer of size `size`. It will -// store a pointer to either env_var_name, or into the relevant entry of -// sys_prop_names into choicen_source, indiciating which value was used. If -// this function returns true, `options` is guaranteed to be null-terminated. +// options string is written to the `options` buffer of size `size`. If this +// function returns true, `options` is guaranteed to be null-terminated. // `options_size` should be at least PROP_VALUE_MAX. __LIBC_HIDDEN__ bool get_config_from_env_or_sysprops(const char* env_var_name, const char* const* sys_prop_names, size_t sys_prop_names_size, char* options, - size_t options_size, - const char** chosen_source); + size_t options_size);