[PATCH] D53789: [hwasan] optionally right-align heap allocations

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: lib/hwasan/hwasan_allocator.cc:136
 
+  if (orig_size % kShadowAlignment) {
+    if (int malloc_align_right = flags()->malloc_align_right) {
----------------
dvyukov wrote:
> Shouldn't we also look at user-requested alignment here? It looks like we only satisfy natural alignment.
Yeah, at least for calls like posix_memalign.

This mode will break some things anyway, ex.:
struct S {
  int count;
  char name[0];
};

When allocated with malloc(sizeof(S) + count) the size will not be a multiple of alignment, so "count" will be misaligned. That's why this mode can not be on by default. Let's hope it is not a common case.



================
Comment at: lib/hwasan/hwasan_allocator.cc:139
+      uptr as_uptr = reinterpret_cast<uptr>(user_ptr);
+      if (malloc_align_right == 2    // always right-align
+          || as_uptr & (1 << 20)) {  // use an ASLR bit as a random choice.
----------------
Use named constants, like kHandleSignalYes.


================
Comment at: lib/hwasan/hwasan_allocator.cc:140
+      if (malloc_align_right == 2    // always right-align
+          || as_uptr & (1 << 20)) {  // use an ASLR bit as a random choice.
+        user_ptr = reinterpret_cast<void *>(AlignRight(as_uptr, orig_size));
----------------
This is not very random - entire region will have the same bit, so long running programs are likely to be stuck in either direction.

Use HwasanThread::random_buffer_ instead?



Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D53789