[llvm] 8f061ed - [HWASan] Ignore shortgranules for global tag selection

Vitaly Buka via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 17:49:35 PDT 2023 

Author: Mitch Phillips
Date: 2023-05-18T17:49:15-07:00
New Revision: 8f061edef83991f41508a19961e833404dfb17f6

URL: https://github.com/llvm/llvm-project/commit/8f061edef83991f41508a19961e833404dfb17f6
DIFF: https://github.com/llvm/llvm-project/commit/8f061edef83991f41508a19961e833404dfb17f6.diff

LOG: [HWASan] Ignore shortgranules for global tag selection

Tag selection for global variables is sequential, starting at a
pseduo-ish seed that's based on the hash of the file name.

Previously, it was possible for a global to be assigned a tag in the
range [1,15]. If the global's size was not a multiple of granules (i.e.
`size % 16 != 0`), then the last granule of the global would be assigned
a short granule tag as well.

If the real memory tag of the global (e.g. '04') happened to collide
with the short granule tag (e.g. '04'), then __hwasan_check would
see that the memory tag matched the short granule tag, and dutifully
assume (in this fast check) that everthing is okay.

Unfortunately, if you tried to access the [5,15]th byte, you never get
to the short granule check. This means you miss intra-granule overflows
on the last granule of a global, if said global was assigned a real
memory tag in the range [1,15].

This causes flakiness in certain global tests, if the SHA of the
filename changes between runs.

This problem also exists for heap and stack allocations as well, but
there's a concern that if we exclude the [1,15] range for heap and stack
that it's an unhappy tradeoff. On Android, this would mean that a 1/255
chance of false positive becomes 1/240. On other platforms though (that
have a less-than-8-bit tag space), this may be unacceptable. We can
think about potential fixes for that in other places, but globals are
fine to reduce the space, as really the only thing that matters is
catching sequential overflow. The false-negative scenario is much, much
more common in use-after-free and use-after-scope, which globals don't
have.

Differential Revision: https://reviews.llvm.org/D150742

Added: 
    

Modified: 
    llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
    llvm/test/Instrumentation/HWAddressSanitizer/X86/globals.ll
    llvm/test/Instrumentation/HWAddressSanitizer/globals-tag.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
index 23ea9aac8ce69..01d2bd042b558 100644
--- a/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1585,11 +1585,14 @@ void HWAddressSanitizer::instrumentGlobals() {
   Hasher.final(Hash);
   uint8_t Tag = Hash[0];
 
+  assert(TagMaskByte >= 16);
+
   for (GlobalVariable *GV : Globals) {
-    Tag &= TagMaskByte;
-    // Skip tag 0 in order to avoid collisions with untagged memory.
-    if (Tag == 0)
-      Tag = 1;
+    // Don't allow globals to be tagged with something that looks like a
+    // short-granule tag, otherwise we lose inter-granule overflow detection, as
+    // the fast path shadow-vs-address check succeeds.
+    if (Tag < 16 || Tag > TagMaskByte)
+      Tag = 16;
     instrumentGlobal(GV, Tag++);
   }
 }

diff  --git a/llvm/test/Instrumentation/HWAddressSanitizer/X86/globals.ll b/llvm/test/Instrumentation/HWAddressSanitizer/X86/globals.ll
index 546530e54e397..ca1153a6a3e63 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/X86/globals.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/X86/globals.ll
@@ -7,19 +7,19 @@
 
 ; CHECK: @hwasan.dummy.global = private constant [0 x i8] zeroinitializer, section "hwasan_globals", comdat($hwasan.module_ctor), !associated [[NOTE:![0-9]+]]
 
-; CHECK: @four.hwasan = private global { i32, [12 x i8] } { i32 1, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00," }, align 16
-; CHECK: @four.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @four.hwasan to i64), i64 ptrtoint (ptr @four.hwasan.descriptor to i64)) to i32), i32 738197508 }, section "hwasan_globals", !associated [[FOUR:![0-9]+]]
+; CHECK: @four.hwasan = private global { i32, [12 x i8] } { i32 1, [12 x i8] c"\00\00\00\00\00\00\00\00\00\00\00\10" }, align 16
+; CHECK: @four.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @four.hwasan to i64), i64 ptrtoint (ptr @four.hwasan.descriptor to i64)) to i32), i32 268435460 }, section "hwasan_globals", !associated [[FOUR:![0-9]+]]
 
 ; CHECK: @sixteen.hwasan = private global [16 x i8] zeroinitializer, align 16
-; CHECK: @sixteen.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @sixteen.hwasan to i64), i64 ptrtoint (ptr @sixteen.hwasan.descriptor to i64)) to i32), i32 754974736 }, section "hwasan_globals", !associated [[SIXTEEN:![0-9]+]]
+; CHECK: @sixteen.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @sixteen.hwasan to i64), i64 ptrtoint (ptr @sixteen.hwasan.descriptor to i64)) to i32), i32 285212688 }, section "hwasan_globals", !associated [[SIXTEEN:![0-9]+]]
 
 ; CHECK: @huge.hwasan = private global [16777232 x i8] zeroinitializer, align 16
-; CHECK: @huge.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @huge.hwasan to i64), i64 ptrtoint (ptr @huge.hwasan.descriptor to i64)) to i32), i32 788529136 }, section "hwasan_globals", !associated [[HUGE:![0-9]+]]
-; CHECK: @huge.hwasan.descriptor.1 = private constant { i32, i32 } { i32 trunc (i64 add (i64 sub (i64 ptrtoint (ptr @huge.hwasan to i64), i64 ptrtoint (ptr @huge.hwasan.descriptor.1 to i64)), i64 16777200) to i32), i32 771751968 }, section "hwasan_globals", !associated [[HUGE]]
+; CHECK: @huge.hwasan.descriptor = private constant { i32, i32 } { i32 trunc (i64 sub (i64 ptrtoint (ptr @huge.hwasan to i64), i64 ptrtoint (ptr @huge.hwasan.descriptor to i64)) to i32), i32 318767088 }, section "hwasan_globals", !associated [[HUGE:![0-9]+]]
+; CHECK: @huge.hwasan.descriptor.1 = private constant { i32, i32 } { i32 trunc (i64 add (i64 sub (i64 ptrtoint (ptr @huge.hwasan to i64), i64 ptrtoint (ptr @huge.hwasan.descriptor.1 to i64)), i64 16777200) to i32), i32 301989920 }, section "hwasan_globals", !associated [[HUGE]]
 
-; CHECK: @four = alias i32, inttoptr (i64 add (i64 ptrtoint (ptr @four.hwasan to i64), i64 6341068275337658368) to ptr)
-; CHECK: @sixteen = alias [16 x i8], inttoptr (i64 add (i64 ptrtoint (ptr @sixteen.hwasan to i64), i64 6485183463413514240) to ptr)
-; CHECK: @huge = alias [16777232 x i8], inttoptr (i64 add (i64 ptrtoint (ptr @huge.hwasan to i64), i64 6629298651489370112) to ptr)
+; CHECK: @four = alias i32, inttoptr (i64 add (i64 ptrtoint (ptr @four.hwasan to i64), i64 2305843009213693952) to ptr)
+; CHECK: @sixteen = alias [16 x i8], inttoptr (i64 add (i64 ptrtoint (ptr @sixteen.hwasan to i64), i64 2449958197289549824) to ptr)
+; CHECK: @huge = alias [16777232 x i8], inttoptr (i64 add (i64 ptrtoint (ptr @huge.hwasan to i64), i64 2594073385365405696) to ptr)
 
 ; CHECK: [[NOTE]] = !{ptr @hwasan.note}
 ; CHECK: [[FOUR]] = !{ptr @four.hwasan}

diff  --git a/llvm/test/Instrumentation/HWAddressSanitizer/globals-tag.ll b/llvm/test/Instrumentation/HWAddressSanitizer/globals-tag.ll
index f7fc6701e24ff..4a3e4d29eb255 100644
--- a/llvm/test/Instrumentation/HWAddressSanitizer/globals-tag.ll
+++ b/llvm/test/Instrumentation/HWAddressSanitizer/globals-tag.ll
@@ -35,21 +35,6 @@ source_filename = "test.cpp"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00~"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00$"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\000"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\01"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\02"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\03"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\04"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\05"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\06"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\07"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\08"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\09"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0A"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0B"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0C"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0D"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0E"
-; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\0F"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\001"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\10"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\11"
@@ -185,20 +170,35 @@ source_filename = "test.cpp"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00e"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00E"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E0"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E0"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E1"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E1"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E2"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E2"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E3"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E3"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E4"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E4"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E5"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E5"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E6"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E6"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E7"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E7"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E8"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E8"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E9"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\E9"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EA"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EA"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EB"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EB"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EC"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EC"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\ED"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\ED"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EE"
+; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EE"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00\EF"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00f"
 ; CHECK-DAG: "\00\00\00\00\00\00\00\00\00\00\00F"

More information about the llvm-commits mailing list