[PATCH] D150742: [HWASan] Ignore shortgranules for global tag selection

[Mitch Phillips via Phabricator via llvm-commits]

hctim created this revision.
hctim added a reviewer: vitalybuka.
Herald added subscribers: Enna1, hiraditya.
Herald added a project: All.
hctim requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150742

Files:
  llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
@@ -1587,9 +1587,15 @@
 
   for (GlobalVariable *GV : Globals) {
     Tag &= TagMaskByte;
-    // Skip tag 0 in order to avoid collisions with untagged memory.
-    if (Tag == 0)
+    if (ClUseShortGranules && Tag < 16) {
+      // 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.
+      Tag = 16;
+    } else if (Tag == 0) {
+      // Skip tag 0 in order to avoid collisions with untagged memory.
       Tag = 1;
+    }
     instrumentGlobal(GV, Tag++);
   }
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150742.522857.patch
Type: text/x-patch
Size: 882 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230517/9a60b16c/attachment.bin>