Reland '[hwasan] Add fixed_shadow_base flag (#73980)' #95445
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was reverted in llvm#95435 because it broke Android static hwasan binaries. This reland excludes Android from the logic, as suggested by Vitaly. Original commit message: When set to non-zero, the HWASan runtime will map the shadow base at the specified constant address. This is particularly useful in conjunction with the existing compiler option 'hwasan-mapping-offset', which bakes a hardcoded constant address into the instrumentation.
llvmbot
added
compiler-rt
compiler-rt:hwasan
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Thurston Dang (thurstond) ChangesThis was reverted in #95435 because it broke Android static hwasan binaries. This reland excludes Android from the logic, as suggested by Vitaly. Original commit message: This is particularly useful in conjunction with the existing compiler option 'hwasan-mapping-offset', which bakes a hardcoded constant address into the instrumentation. Full diff: https://github.com/llvm/llvm-project/pull/95445.diff 3 Files Affected:
diff --git a/compiler-rt/lib/hwasan/hwasan_flags.inc b/compiler-rt/lib/hwasan/hwasan_flags.inc
index 978fa46b705cb..058a0457b9e7f 100644
--- a/compiler-rt/lib/hwasan/hwasan_flags.inc
+++ b/compiler-rt/lib/hwasan/hwasan_flags.inc
@@ -84,3 +84,10 @@ HWASAN_FLAG(bool, malloc_bisect_dump, false,
// are untagged before the call.
HWASAN_FLAG(bool, fail_without_syscall_abi, true,
"Exit if fail to request relaxed syscall ABI.")
+
+HWASAN_FLAG(
+ uptr, fixed_shadow_base, -1,
+ "If not -1, HWASan will attempt to allocate the shadow at this address, "
+ "instead of choosing one dynamically."
+ "Tip: this can be combined with the compiler option, "
+ "-hwasan-mapping-offset, to optimize the instrumentation.")
diff --git a/compiler-rt/lib/hwasan/hwasan_linux.cpp b/compiler-rt/lib/hwasan/hwasan_linux.cpp
index c254670ee2d48..ce15b80c1e6cc 100644
--- a/compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -106,8 +106,13 @@ static uptr GetHighMemEnd() {
}
static void InitializeShadowBaseAddress(uptr shadow_size_bytes) {
- __hwasan_shadow_memory_dynamic_address =
- FindDynamicShadowStart(shadow_size_bytes);
+ // Android static HWASan doesn't initialize flags correctly
+ if (!SANITIZER_ANDROID && flags()->fixed_shadow_base != (uptr)-1) {
+ __hwasan_shadow_memory_dynamic_address = flags()->fixed_shadow_base;
+ } else {
+ __hwasan_shadow_memory_dynamic_address =
+ FindDynamicShadowStart(shadow_size_bytes);
+ }
}
static void MaybeDieIfNoTaggingAbi(const char *message) {
diff --git a/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c b/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c
new file mode 100644
index 0000000000000..4ff1d3e64c1d0
--- /dev/null
+++ b/compiler-rt/test/hwasan/TestCases/Linux/fixed-shadow.c
@@ -0,0 +1,76 @@
+// Test fixed shadow base functionality.
+//
+// Default compiler instrumentation works with any shadow base (dynamic or fixed).
+// RUN: %clang_hwasan %s -o %t && %run %t
+// RUN: %clang_hwasan %s -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 %run %t
+// RUN: %clang_hwasan %s -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 %run %t
+//
+// If -hwasan-mapping-offset is set, then the fixed_shadow_base needs to match.
+// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=263878495698944 -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 %run %t
+// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=4398046511104 -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 %run %t
+// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=263878495698944 -o %t && HWASAN_OPTIONS=fixed_shadow_base=4398046511104 not %run %t
+// RUN: %clang_hwasan %s -mllvm -hwasan-mapping-offset=4398046511104 -o %t && HWASAN_OPTIONS=fixed_shadow_base=263878495698944 not %run %t
+//
+// Note: if fixed_shadow_base is not set, compiler-rt will dynamically choose a
+// shadow base, which has a tiny but non-zero probability of matching the
+// compiler instrumentation. To avoid test flake, we do not test this case.
+//
+// Assume 48-bit VMA
+// REQUIRES: aarch64-target-arch
+//
+// REQUIRES: Clang
+//
+// UNSUPPORTED: android
+
+#include <assert.h>
+#include <sanitizer/allocator_interface.h>
+#include <sanitizer/hwasan_interface.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/mman.h>
+
+int main() {
+ __hwasan_enable_allocator_tagging();
+
+ // We test that the compiler instrumentation is able to access shadow memory
+ // for many different addresses. If we only test a small number of addresses,
+ // it might work by chance even if the shadow base does not match between the
+ // compiler instrumentation and compiler-rt.
+ void **mmaps[256];
+ // 48-bit VMA
+ for (int i = 0; i < 256; i++) {
+ unsigned long long addr = (i * (1ULL << 40));
+
+ void *p = mmap((void *)addr, 4096, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ // We don't use MAP_FIXED, to avoid overwriting critical memory.
+ // However, if we don't get allocated the requested address, it
+ // isn't a useful test.
+ if ((unsigned long long)p != addr) {
+ munmap(p, 4096);
+ mmaps[i] = MAP_FAILED;
+ } else {
+ mmaps[i] = p;
+ }
+ }
+
+ int failures = 0;
+ for (int i = 0; i < 256; i++) {
+ if (mmaps[i] == MAP_FAILED) {
+ failures++;
+ } else {
+ printf("%d %p\n", i, mmaps[i]);
+ munmap(mmaps[i], 4096);
+ }
+ }
+
+ // We expect roughly 17 failures:
+ // - the page at address zero
+ // - 16 failures because the shadow memory takes up 1/16th of the address space
+ // We could also get unlucky e.g., if libraries or binaries are loaded into the
+ // exact addresses where we tried to map.
+ // To avoid test flake, we allow some margin of error.
+ printf("Failed: %d\n", failures);
+ assert(failures < 48);
+ return 0;
+}
|
vitalybuka
reviewed
Jun 13, 2024
|
If you can wait, I can confirm this works later today by cherry-picking this and trying to reproduce the Android problem. |
LGTM but not critical, as the cause looks straightforward. However fix for flag initialization will need real testing. |
vitalybuka
approved these changes
Jun 13, 2024
|
Now also on the correct PR: I think that's incorrect: ANDROID is Linux and does in fact support preinit array, we just don't use it for HWASan. |
I believe it's disabled because hwasan is dynamic lib. |
I'll double check but I don't think it is, the bug still happened on Android when I applied this |
|
Oh, |
fmayer
approved these changes
Jun 14, 2024
fmayer
reviewed
Jun 14, 2024
|
Thanks @fmayer for testing and @vitalybuka for landing the appropriate patch! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was reverted in #95435 because it broke Android static hwasan binaries. This reland limits the change to !SANITIZER_ANDROID.
Original commit message:
When set to non-zero, the HWASan runtime will map the shadow base at the specified constant address.
This is particularly useful in conjunction with the existing compiler option 'hwasan-mapping-offset', which bakes a hardcoded constant address into the instrumentation.