[compiler-rt] [ASan][Win][compiler-rt] Fixes stack overflow when ntdll has mem* calls during exception handling (PR #120110)
Zack Johnson via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 17 06:12:47 PST 2024
https://github.com/zacklj89 updated https://github.com/llvm/llvm-project/pull/120110
>From b7325c4fbb3c71ae832a9c658334c5fda74d92cc Mon Sep 17 00:00:00 2001
From: Zack Johnson <zacklj89 at gmail.com>
Date: Mon, 16 Dec 2024 11:38:25 -0500
Subject: [PATCH 1/4] Fixes stack overflow when ntdll has mem* calls during
exception handling
---
.../asan/asan_interceptors_memintrinsics.cpp | 99 ++++++++++++++-----
compiler-rt/lib/asan/asan_poisoning.h | 31 ++++++
compiler-rt/lib/asan/asan_rtl.cpp | 4 +
.../lib/sanitizer_common/sanitizer_win.cpp | 12 +++
.../lib/sanitizer_common/sanitizer_win.h | 6 ++
5 files changed, 125 insertions(+), 27 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
index bdf328f8920634..0e27d868dff9b7 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -20,43 +20,88 @@
#include "asan_stack.h"
#include "asan_suppressions.h"
+#if SANITIZER_WINDOWS64
+# include "asan_poisoning.h"
+# include "sanitizer_common/sanitizer_win.h"
+#endif
+
+namespace __asan {
+// On x64, the ShadowExceptionHandler is expected to handle all AVs that happen
+// as a result of uncommitted shadow memory pages. However, in programs that use
+// ntdll (a Windows-specific library that contains some memory intrinsics as
+// well as Windows-specific exception handling mechanisms) as their C Runtime,
+// or in cases where ntdll uses mem* functions inside
+// its exception handling infrastructure, ASAN can end up rethrowing a shadow
+// memory AV until a stack overflow occurs. In other words, ntdll can call back
+// into ASAN for a poisoning check, which creates infinite recursion. To remedy
+// this, we precommit the shadow memory of the address being accessed on x64 for
+// ntdll callees.
+bool ShouldReplaceIntrinsic(bool isNtdllCallee, void *addr, uptr size,
+ const void *from = nullptr) {
+#if SANITIZER_WINDOWS64
+ if (isNtdllCallee) {
+ CommitShadowMemory(reinterpret_cast<uptr>(addr), size);
+ if (from) {
+ CommitShadowMemory(reinterpret_cast<uptr>(from), size);
+ }
+ }
+#endif
+ return replace_intrin_cached;
+}
+} // namespace __asan
+
using namespace __asan;
+#if SANITIZER_WINDOWS64
+#define IS_NTDLL_CALLEE __sanitizer::IsNtdllCallee(_ReturnAddress())
+#else
+#define IS_NTDLL_CALLEE false
+#endif
+
// memcpy is called during __asan_init() from the internals of printf(...).
// We do not treat memcpy with to==from as a bug.
// See http://llvm.org/bugs/show_bug.cgi?id=11763.
-#define ASAN_MEMCPY_IMPL(ctx, to, from, size) \
- do { \
- if (LIKELY(replace_intrin_cached)) { \
- if (LIKELY(to != from)) { \
- CHECK_RANGES_OVERLAP("memcpy", to, size, from, size); \
- } \
- ASAN_READ_RANGE(ctx, from, size); \
- ASAN_WRITE_RANGE(ctx, to, size); \
- } else if (UNLIKELY(!AsanInited())) { \
- return internal_memcpy(to, from, size); \
- } \
- return REAL(memcpy)(to, from, size); \
+#define ASAN_MEMCPY_IMPL(ctx, to, from, size) \
+ do { \
+ if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
+ return REAL(memcpy)(to, from, size); \
+ } \
+ if (LIKELY(replace_intrin_cached)) { \
+ if (LIKELY(to != from)) { \
+ CHECK_RANGES_OVERLAP("memcpy", to, size, from, size); \
+ } \
+ ASAN_READ_RANGE(ctx, from, size); \
+ ASAN_WRITE_RANGE(ctx, to, size); \
+ } else if (UNLIKELY(!AsanInited())) { \
+ return internal_memcpy(to, from, size); \
+ } \
+ return REAL(memcpy)(to, from, size); \
} while (0)
// memset is called inside Printf.
-#define ASAN_MEMSET_IMPL(ctx, block, c, size) \
- do { \
- if (LIKELY(replace_intrin_cached)) { \
- ASAN_WRITE_RANGE(ctx, block, size); \
- } else if (UNLIKELY(!AsanInited())) { \
- return internal_memset(block, c, size); \
- } \
- return REAL(memset)(block, c, size); \
+#define ASAN_MEMSET_IMPL(ctx, block, c, size) \
+ do { \
+ if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, block, size)) { \
+ return REAL(memset)(block, c, size); \
+ } \
+ if (LIKELY(replace_intrin_cached)) { \
+ ASAN_WRITE_RANGE(ctx, block, size); \
+ } else if (UNLIKELY(!AsanInited())) { \
+ return internal_memset(block, c, size); \
+ } \
+ return REAL(memset)(block, c, size); \
} while (0)
-#define ASAN_MEMMOVE_IMPL(ctx, to, from, size) \
- do { \
- if (LIKELY(replace_intrin_cached)) { \
- ASAN_READ_RANGE(ctx, from, size); \
- ASAN_WRITE_RANGE(ctx, to, size); \
- } \
- return internal_memmove(to, from, size); \
+#define ASAN_MEMMOVE_IMPL(ctx, to, from, size) \
+ do { \
+ if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
+ return internal_memmove(to, from, size); \
+ } \
+ if (LIKELY(replace_intrin_cached)) { \
+ ASAN_READ_RANGE(ctx, from, size); \
+ ASAN_WRITE_RANGE(ctx, to, size); \
+ } \
+ return internal_memmove(to, from, size); \
} while (0)
void *__asan_memcpy(void *to, const void *from, uptr size) {
diff --git a/compiler-rt/lib/asan/asan_poisoning.h b/compiler-rt/lib/asan/asan_poisoning.h
index 600bd011f304cb..7fb12b4d828b5c 100644
--- a/compiler-rt/lib/asan/asan_poisoning.h
+++ b/compiler-rt/lib/asan/asan_poisoning.h
@@ -17,6 +17,25 @@
#include "sanitizer_common/sanitizer_flags.h"
#include "sanitizer_common/sanitizer_platform.h"
+#if SANITIZER_WINDOWS64
+# include "sanitizer_common/sanitizer_win.h"
+# include "sanitizer_common/sanitizer_win_defs.h"
+
+// These definitions are duplicated from Window.h in order to avoid conflicts
+// with other types in Windows.h.
+// These functions and types are used to manipulate the shadow memory on
+// x64 Windows.
+typedef unsigned long DWORD;
+typedef void *LPVOID;
+typedef int BOOL;
+
+constexpr DWORD MEM_COMMIT = 0x00001000;
+constexpr DWORD MEM_DECOMMIT = 0x00004000;
+constexpr DWORD PAGE_READWRITE = 0x04;
+
+extern "C" LPVOID WINAPI VirtualAlloc(LPVOID, size_t, DWORD, DWORD);
+#endif
+
namespace __asan {
// Enable/disable memory poisoning.
@@ -95,4 +114,16 @@ ALWAYS_INLINE void FastPoisonShadowPartialRightRedzone(
// [MemToShadow(p), MemToShadow(p+size)].
void FlushUnneededASanShadowMemory(uptr p, uptr size);
+// Commits the shadow memory for a range of aligned memory. This only matters
+// on 64-bit Windows where relying on pages to get paged in on access
+// violation is inefficient when we know the memory range ahead of time.
+ALWAYS_INLINE void CommitShadowMemory(uptr aligned_beg, uptr aligned_size) {
+#if SANITIZER_WINDOWS64
+ uptr shadow_beg = MEM_TO_SHADOW(aligned_beg);
+ uptr shadow_end =
+ MEM_TO_SHADOW(aligned_beg + aligned_size - ASAN_SHADOW_GRANULARITY) + 1;
+ ::VirtualAlloc((LPVOID)shadow_beg, (size_t)(shadow_end - shadow_beg),
+ MEM_COMMIT, PAGE_READWRITE);
+#endif
+}
} // namespace __asan
diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index 19c6c210b564c5..7c88ec2426a16a 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -463,6 +463,10 @@ static bool AsanInitInternal() {
if (SANITIZER_START_BACKGROUND_THREAD_IN_ASAN_INTERNAL)
MaybeStartBackgroudThread();
+#if SANITIZER_WINDOWS64
+ __sanitizer::InitializeNtdllInfo();
+#endif
+
// On Linux AsanThread::ThreadStart() calls malloc() that's why asan_inited
// should be set to 1 prior to initializing the threads.
replace_intrin_cached = flags()->replace_intrin;
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
index fd0f989ee392bb..e3ecdd958bdfb7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
@@ -1264,6 +1264,18 @@ void LogFullErrorReport(const char *buffer) {
void InitializePlatformCommonFlags(CommonFlags *cf) {}
+static MODULEINFO ntdllInfo;
+void InitializeNtdllInfo() {
+ GetModuleInformation(GetCurrentProcess(), GetModuleHandle(L"ntdll.dll"),
+ &ntdllInfo, sizeof(ntdllInfo));
+}
+
+bool IsNtdllCallee(void *calleeAddr) {
+ return (uptr)ntdllInfo.lpBaseOfDll <= (uptr)calleeAddr &&
+ ((uptr)ntdllInfo.lpBaseOfDll + (uptr)ntdllInfo.SizeOfImage) >=
+ (uptr)calleeAddr;
+}
+
} // namespace __sanitizer
#endif // _WIN32
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.h b/compiler-rt/lib/sanitizer_common/sanitizer_win.h
index ff8939ca5e8557..e1080f4ce86d32 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.h
@@ -19,6 +19,12 @@
namespace __sanitizer {
// Check based on flags if we should handle the exception.
bool IsHandledDeadlyException(DWORD exceptionCode);
+
+// Initializes module information of ntdll for referencing callee addresses
+void InitializeNtdllInfo();
+
+// Returns whether or not the callee address lies within ntdll
+bool IsNtdllCallee(void* calleeAddr);
} // namespace __sanitizer
#endif // SANITIZER_WINDOWS
>From 5028b32a742a800643aec5bd27369c06f4f71b13 Mon Sep 17 00:00:00 2001
From: Zack Johnson <zacklj89 at gmail.com>
Date: Mon, 16 Dec 2024 15:45:04 -0500
Subject: [PATCH 2/4] clang-format fixes
---
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
index 0e27d868dff9b7..68b02f981aa416 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -53,9 +53,9 @@ bool ShouldReplaceIntrinsic(bool isNtdllCallee, void *addr, uptr size,
using namespace __asan;
#if SANITIZER_WINDOWS64
-#define IS_NTDLL_CALLEE __sanitizer::IsNtdllCallee(_ReturnAddress())
+# define IS_NTDLL_CALLEE __sanitizer::IsNtdllCallee(_ReturnAddress())
#else
-#define IS_NTDLL_CALLEE false
+# define IS_NTDLL_CALLEE false
#endif
// memcpy is called during __asan_init() from the internals of printf(...).
>From da6531039c36c7e0f6c2267b9a9c155e2bd26fa7 Mon Sep 17 00:00:00 2001
From: Zack Johnson <zacklj89 at gmail.com>
Date: Mon, 16 Dec 2024 15:57:13 -0500
Subject: [PATCH 3/4] maintain last error; add regression test
---
.../asan/asan_interceptors_memintrinsics.cpp | 10 ++++++-
.../Windows/ntdll_regression_tests.cpp | 26 +++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 compiler-rt/test/asan/TestCases/Windows/ntdll_regression_tests.cpp
diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
index 68b02f981aa416..875e4e1cfa7f18 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -23,6 +23,9 @@
#if SANITIZER_WINDOWS64
# include "asan_poisoning.h"
# include "sanitizer_common/sanitizer_win.h"
+
+extern "C" void WINAPI RestoreLastError(DWORD);
+extern "C" DWORD WINAPI GetLastError();
#endif
namespace __asan {
@@ -40,10 +43,15 @@ bool ShouldReplaceIntrinsic(bool isNtdllCallee, void *addr, uptr size,
const void *from = nullptr) {
#if SANITIZER_WINDOWS64
if (isNtdllCallee) {
- CommitShadowMemory(reinterpret_cast<uptr>(addr), size);
+ const auto lastError = GetLastError(); // Grab last error here to maintain
+ // status for after internal calls
+ if (addr) {
+ CommitShadowMemory(reinterpret_cast<uptr>(addr), size);
+ }
if (from) {
CommitShadowMemory(reinterpret_cast<uptr>(from), size);
}
+ RestoreLastError(lastError);
}
#endif
return replace_intrin_cached;
diff --git a/compiler-rt/test/asan/TestCases/Windows/ntdll_regression_tests.cpp b/compiler-rt/test/asan/TestCases/Windows/ntdll_regression_tests.cpp
new file mode 100644
index 00000000000000..b6a951b58e3ee0
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/Windows/ntdll_regression_tests.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cl_asan /Od %s -Fe%t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+#include <Windows.h>
+#include <iostream>
+
+// Small sanity test to make sure ASAN does not stomp on
+// GetLastError values. This is motivated by __asan::ShouldReplaceIntrinsic,
+// which remedied infinite recursion due to ntdll exception handling paths calling
+// instrumented functions on startup before shadow memory can be committed.
+int TestNTStatusMaintained() {
+ ::SetLastError(ERROR_SUCCESS);
+ constexpr unsigned long c_initialSizeGuess = 1;
+ wchar_t szBuffer[c_initialSizeGuess];
+ unsigned long size = ::GetDllDirectoryW(c_initialSizeGuess, szBuffer);
+ auto le = ::GetLastError();
+ if (size == 0 && le != ERROR_SUCCESS) {
+ std::cerr << "Last error is different.\n";
+ return -1;
+ }
+ std::cerr << "Success.\n";
+ return 0;
+ // CHECK: Success.
+}
+
+int main() { return TestNTStatusMaintained(); }
\ No newline at end of file
>From e609aa1693cd842a336e78e4f92c9b7d328c3497 Mon Sep 17 00:00:00 2001
From: Zack Johnson <zacklj89 at gmail.com>
Date: Tue, 17 Dec 2024 09:12:33 -0500
Subject: [PATCH 4/4] Fix platform specific checks
---
compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
index 875e4e1cfa7f18..6f6196f076314e 100644
--- a/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
+++ b/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp
@@ -71,7 +71,8 @@ using namespace __asan;
// See http://llvm.org/bugs/show_bug.cgi?id=11763.
#define ASAN_MEMCPY_IMPL(ctx, to, from, size) \
do { \
- if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
+ if (SANITIZER_WINDOWS64 && \
+ !ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
return REAL(memcpy)(to, from, size); \
} \
if (LIKELY(replace_intrin_cached)) { \
@@ -89,7 +90,8 @@ using namespace __asan;
// memset is called inside Printf.
#define ASAN_MEMSET_IMPL(ctx, block, c, size) \
do { \
- if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, block, size)) { \
+ if (SANITIZER_WINDOWS64 && \
+ !ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, block, size)) { \
return REAL(memset)(block, c, size); \
} \
if (LIKELY(replace_intrin_cached)) { \
@@ -102,7 +104,8 @@ using namespace __asan;
#define ASAN_MEMMOVE_IMPL(ctx, to, from, size) \
do { \
- if (!ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
+ if (SANITIZER_WINDOWS64 && \
+ !ShouldReplaceIntrinsic(IS_NTDLL_CALLEE, to, size, from)) { \
return internal_memmove(to, from, size); \
} \
if (LIKELY(replace_intrin_cached)) { \
More information about the llvm-commits
mailing list