-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[hwasan] Fixing false invalid-free with disabled tagging #67169
Conversation
@llvm/pr-subscribers-compiler-rt-sanitizer ChangesThis problem was accidentally discovered by the internal symbolizer, but If we just disable tagging, there may still be tagged allocations that We cannot just disable the "invalid-free" check with disabled tagging, The fix is to continue tagging with zero even if tagging is disabled. This makes the "disabled" mode less efficient, but this is not the Full diff: https://github.com/llvm/llvm-project/pull/67169.diff 2 Files Affected:
diff --git a/compiler-rt/lib/hwasan/hwasan_allocator.cpp b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
index c99d730e059faab..7cebe2c4d8c6721 100644
--- a/compiler-rt/lib/hwasan/hwasan_allocator.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_allocator.cpp
@@ -239,23 +239,20 @@ static void *HwasanAllocate(StackTrace *stack, uptr orig_size, uptr alignment,
// retag to 0.
if (InTaggableRegion(reinterpret_cast<uptr>(user_ptr)) &&
(flags()->tag_in_malloc || flags()->tag_in_free) &&
- atomic_load_relaxed(&hwasan_allocator_tagging_enabled)) {
- if (flags()->tag_in_malloc && malloc_bisect(stack, orig_size)) {
- tag_t tag = t ? t->GenerateRandomTag() : kFallbackAllocTag;
- uptr tag_size = orig_size ? orig_size : 1;
- uptr full_granule_size = RoundDownTo(tag_size, kShadowAlignment);
- user_ptr =
- (void *)TagMemoryAligned((uptr)user_ptr, full_granule_size, tag);
- if (full_granule_size != tag_size) {
- u8 *short_granule =
- reinterpret_cast<u8 *>(allocated) + full_granule_size;
- TagMemoryAligned((uptr)short_granule, kShadowAlignment,
- tag_size % kShadowAlignment);
- short_granule[kShadowAlignment - 1] = tag;
- }
- } else {
- user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
+ atomic_load_relaxed(&hwasan_allocator_tagging_enabled) &&
+ flags()->tag_in_malloc && malloc_bisect(stack, orig_size)) {
+ tag_t tag = t ? t->GenerateRandomTag() : kFallbackAllocTag;
+ uptr tag_size = orig_size ? orig_size : 1;
+ uptr full_granule_size = RoundDownTo(tag_size, kShadowAlignment);
+ user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, full_granule_size, tag);
+ if (full_granule_size != tag_size) {
+ u8 *short_granule = reinterpret_cast<u8 *>(allocated) + full_granule_size;
+ TagMemoryAligned((uptr)short_granule, kShadowAlignment,
+ tag_size % kShadowAlignment);
+ short_granule[kShadowAlignment - 1] = tag;
}
+ } else {
+ user_ptr = (void *)TagMemoryAligned((uptr)user_ptr, size, 0);
}
Metadata *meta =
diff --git a/compiler-rt/test/hwasan/TestCases/enable-disable.c b/compiler-rt/test/hwasan/TestCases/enable-disable.c
new file mode 100644
index 000000000000000..d686b6bdc62fd3c
--- /dev/null
+++ b/compiler-rt/test/hwasan/TestCases/enable-disable.c
@@ -0,0 +1,23 @@
+// RUN: %clang_hwasan -O1 %s -o %t && %run %t 2>&1
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <sanitizer/hwasan_interface.h>
+
+#define SZ 1000
+void * x[SZ];
+
+int main() {
+ __hwasan_enable_allocator_tagging();
+ for (unsigned i = 0; i < SZ; ++i)
+ x[i] = malloc(10);
+ for (unsigned i = 0; i < SZ; ++i)
+ free(x[i]);
+ __hwasan_disable_allocator_tagging();
+
+ for (unsigned i = 0; i < SZ; ++i)
+ x[i] = malloc(10);
+ for (unsigned i = 0; i < SZ; ++i)
+ free(x[i]);
+ return 0;
+}
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
8d3caba
to
6fe62b6
Compare
This problem was accidentally discovered by the internal symbolizer, but it's relevant for external one as well, see the test. If we just disable tagging, there may still be tagged allocations that have already been freed. After disabling tagging, these tagged allocations can be released to the user as-is, which would later break the "invalid-free" check. We cannot just disable the "invalid-free" check with disabled tagging, because if we re-enable tagging, the issue still applies to allocations created when it was disabled. The fix is to continue tagging with zero even if tagging is disabled. This makes the "disabled" mode less efficient, but this is not the primary use case.
This problem was accidentally discovered by the internal symbolizer, but
it's relevant for external one as well, see the test.
If we just disable tagging, there may still be tagged allocations that
have already been freed. After disabling tagging, these tagged
allocations can be released to the user as-is, which would later break
the "invalid-free" check.
We cannot just disable the "invalid-free" check with disabled tagging,
because if we re-enable tagging, the issue still applies to allocations
created when it was disabled.
The fix is to continue tagging with zero even if tagging is disabled.
This makes the "disabled" mode less efficient, but this is not the
primary use case.