[compiler-rt] [Darwin] Strip MTE tags from ASAN and TSAN

[ndrewh]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

rdar://163518624

[llvmbot]

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Andrew Haberlandt (ndrewh)

Changes

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

---

Full diff: https://github.com/llvm/llvm-project/pull/166453.diff

2 Files Affected:

• (modified) compiler-rt/lib/asan/asan_mapping.h (+10-1)
• (modified) compiler-rt/lib/tsan/rtl/tsan_platform.h (+13-3)

diff --git a/compiler-rt/lib/asan/asan_mapping.h b/compiler-rt/lib/asan/asan_mapping.h
index bddae9a074056..9fa7f9014445c 100644
--- a/compiler-rt/lib/asan/asan_mapping.h
+++ b/compiler-rt/lib/asan/asan_mapping.h
@@ -281,11 +281,18 @@ extern uptr kHighMemEnd, kMidMemBeg, kMidMemEnd;  // Initialized in __asan_init.
 
 }  // namespace __asan
 
+#  if SANITIZER_APPLE && SANITIZER_WORDSIZE == 64
+#    define TAG_MASK ((uptr)0x0f << 56)  // Lower half of top byte
+#    define STRIP_TAG(addr) ((addr) & ~TAG_MASK)
+#  else
+#    define STRIP_TAG(addr) (addr)
+#  endif
+
 #  if defined(__sparc__) && SANITIZER_WORDSIZE == 64
 #    include "asan_mapping_sparc64.h"
 #  else
 #    define MEM_TO_SHADOW(mem) \
-      (((mem) >> ASAN_SHADOW_SCALE) + (ASAN_SHADOW_OFFSET))
+      ((STRIP_TAG(mem) >> ASAN_SHADOW_SCALE) + (ASAN_SHADOW_OFFSET))
 #    define SHADOW_TO_MEM(mem) \
       (((mem) - (ASAN_SHADOW_OFFSET)) << (ASAN_SHADOW_SCALE))
 
@@ -377,6 +384,7 @@ static inline uptr MemToShadowSize(uptr size) {
 
 static inline bool AddrIsInMem(uptr a) {
   PROFILE_ASAN_MAPPING();
+  a = STRIP_TAG(a);
   return AddrIsInLowMem(a) || AddrIsInMidMem(a) || AddrIsInHighMem(a) ||
          (flags()->protect_shadow_gap == 0 && AddrIsInShadowGap(a));
 }
@@ -389,6 +397,7 @@ static inline uptr MemToShadow(uptr p) {
 
 static inline bool AddrIsInShadow(uptr a) {
   PROFILE_ASAN_MAPPING();
+  a = STRIP_TAG(a);
   return AddrIsInLowShadow(a) || AddrIsInMidShadow(a) || AddrIsInHighShadow(a);
 }
 
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform.h b/compiler-rt/lib/tsan/rtl/tsan_platform.h
index 00b493bf2d931..fc7d1be61ca4c 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform.h
@@ -947,6 +947,16 @@ uptr MetaShadowBeg(void) { return SelectMapping<MappingField>(kMetaShadowBeg); }
 ALWAYS_INLINE
 uptr MetaShadowEnd(void) { return SelectMapping<MappingField>(kMetaShadowEnd); }
 
+ALWAYS_INLINE
+uptr StripTag(uptr addr) {
+#if SANITIZER_APPLE
+  constexpr uptr kTagMask = ((uptr)0x0f << 56);  // Lower half of top byte
+  return addr & ~kTagMask;
+#else
+  return addr;
+#endif
+}
+
 struct IsAppMemImpl {
   template <typename Mapping>
   static bool Apply(uptr mem) {
@@ -958,7 +968,7 @@ struct IsAppMemImpl {
 };
 
 ALWAYS_INLINE
-bool IsAppMem(uptr mem) { return SelectMapping<IsAppMemImpl>(mem); }
+bool IsAppMem(uptr mem) { return SelectMapping<IsAppMemImpl>(StripTag(mem)); }
 
 struct IsShadowMemImpl {
   template <typename Mapping>
@@ -997,7 +1007,7 @@ struct MemToShadowImpl {
 
 ALWAYS_INLINE
 RawShadow *MemToShadow(uptr x) {
-  return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(x));
+  return reinterpret_cast<RawShadow *>(SelectMapping<MemToShadowImpl>(StripTag(mem)));
 }
 
 struct MemToMetaImpl {
@@ -1011,7 +1021,7 @@ struct MemToMetaImpl {
 };
 
 ALWAYS_INLINE
-u32 *MemToMeta(uptr x) { return SelectMapping<MemToMetaImpl>(x); }
+u32 *MemToMeta(uptr x) { return SelectMapping<MemToMetaImpl>(StripTag(x)); }
 
 struct ShadowToMemImpl {
   template <typename Mapping>

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[thurstond]

Would StripTag be better off in sanitizer_common?

P.S. HWASan has a similar function, UntagAddr.

[vitalybuka]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

[vitalybuka]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

Also Tsan has its own allocator, which does not tag.

[DanBlackwell]

Would StripTag be better off in sanitizer_common?

P.S. HWASan has a similar function, UntagAddr.

This is true, but HWASan uses all 8 bits of TBI [63:56]; while MTE uses only [59:56]. Perhaps StripTag should be named StripMTEtag in order to avoid future confusion? I do agree that this could move up to sanitizer_common.

[DanBlackwell]

ASAN and TSAN need to strip tags in order to compute the correct shadow addresses.

What is the point of running Asan with MTE?

Also Tsan has its own allocator, which does not tag.

Other linked (uninstrumented) libraries can use their own MTE-backed allocator, but Asan / Tsan will still intercept their calls and then IsAddrInMem and friends could misclassify them (due to the tag making it appear to be an ultra-high address).

[thurstond]

Would StripTag be better off in sanitizer_common?
P.S. HWASan has a similar function, UntagAddr.

This is true, but HWASan uses all 8 bits of TBI [63:56]; while MTE uses only [59:56].

Ah, good point!

Perhaps StripTag should be named StripMTEtag in order to avoid future confusion?

Sure. I would also be partial to something like "UntagMTEAddr", for consistency with HWASan.

[ndrewh]

I've renamed the macro to STRIP_MTE_TAG and moved it into sanitizer_common (sanitizer_platform.h seemed appropriate) and updated asan and tsan accordingly.

I have a slight preference for the macro over an inline function, is there a good reason to also have the inline function?

[thurstond]

I've renamed the macro to STRIP_MTE_TAG and moved it into sanitizer_common (sanitizer_platform.h seemed appropriate) and updated asan and tsan accordingly.

Thanks!

I have a slight preference for the macro over an inline function, is there a good reason to also have the inline function?

I guess it's simple enough that there isn't a need for the inline function.

[fmayer]

there a good reason to also have the inline function?

I don't care a lot, but IMO the question should be the other way round. Things should be an (inline) function unless they have to a macro.

[yln]

What is the point of running Asan with MTE?

tl;dr
It's pointless, but we want to enable the use case of applying ASan to your MTE-enabled binary.

Address Sanitizer & MTE compatibility
ASan takes over the memory allocator, so the system allocator (libmalloc) does not manage any allocations and therefore even when we run with MTE in a process, no allocations are tagged. However, a few other system components (other than libmalloc) might still manage and tag their own allocations and when those allocations are used in APIs that ASan intercepts, then we observe tagged pointers in those interceptors.
In this sense, when we run with both ASan & MTE, then ASan “wins” and MTE does nothing. So this configuration is pointless, but we should still avoid crashing so you can temporarily use ASan on your MTE-entitled binary without worrying about it.

cc: @vitalybuka

[fmayer]

In this sense, when we run with both ASan & MTE, then ASan “wins” and MTE does nothing.

+1. We do the same for HWASan and MTE on Android. For HWASan binaries, the linker just disables MTE.

[ndrewh]

I'm going to merge this as-is and leave conversion of these macros to inline functions to a future PR:

If I converted this to an inline function, it ought not stay in sanitizer_platform.h because that file is for "macros" (per the description at the top of the file). There is a file for ptrauth-related macros (note: macros not inline functions), but I am considering unifying that with the new mte-related macros (since they both pertain to the upper bits of pointers), and we can replace all of these with inline functions at the same time.