From 827c83497bf4eb9816de2c223ea5c054e19954af Mon Sep 17 00:00:00 2001
From: Jan Vorlicek <janvorli@microsoft.com>
Date: Sat, 27 Jul 2024 06:01:09 +0200
Subject: [PATCH] Remove CONTEXT_XSTATE in
 FaultingExceptionFrame::UpdateRegDisplay (#105569)

* Remove CONTEXT_XSTATE in FaultingExceptionFrame::UpdateRegDisplay

There is a bug in updating REGDISPLAY from a faulting exception frame.
The context stored in the frame can contain extended state, but we only
copy the basic CONTEXT part. But we are not removing the CONTEXT_XSTATE
flag. There was an issue found on arm64 Windows with SVE enabled. The
context from a hardware exception contains the SVE extended state and
when we resume after catch for the exception or start propagating it
through native frames, the RtlRestoreContext probably uses some garbage
to try to restore the extended state and ends up corrupting memory.

The fix is to remove the CONTEXT_XSTATE flag from the context after we
copy it to the REGDISPLAY.

While we have hit this problem on Windows ARM64 with SVE only, I have
made the same change for other targets that can have extended state too.

Close #105483

* Move the CONTEXT_XSTATE definition to clrnt.h

---------

Co-authored-by: Jan Kotas <jkotas@microsoft.com>
---
 src/coreclr/inc/clrnt.h            | 18 ++++++++++++++++++
 src/coreclr/vm/amd64/cgenamd64.cpp |  4 ++++
 src/coreclr/vm/arm64/stubs.cpp     |  4 ++++
 src/coreclr/vm/i386/cgenx86.cpp    |  4 ++++
 src/coreclr/vm/threadsuspend.cpp   | 20 --------------------
 5 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/src/coreclr/inc/clrnt.h b/src/coreclr/inc/clrnt.h
index cacc865b715f0..bfea4fdf6cae7 100644
--- a/src/coreclr/inc/clrnt.h
+++ b/src/coreclr/inc/clrnt.h
@@ -369,6 +369,24 @@ RtlVirtualUnwind(
     IN OUT PKNONVOLATILE_CONTEXT_POINTERS ContextPointers OPTIONAL
     );
 
+// Mirror the XSTATE_ARM64_SVE flags from winnt.h
+
+#ifndef XSTATE_ARM64_SVE
+#define XSTATE_ARM64_SVE (2)
+#endif // XSTATE_ARM64_SVE
+
+#ifndef XSTATE_MASK_ARM64_SVE
+#define XSTATE_MASK_ARM64_SVE (1ui64 << (XSTATE_ARM64_SVE))
+#endif // XSTATE_MASK_ARM64_SVE
+
+#ifndef CONTEXT_ARM64_XSTATE
+#define CONTEXT_ARM64_XSTATE (CONTEXT_ARM64 | 0x20L)
+#endif // CONTEXT_ARM64_XSTATE
+
+#ifndef CONTEXT_XSTATE
+#define CONTEXT_XSTATE CONTEXT_ARM64_XSTATE
+#endif // CONTEXT_XSTATE
+
 #endif
 
 #ifdef TARGET_LOONGARCH64
diff --git a/src/coreclr/vm/amd64/cgenamd64.cpp b/src/coreclr/vm/amd64/cgenamd64.cpp
index a7ec8d33f050b..4927d2d13a0d4 100644
--- a/src/coreclr/vm/amd64/cgenamd64.cpp
+++ b/src/coreclr/vm/amd64/cgenamd64.cpp
@@ -231,6 +231,10 @@ void FaultingExceptionFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool update
 
     memcpy(pRD->pCurrentContext, &m_ctx, sizeof(CONTEXT));
 
+    // Clear the CONTEXT_XSTATE, since the REGDISPLAY contains just plain CONTEXT structure
+    // that cannot contain any extended state.
+    pRD->pCurrentContext->ContextFlags &= ~CONTEXT_XSTATE;
+
     pRD->ControlPC = m_ctx.Rip;
 
     pRD->SP = m_ctx.Rsp;
diff --git a/src/coreclr/vm/arm64/stubs.cpp b/src/coreclr/vm/arm64/stubs.cpp
index 25a62472d1e6b..dadcc2894932c 100644
--- a/src/coreclr/vm/arm64/stubs.cpp
+++ b/src/coreclr/vm/arm64/stubs.cpp
@@ -635,6 +635,10 @@ void FaultingExceptionFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool update
     // Copy the context to regdisplay
     memcpy(pRD->pCurrentContext, &m_ctx, sizeof(T_CONTEXT));
 
+    // Clear the CONTEXT_XSTATE, since the REGDISPLAY contains just plain CONTEXT structure
+    // that cannot contain any extended state.
+    pRD->pCurrentContext->ContextFlags &= ~CONTEXT_XSTATE;
+
     pRD->ControlPC = ::GetIP(&m_ctx);
     pRD->SP = ::GetSP(&m_ctx);
 
diff --git a/src/coreclr/vm/i386/cgenx86.cpp b/src/coreclr/vm/i386/cgenx86.cpp
index 1738fd0490504..2dc84b4c5959a 100644
--- a/src/coreclr/vm/i386/cgenx86.cpp
+++ b/src/coreclr/vm/i386/cgenx86.cpp
@@ -478,6 +478,10 @@ void FaultingExceptionFrame::UpdateRegDisplay(const PREGDISPLAY pRD, bool update
 
     memcpy(pRD->pCurrentContext, &m_ctx, sizeof(CONTEXT));
 
+    // Clear the CONTEXT_XSTATE, since the REGDISPLAY contains just plain CONTEXT structure
+    // that cannot contain any extended state.
+    pRD->pCurrentContext->ContextFlags &= ~CONTEXT_XSTATE;
+
     pRD->SP = m_ctx.Esp;
     pRD->ControlPC = m_ctx.Eip;
 
diff --git a/src/coreclr/vm/threadsuspend.cpp b/src/coreclr/vm/threadsuspend.cpp
index e9b9ab99b3737..f8ecdcba72417 100644
--- a/src/coreclr/vm/threadsuspend.cpp
+++ b/src/coreclr/vm/threadsuspend.cpp
@@ -28,26 +28,6 @@ ThreadSuspend::SUSPEND_REASON ThreadSuspend::m_suspendReason;
 void* ThreadSuspend::g_returnAddressHijackTarget = NULL;
 #endif // TARGET_WINDOWS
 
-#if defined(TARGET_ARM64)
-// Mirror the XSTATE_ARM64_SVE flags from winnt.h
-
-#ifndef XSTATE_ARM64_SVE
-#define XSTATE_ARM64_SVE (2)
-#endif // XSTATE_ARM64_SVE
-
-#ifndef XSTATE_MASK_ARM64_SVE
-#define XSTATE_MASK_ARM64_SVE (1ui64 << (XSTATE_ARM64_SVE))
-#endif // XSTATE_MASK_ARM64_SVE
-
-#ifndef CONTEXT_ARM64_XSTATE
-#define CONTEXT_ARM64_XSTATE (CONTEXT_ARM64 | 0x20L)
-#endif // CONTEXT_ARM64_XSTATE
-
-#ifndef CONTEXT_XSTATE
-#define CONTEXT_XSTATE CONTEXT_ARM64_XSTATE
-#endif // CONTEXT_XSTATE
-#endif // TARGET_ARM64
-
 // If you add any thread redirection function, make sure the debugger can 1) recognize the redirection
 // function, and 2) retrieve the original CONTEXT.  See code:Debugger.InitializeHijackFunctionAddress and
 // code:DacDbiInterfaceImpl.RetrieveHijackedContext.