Index: lib/hwasan/hwasan_allocator.h =================================================================== --- lib/hwasan/hwasan_allocator.h +++ lib/hwasan/hwasan_allocator.h @@ -29,7 +29,8 @@ namespace __hwasan { struct Metadata { - u32 requested_size; // sizes are < 4G. + u32 requested_size : 31; // sizes are < 2G. + u32 right_aligned : 1; u32 alloc_context_id; }; Index: lib/hwasan/hwasan_allocator.cc =================================================================== --- lib/hwasan/hwasan_allocator.cc +++ lib/hwasan/hwasan_allocator.cc @@ -35,7 +35,16 @@ return metadata_ && metadata_->alloc_context_id && metadata_->requested_size; } +// Aligns the 'addr' right to the granule boundary. +static uptr AlignRight(uptr addr, uptr requested_size) { + uptr tail_size = requested_size % kShadowAlignment; + if (!tail_size) return addr; + return addr + kShadowAlignment - tail_size; +} + uptr HwasanChunkView::Beg() const { + if (metadata_ && metadata_->right_aligned) + return AlignRight(block_, metadata_->requested_size); return block_; } uptr HwasanChunkView::End() const { @@ -110,6 +119,7 @@ reinterpret_cast(allocator.GetMetaData(allocated)); meta->requested_size = static_cast(orig_size); meta->alloc_context_id = StackDepotPut(*stack); + meta->right_aligned = false; if (zeroise) { internal_memset(allocated, 0, size); } else if (flags()->max_malloc_fill_size > 0) { @@ -123,6 +133,17 @@ user_ptr = (void *)TagMemoryAligned( (uptr)user_ptr, size, t ? t->GenerateRandomTag() : kFallbackAllocTag); + if ((orig_size % kShadowAlignment) && (alignment <= kShadowAlignment)) { + if (int malloc_align_right = flags()->malloc_align_right) { + uptr as_uptr = reinterpret_cast(user_ptr); + if (malloc_align_right == kRightAlignAlways || + GetTagFromPointer(as_uptr) & 1) { // use a tag bit, i.e. random. + user_ptr = reinterpret_cast(AlignRight(as_uptr, orig_size)); + meta->right_aligned = 1; + } + } + } + HWASAN_MALLOC_HOOK(user_ptr, size); return user_ptr; } @@ -143,8 +164,10 @@ ReportInvalidFree(stack, reinterpret_cast(tagged_ptr)); void *untagged_ptr = UntagPtr(tagged_ptr); + void *aligned_ptr = reinterpret_cast( + RoundDownTo(reinterpret_cast(untagged_ptr), kShadowAlignment)); Metadata *meta = - reinterpret_cast(allocator.GetMetaData(untagged_ptr)); + reinterpret_cast(allocator.GetMetaData(aligned_ptr)); uptr orig_size = meta->requested_size; u32 free_context_id = StackDepotPut(*stack); u32 alloc_context_id = meta->alloc_context_id; @@ -154,22 +177,23 @@ // poisoned. Thread *t = GetCurrentThread(); if (flags()->max_free_fill_size > 0) { - uptr fill_size = Min(orig_size, (uptr)flags()->max_free_fill_size); - internal_memset(untagged_ptr, flags()->free_fill_byte, fill_size); + uptr fill_size = + Min(TaggedSize(orig_size), (uptr)flags()->max_free_fill_size); + internal_memset(aligned_ptr, flags()->free_fill_byte, fill_size); } if (flags()->tag_in_free && atomic_load_relaxed(&hwasan_allocator_tagging_enabled)) - TagMemoryAligned((uptr)untagged_ptr, TaggedSize(orig_size), + TagMemoryAligned(reinterpret_cast(aligned_ptr), TaggedSize(orig_size), t ? t->GenerateRandomTag() : kFallbackFreeTag); if (t) { - allocator.Deallocate(t->allocator_cache(), untagged_ptr); + allocator.Deallocate(t->allocator_cache(), aligned_ptr); if (auto *ha = t->heap_allocations()) ha->push({reinterpret_cast(tagged_ptr), alloc_context_id, free_context_id, static_cast(orig_size)}); } else { SpinMutexLock l(&fallback_mutex); AllocatorCache *cache = &fallback_allocator_cache; - allocator.Deallocate(cache, untagged_ptr); + allocator.Deallocate(cache, aligned_ptr); } } @@ -213,8 +237,8 @@ const void *untagged_ptr = UntagPtr(tagged_ptr); if (!untagged_ptr) return 0; const void *beg = allocator.GetBlockBegin(untagged_ptr); - if (beg != untagged_ptr) return 0; Metadata *b = (Metadata *)allocator.GetMetaData(untagged_ptr); + if (beg != untagged_ptr && !b->right_aligned) return 0; return b->requested_size; } Index: lib/hwasan/hwasan_flags.h =================================================================== --- lib/hwasan/hwasan_flags.h +++ lib/hwasan/hwasan_flags.h @@ -23,6 +23,12 @@ void SetDefaults(); }; +enum RightAlignMode { + kRightAlignNever, + kRightAlignSometimes, + kRightAlignAlways +}; + Flags *flags(); } // namespace __hwasan Index: lib/hwasan/hwasan_flags.inc =================================================================== --- lib/hwasan/hwasan_flags.inc +++ lib/hwasan/hwasan_flags.inc @@ -38,6 +38,11 @@ "HWASan allocator flag. max_malloc_fill_size is the maximal amount of " "bytes that will be filled with malloc_fill_byte on malloc.") HWASAN_FLAG( + int, malloc_align_right, 0, // off by default + "HWASan allocator flag. 0 (default) means allocations are always aligned" + "left to the 16-byte boundary; 1 means allocations are sometimes aligned" + "right; 2 means they are always aligned right.") +HWASAN_FLAG( int, max_free_fill_size, 0, "HWASan allocator flag. max_free_fill_size is the maximal amount of " "bytes that will be filled with free_fill_byte during free.") Index: test/hwasan/TestCases/heap-buffer-overflow.c =================================================================== --- test/hwasan/TestCases/heap-buffer-overflow.c +++ test/hwasan/TestCases/heap-buffer-overflow.c @@ -1,10 +1,17 @@ // RUN: %clang_hwasan %s -o %t -// RUN: not %run %t 40 2>&1 | FileCheck %s --check-prefix=CHECK40 -// RUN: not %run %t 80 2>&1 | FileCheck %s --check-prefix=CHECK80 +// RUN: not %run %t 40 2>&1 | FileCheck %s --check-prefix=CHECK40-LEFT +// RUN: %env_hwasan_opts=malloc_align_right=2 not %run %t 40 2>&1 | FileCheck %s --check-prefix=CHECK40-RIGHT +// RUN: not %run %t 80 2>&1 | FileCheck %s --check-prefix=CHECK80-LEFT +// RUN: %env_hwasan_opts=malloc_align_right=2 not %run %t 80 2>&1 | FileCheck %s --check-prefix=CHECK80-RIGHT // RUN: not %run %t -30 2>&1 | FileCheck %s --check-prefix=CHECKm30 // RUN: not %run %t -30 1000000 2>&1 | FileCheck %s --check-prefix=CHECKMm30 // RUN: not %run %t 1000000 1000000 2>&1 | FileCheck %s --check-prefix=CHECKM +// Test OOB within the granule. +// Misses the bug when malloc is left-aligned, catches it otherwise. +// RUN: %run %t 31 +// RUN: %env_hwasan_opts=malloc_align_right=2 not %run %t 31 2>&1 | FileCheck %s --check-prefix=CHECK31 + // REQUIRES: stable-runtime #include @@ -16,12 +23,18 @@ int offset = argc < 2 ? 40 : atoi(argv[1]); int size = argc < 3 ? 30 : atoi(argv[2]); char * volatile x = (char*)malloc(size); + fprintf(stderr, "base: %p access: %p\n", x, &x[offset]); x[offset] = 42; -// CHECK40: allocated heap chunk; size: 32 offset: 8 -// CHECK40: is located 10 bytes to the right of 30-byte region + +// CHECK40-LEFT: allocated heap chunk; size: 32 offset: 8 +// CHECK40-LEFT: is located 10 bytes to the right of 30-byte region +// CHECK40-RIGHT: allocated heap chunk; size: 32 offset: 10 +// CHECK40-RIGHT: is located 10 bytes to the right of 30-byte region // -// CHECK80: allocated heap chunk; size: 32 offset: 16 -// CHECK80: is located 50 bytes to the right of 30-byte region +// CHECK80-LEFT: allocated heap chunk; size: 32 offset: 16 +// CHECK80-LEFT: is located 50 bytes to the right of 30-byte region +// CHECK80-RIGHT: allocated heap chunk; size: 32 offset: 18 +// CHECK80-RIGHT: is located 50 bytes to the right of 30-byte region // // CHECKm30: allocated heap chunk; size: 32 offset: 2 // CHECKm30: is located 30 bytes to the left of 30-byte region @@ -31,5 +44,7 @@ // // CHECKM: is a large allocated heap chunk; size: 1003520 offset: 1000000 // CHECKM: is located 0 bytes to the right of 1000000-byte region +// +// CHECK31: is located 1 bytes to the right of 30-byte region free(x); } Index: test/hwasan/TestCases/use-after-free.c =================================================================== --- test/hwasan/TestCases/use-after-free.c +++ test/hwasan/TestCases/use-after-free.c @@ -22,8 +22,8 @@ if (ISREAD) r = x[5]; else x[5] = 42; // should be on the same line. // CHECK: [[TYPE]] of size 1 at {{.*}} tags: [[PTR_TAG:[0-9a-f][0-9a-f]]]/[[MEM_TAG:[0-9a-f][0-9a-f]]] (ptr/mem) // CHECK: #0 {{.*}} in main {{.*}}use-after-free.c:[[@LINE-2]] - - // CHECK: is a small unallocated heap chunk; size: 16 offset: 5 + // Offset is 5 or 11 depending on left/right alignment. + // CHECK: is a small unallocated heap chunk; size: 16 offset: {{5|11}} // CHECK: is located 5 bytes inside of 10-byte region // // CHECK: freed by thread {{.*}} here: