[compiler-rt] [HWASAN]Implement memcmp interceptor in HWASAN (PR #67204)
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 29 15:59:40 PDT 2023
https://github.com/vitalybuka updated https://github.com/llvm/llvm-project/pull/67204
>From 07c005c4110854fe66f5c126c54bb45c2fffb137 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 22 Sep 2023 22:57:55 +0000
Subject: [PATCH 1/9] [HWASAN] Implement memcmp interceptor in HWASAN
DON NOT SUBMIT - NEED TO TEST ON ARM
---
.../lib/hwasan/hwasan_interceptors.cpp | 23 +++++++++++++++----
.../lib/hwasan/hwasan_platform_interceptors.h | 2 +-
.../sanitizer_common_interceptors.inc | 4 +++-
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 17 ++++++++++++++
4 files changed, 39 insertions(+), 7 deletions(-)
create mode 100644 compiler-rt/test/hwasan/TestCases/memcmp.cpp
diff --git a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
index 92c8ec7cf55f412..f9af154ef2dcab2 100644
--- a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
@@ -31,6 +31,21 @@
using namespace __hwasan;
+struct HWAsanInterceptorContext {
+ const char *interceptor_name;
+};
+
+# define ACCESS_MEMORY_RANGE(ctx, offset, size, access) \
+ do { \
+ __hwasan::CheckAddressSized<ErrorAction::Abort, access>((uptr)offset, \
+ size); \
+ } while (0)
+
+#define HWASAN_READ_RANGE(ctx, offset, size) \
+ ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
+#define HWASAN_WRITE_RANGE(ctx, offset, size) \
+ ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
+
# if !SANITIZER_APPLE
# define HWASAN_INTERCEPT_FUNC(name) \
do { \
@@ -79,13 +94,11 @@ using namespace __hwasan;
} while (false)
# define COMMON_INTERCEPTOR_READ_RANGE(ctx, ptr, size) \
- do { \
- (void)(ctx); \
- (void)(ptr); \
- (void)(size); \
- } while (false)
+ HWASAN_READ_RANGE(ctx, ptr, size)
# define COMMON_INTERCEPTOR_ENTER(ctx, func, ...) \
+ HWAsanInterceptorContext _ctx = {#func}; \
+ ctx = (void *)&_ctx; \
do { \
(void)(ctx); \
(void)(func); \
diff --git a/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h b/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
index 33ae70a4ded90e5..9cc53201b2a800c 100644
--- a/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
+++ b/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
@@ -66,7 +66,7 @@
#define SANITIZER_INTERCEPT_MEMCPY 0
#undef SANITIZER_INTERCEPT_MEMCMP
-#define SANITIZER_INTERCEPT_MEMCMP 0
+#define SANITIZER_INTERCEPT_MEMCMP 1
#undef SANITIZER_INTERCEPT_BCMP
#define SANITIZER_INTERCEPT_BCMP 0
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
index 0e563fa12022a3e..80efaf54a0607f6 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
@@ -445,11 +445,13 @@ INTERCEPTOR(char*, textdomain, const char *domainname) {
#define INIT_TEXTDOMAIN
#endif
-#if SANITIZER_INTERCEPT_STRCMP
+#if SANITIZER_INTERCEPT_STRCMP || SANITIZER_INTERCEPT_MEMCMP
static inline int CharCmpX(unsigned char c1, unsigned char c2) {
return (c1 == c2) ? 0 : (c1 < c2) ? -1 : 1;
}
+#endif
+#if SANITIZER_INTERCEPT_STRCMP
DECLARE_WEAK_INTERCEPTOR_HOOK(__sanitizer_weak_hook_strcmp, uptr called_pc,
const char *s1, const char *s2, int result)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
new file mode 100644
index 000000000000000..231b17393b5684c
--- /dev/null
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -0,0 +1,17 @@
+// RUN: %clangxx_hwasan -O0 %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_hwasan -O1 %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_hwasan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_hwasan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s
+
+// REQUIRES: arm
+
+#include <string.h>
+int main(int argc, char **argv) {
+ char a1[] = {static_cast<char>(argc), 2, 3, 4};
+ char a2[] = {1, static_cast<char>(2*argc), 3, 4};
+ int res = memcmp(a1, a2, 4 + argc); // BOOM
+ // CHECK: AddressSanitizer: stack-buffer-overflow
+ // CHECK: {{#[0-9]+ .*memcmp}}
+ // CHECK: {{#[0-9]+ .*main}}
+ return res;
+}
>From 0f8281fe909d8ececbbeda2c840f069daa7d89b9 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Tue, 26 Sep 2023 17:35:42 +0000
Subject: [PATCH 2/9] Fixed test to pass on ARM.
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index 231b17393b5684c..2c22983b781f09e 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -3,15 +3,13 @@
// RUN: %clangxx_hwasan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_hwasan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s
-// REQUIRES: arm
+// REQUIRES: pointer-tagging
#include <string.h>
int main(int argc, char **argv) {
char a1[] = {static_cast<char>(argc), 2, 3, 4};
char a2[] = {1, static_cast<char>(2*argc), 3, 4};
int res = memcmp(a1, a2, 4 + argc); // BOOM
- // CHECK: AddressSanitizer: stack-buffer-overflow
- // CHECK: {{#[0-9]+ .*memcmp}}
- // CHECK: {{#[0-9]+ .*main}}
+ // CHECK: HWAddressSanitizer: tag-mismatch on address
return res;
}
>From 3cf5f0c2b4a3433d75fde74c89b7357eeb00165a Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Tue, 26 Sep 2023 17:39:37 +0000
Subject: [PATCH 3/9] Address comments.
---
.../lib/hwasan/hwasan_platform_interceptors.h | 4 ++--
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 15 +++++++++------
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h b/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
index 9cc53201b2a800c..390c9d80c38edd9 100644
--- a/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
+++ b/compiler-rt/lib/hwasan/hwasan_platform_interceptors.h
@@ -65,8 +65,8 @@
#undef SANITIZER_INTERCEPT_MEMCPY
#define SANITIZER_INTERCEPT_MEMCPY 0
-#undef SANITIZER_INTERCEPT_MEMCMP
-#define SANITIZER_INTERCEPT_MEMCMP 1
+// #undef SANITIZER_INTERCEPT_MEMCMP
+// #define SANITIZER_INTERCEPT_MEMCMP 0
#undef SANITIZER_INTERCEPT_BCMP
#define SANITIZER_INTERCEPT_BCMP 0
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index 2c22983b781f09e..e58e260047f908e 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -3,13 +3,16 @@
// RUN: %clangxx_hwasan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_hwasan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s
-// REQUIRES: pointer-tagging
-
#include <string.h>
+#include <stdlib.h>
+#include <sanitizer/hwasan_interface.h>
+
int main(int argc, char **argv) {
- char a1[] = {static_cast<char>(argc), 2, 3, 4};
- char a2[] = {1, static_cast<char>(2*argc), 3, 4};
- int res = memcmp(a1, a2, 4 + argc); // BOOM
+ __hwasan_enable_allocator_tagging();
+ char a[] = {static_cast<char>(argc), 2, 3, 4};
+ char *p = (char *)malloc(sizeof(a));
+ free(p);
+ memcpy(p, a, sizeof(a));
// CHECK: HWAddressSanitizer: tag-mismatch on address
- return res;
+ return memcmp(p, a, sizeof(a));
}
>From cd3b2bce1ba20e70f28ce1e71fcfa5e575887b91 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Thu, 28 Sep 2023 21:12:27 +0000
Subject: [PATCH 4/9] Add extra check for use-after-free
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index e58e260047f908e..37d2e8b8cf70189 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -14,5 +14,6 @@ int main(int argc, char **argv) {
free(p);
memcpy(p, a, sizeof(a));
// CHECK: HWAddressSanitizer: tag-mismatch on address
+ // CHECK: Cause: use-after-free
return memcmp(p, a, sizeof(a));
}
>From 40dd2f50e6cb5bfd91efc2fedb4a494eec2e65b5 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Thu, 28 Sep 2023 21:17:09 +0000
Subject: [PATCH 5/9] Fix format
---
compiler-rt/lib/hwasan/hwasan_interceptors.cpp | 8 ++++----
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 4 ++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
index f9af154ef2dcab2..0889831373a8039 100644
--- a/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
+++ b/compiler-rt/lib/hwasan/hwasan_interceptors.cpp
@@ -41,10 +41,10 @@ struct HWAsanInterceptorContext {
size); \
} while (0)
-#define HWASAN_READ_RANGE(ctx, offset, size) \
- ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
-#define HWASAN_WRITE_RANGE(ctx, offset, size) \
- ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
+# define HWASAN_READ_RANGE(ctx, offset, size) \
+ ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Load)
+# define HWASAN_WRITE_RANGE(ctx, offset, size) \
+ ACCESS_MEMORY_RANGE(ctx, offset, size, AccessType::Store)
# if !SANITIZER_APPLE
# define HWASAN_INTERCEPT_FUNC(name) \
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index 37d2e8b8cf70189..87228cdcb5b9194 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -3,9 +3,9 @@
// RUN: %clangxx_hwasan -O2 %s -o %t && not %run %t 2>&1 | FileCheck %s
// RUN: %clangxx_hwasan -O3 %s -o %t && not %run %t 2>&1 | FileCheck %s
-#include <string.h>
-#include <stdlib.h>
#include <sanitizer/hwasan_interface.h>
+#include <stdlib.h>
+#include <string.h>
int main(int argc, char **argv) {
__hwasan_enable_allocator_tagging();
>From d091065677da64fd17bdde8bebd0b58c09e66c9d Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Thu, 28 Sep 2023 21:47:55 +0000
Subject: [PATCH 6/9] Test is the way it was intended to be, but it fails.
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index 87228cdcb5b9194..9fab1a592d34005 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -11,8 +11,8 @@ int main(int argc, char **argv) {
__hwasan_enable_allocator_tagging();
char a[] = {static_cast<char>(argc), 2, 3, 4};
char *p = (char *)malloc(sizeof(a));
- free(p);
memcpy(p, a, sizeof(a));
+ free(p);
// CHECK: HWAddressSanitizer: tag-mismatch on address
// CHECK: Cause: use-after-free
return memcmp(p, a, sizeof(a));
>From d402588d0d56bf5a90b887b00d53515dcce8cd66 Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 29 Sep 2023 07:42:51 +0000
Subject: [PATCH 7/9] Fixed the test to actually use memcmp. The code before
got optimized out because the compiler knew it was 4 bytes so it just inlined
the check. This looks like a bug in HWASAN code generation because there was
no memory checks added to this code.
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index 9fab1a592d34005..d1265d5a927a367 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -6,14 +6,25 @@
#include <sanitizer/hwasan_interface.h>
#include <stdlib.h>
#include <string.h>
+#include <unistd.h>
+
+static __attribute__ ((__noinline__))
+char *MakeArray(char* a, int size, int* new_size) {
+ char *p = (char *)malloc(size);
+ *new_size = size;
+ memcpy(p, a, size);
+ return p;
+}
int main(int argc, char **argv) {
__hwasan_enable_allocator_tagging();
char a[] = {static_cast<char>(argc), 2, 3, 4};
- char *p = (char *)malloc(sizeof(a));
- memcpy(p, a, sizeof(a));
+ int size = 0;
+ char *p = MakeArray(a, sizeof(a), &size);
free(p);
// CHECK: HWAddressSanitizer: tag-mismatch on address
+ // CHECK: MemcmpInterceptorCommon
// CHECK: Cause: use-after-free
- return memcmp(p, a, sizeof(a));
+ int res = memcmp(p, a, size);
+ return res;
}
>From 3d90d11ae256960d9ca79fbfe79668195dfa93af Mon Sep 17 00:00:00 2001
From: Kirill Stoimenov <kstoimenov at google.com>
Date: Fri, 29 Sep 2023 07:46:01 +0000
Subject: [PATCH 8/9] Removed redundant variable.
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index d1265d5a927a367..db1d0390b4742ba 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -25,6 +25,5 @@ int main(int argc, char **argv) {
// CHECK: HWAddressSanitizer: tag-mismatch on address
// CHECK: MemcmpInterceptorCommon
// CHECK: Cause: use-after-free
- int res = memcmp(p, a, size);
- return res;
+ return memcmp(p, a, size);
}
>From deb5f65fb7826dddbe636b5e04b65434d1d32955 Mon Sep 17 00:00:00 2001
From: Vitaly Buka <vitalybuka at gmail.com>
Date: Fri, 29 Sep 2023 15:59:30 -0700
Subject: [PATCH 9/9] Simplify test
---
compiler-rt/test/hwasan/TestCases/memcmp.cpp | 23 ++++++++++----------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/compiler-rt/test/hwasan/TestCases/memcmp.cpp b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
index db1d0390b4742ba..15ada3082687516 100644
--- a/compiler-rt/test/hwasan/TestCases/memcmp.cpp
+++ b/compiler-rt/test/hwasan/TestCases/memcmp.cpp
@@ -8,22 +8,21 @@
#include <string.h>
#include <unistd.h>
-static __attribute__ ((__noinline__))
-char *MakeArray(char* a, int size, int* new_size) {
- char *p = (char *)malloc(size);
- *new_size = size;
- memcpy(p, a, size);
- return p;
-}
-
+char *p;
int main(int argc, char **argv) {
__hwasan_enable_allocator_tagging();
char a[] = {static_cast<char>(argc), 2, 3, 4};
- int size = 0;
- char *p = MakeArray(a, sizeof(a), &size);
+ volatile int size = sizeof(a);
+ char *volatile p = (char *)malloc(size);
+ memcpy(p, a, size);
free(p);
+ return memcmp(p, a, size);
// CHECK: HWAddressSanitizer: tag-mismatch on address
- // CHECK: MemcmpInterceptorCommon
+ // CHECK: READ of size 4
+ // CHECK: #{{[[:digit:]]+}} 0x{{[[:xdigit:]]+}} in main {{.*}}memcmp.cpp:[[@LINE-3]]
// CHECK: Cause: use-after-free
- return memcmp(p, a, size);
+ // CHECK: freed by thread
+ // CHECK: #{{[[:digit:]]+}} 0x{{[[:xdigit:]]+}} in main {{.*}}memcmp.cpp:[[@LINE-7]]
+ // CHECK: previously allocated by thread
+ // CHECK: #{{[[:digit:]]+}} 0x{{[[:xdigit:]]+}} in main {{.*}}memcmp.cpp:[[@LINE-11]]
}
More information about the llvm-commits
mailing list