Merge pull request #2640 from janvorli/fix-stack-overflow

Fix Unix stack overflow detection

5 files changed, 119 insertions, 74 deletions

diff --git a/src/pal/src/exception/machexception.cpp b/src/pal/src/exception/machexception.cpp
index bf6d83c1c6..4142e7c3e9 100644
--- a/src/pal/src/exception/machexception.cpp
+++ b/src/pal/src/exception/machexception.cpp
@@ -801,25 +801,19 @@ catch_exception_raise(
     // pthread easily and (b) the pthread functions lie about the bounds on the main thread.
 
     // Instead we inspect the target thread SP we just retrieved above and compare it with the AV address. If
-    // they both lie in the same page or the SP is at a higher address than the AV but still reasonably close
-    // (we'll define close below) then we'll consider the AV to be an SO. Note that we can't assume that SP
-    // will be in the same page as the AV on an SO, even though we force GCC to generate stack probes on stack
-    // extension (-fstack-check). That's because GCC currently generates the probe *before* altering SP.
-    // Since a given stack extension can involve multiple pages and GCC generates all the required probes
-    // before updating SP in a single operation, the faulting probe can be at an address that is far removed
-    // from the thread's current value of SP.
-
-    // To work around this we'll first bound the definition of "close" to 512KB. This is the current size of
-    // pthread stacks by default. While it's true that this value can be altered for a given thread (and the
-    // main thread for that matter usually starts with an 8MB stack) I think it is reasonable to assume that a
-    // single stack frame, alloca etc. should never be extending the stack this much in one go.
-
-    // If we pass this check then we'll confirm it (in the case where the AV and SP aren't in the same or
-    // adjacent pages) by checking that the first page following the faulting address belongs in the same VM
-    // region as the current value of SP. Since all pages in a VM region have the same attributes this check
-    // eliminates the possibility that there's another guard page in the range between the fault and the SP,
-    // effectively establishing that the AV occurred in the guard page associated with the stack associated
-    // with the SP.
+    // they both lie in the same page or the SP is at a higher address than the AV but in the same VM region,
+    // then we'll consider the AV to be an SO. Note that we can't assume that SP will be in the same page as
+    // the AV on an SO, even though we force GCC to generate stack probes on stack extension (-fstack-check).
+    // That's because GCC currently generates the probe *before* altering SP. Since a given stack extension can
+    // involve multiple pages and GCC generates all the required probes before updating SP in a single
+    // operation, the faulting probe can be at an address that is far removed from the thread's current value
+    // of SP.
+
+    // In the case where the AV and SP aren't in the same or adjacent pages we check if the first page
+    // following the faulting address belongs in the same VM region as the current value of SP. Since all pages
+    // in a VM region have the same attributes this check eliminates the possibility that there's another guard
+    // page in the range between the fault and the SP, effectively establishing that the AV occurred in the
+    // guard page associated with the stack associated with the SP.
 
     // We are assuming here that thread stacks are always allocated in a single VM region. I've seen no
     // evidence thus far that this is not the case (and the mere fact we rely on Mach apis already puts us on
@@ -857,10 +851,9 @@ catch_exception_raise(
             // The easy case is when the AV occurred in the same or adjacent page as the stack pointer.
             fIsStackOverflow = true;
         }
-        else if (pFaultPage < pStackTopPage && (pStackTopPage - pFaultPage) < (512 * 1024))
+        else if (pFaultPage < pStackTopPage)
         {
-            // If the two addresses look fairly close together (the size of the average pthread stack) we'll
-            // dig deeper. Calculate the address of the page immediately following the fault and check that it
+            // Calculate the address of the page immediately following the fault and check that it
             // lies in the same VM region as the stack pointer.
             vm_address_t vm_address;
             vm_size_t vm_size;
@@ -938,6 +931,7 @@ catch_exception_raise(
             // thread's stack any further. Note that we cannot call most PAL functions from the context of
             // this thread since we're not a PAL thread.
 
+            write(STDERR_FILENO, StackOverflowMessage, sizeof(StackOverflowMessage) - 1);
             abort();
         }
     }
diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp
index 69519d4a87..ef7b569f69 100644
--- a/src/pal/src/exception/signal.cpp
+++ b/src/pal/src/exception/signal.cpp
@@ -283,6 +283,21 @@ static void sigsegv_handler(int code, siginfo_t *siginfo, void *context)
         record.ExceptionAddress = GetNativeContextPC(ucontext);
         record.NumberParameters = 2;
 
+        if (record.ExceptionCode == EXCEPTION_ACCESS_VIOLATION)
+        {
+            // Check if the failed access has hit a stack guard page. In such case, it
+            // was a stack probe that detected that there is not enough stack left.
+            void* stackLimit = CPalThread::GetStackLimit();
+            void* stackGuard = (void*)((size_t)stackLimit - getpagesize());
+            if ((siginfo->si_addr >= stackGuard) && (siginfo->si_addr < stackLimit))
+            {
+                // The exception happened in the page right below the stack limit,
+                // so it is a stack overflow
+                write(STDERR_FILENO, StackOverflowMessage, sizeof(StackOverflowMessage) - 1);
+                abort();
+            }
+        }
+
         // TODO: First parameter says whether a read (0) or write (non-0) caused the
         // fault. We must disassemble the instruction at record.ExceptionAddress
         // to correctly fill in this value.
diff --git a/src/pal/src/include/pal/palinternal.h b/src/pal/src/include/pal/palinternal.h
index c059db545c..a50456164a 100644
--- a/src/pal/src/include/pal/palinternal.h
+++ b/src/pal/src/include/pal/palinternal.h
@@ -689,6 +689,8 @@ inline T* InterlockedCompareExchangePointerT(
 
 #include "volatile.h"
 
+const char StackOverflowMessage[] = "Process is terminated due to StackOverflowException.\n";
+
 #endif // __cplusplus
 
 #endif /* _PAL_INTERNAL_H_ */
diff --git a/src/pal/src/include/pal/thread.hpp b/src/pal/src/include/pal/thread.hpp
index ae695d38cf..307c8fbec1 100644
--- a/src/pal/src/include/pal/thread.hpp
+++ b/src/pal/src/include/pal/thread.hpp
@@ -673,19 +673,33 @@ namespace CorUnix
             void
             );
 
-        // Get base address of this thread's stack
-        // Can be called only for the current thread.
+        // Get base address of the current thread's stack
+        static
         void *
         GetStackBase(
             void
             );
 
-        // Get limit address of this thread's stack
+        // Get cached base address of this thread's stack
         // Can be called only for the current thread.
         void *
+        GetCachedStackBase(
+            void
+            );
+
+        // Get limit address of the current thread's stack
+        static
+        void *
         GetStackLimit(
             void
             );
+
+        // Get cached limit address of this thread's stack
+        // Can be called only for the current thread.
+        void *
+        GetCachedStackLimit(
+            void
+            );
         
 #ifdef FEATURE_PAL_SXS
         //
diff --git a/src/pal/src/thread/thread.cpp b/src/pal/src/thread/thread.cpp
index 69466ecc04..33807154f5 100644
--- a/src/pal/src/thread/thread.cpp
+++ b/src/pal/src/thread/thread.cpp
@@ -2505,89 +2505,109 @@ ThreadInitializationRoutine(
     return NO_ERROR;
 }
 
-// Get base address of this thread's stack
-// Can be called only for the current thread.
+// Get base address of the current thread's stack
 void *
 CPalThread::GetStackBase()
 {
-    _ASSERT_MSG(this == InternalGetCurrentThread(), "CPalThread::GetStackBase called from foreign thread");
-
-    if (m_stackBase == NULL)
-    {
+    void* stackBase;
 #ifdef _TARGET_MAC64
-        // This is a Mac specific method
-        m_stackBase = pthread_get_stackaddr_np(pthread_self());
+    // This is a Mac specific method
+    stackBase = pthread_get_stackaddr_np(pthread_self());
 #else
-        pthread_attr_t attr;
-        void* stackAddr;
-        size_t stackSize;
-        int status;
+    pthread_attr_t attr;
+    void* stackAddr;
+    size_t stackSize;
+    int status;
 
-        pthread_t thread = pthread_self();
+    pthread_t thread = pthread_self();
 
-        status = pthread_attr_init(&attr);
-        _ASSERT_MSG(status == 0, "pthread_attr_init call failed");
+    status = pthread_attr_init(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_init call failed");
 
 #if HAVE_PTHREAD_ATTR_GET_NP
-        status = pthread_attr_get_np(thread, &attr);
+    status = pthread_attr_get_np(thread, &attr);
 #elif HAVE_PTHREAD_GETATTR_NP
-        status = pthread_getattr_np(thread, &attr);
+    status = pthread_getattr_np(thread, &attr);
 #else
 #error Dont know how to get thread attributes on this platform!
 #endif
-        _ASSERT_MSG(status == 0, "pthread_getattr_np call failed");
+    _ASSERT_MSG(status == 0, "pthread_getattr_np call failed");
 
-        status = pthread_attr_getstack(&attr, &stackAddr, &stackSize);
-        _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
+    status = pthread_attr_getstack(&attr, &stackAddr, &stackSize);
+    _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
 
-        status = pthread_attr_destroy(&attr);
-        _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
+    status = pthread_attr_destroy(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
 
-        m_stackBase = (void*)((size_t)stackAddr + stackSize);
+    stackBase = (void*)((size_t)stackAddr + stackSize);
 #endif
-    }
 
-    return m_stackBase;
+    return stackBase;
 }
 
-// Get limit address of this thread's stack.
-// Can be called only for the current thread.
+// Get limit address of the current thread's stack
 void *
 CPalThread::GetStackLimit()
 {
-    _ASSERT_MSG(this == InternalGetCurrentThread(), "CPalThread::GetStackLimit called from foreign thread");
-
-    if (m_stackLimit == NULL)
-    {
+    void* stackLimit;
 #ifdef _TARGET_MAC64
-        // This is a Mac specific method
-        m_stackLimit = ((BYTE *)pthread_get_stackaddr_np(pthread_self()) -
-                       pthread_get_stacksize_np(pthread_self()));
+    // This is a Mac specific method
+    stackLimit = ((BYTE *)pthread_get_stackaddr_np(pthread_self()) -
+                   pthread_get_stacksize_np(pthread_self()));
 #else
-        pthread_attr_t attr;
-        size_t stackSize;
-        int status;
+    pthread_attr_t attr;
+    size_t stackSize;
+    int status;
 
-        pthread_t thread = pthread_self();
+    pthread_t thread = pthread_self();
 
-        status = pthread_attr_init(&attr);
-        _ASSERT_MSG(status == 0, "pthread_attr_init call failed");
+    status = pthread_attr_init(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_init call failed");
 
 #if HAVE_PTHREAD_ATTR_GET_NP
-        status = pthread_attr_get_np(thread, &attr);
+    status = pthread_attr_get_np(thread, &attr);
 #elif HAVE_PTHREAD_GETATTR_NP
-        status = pthread_getattr_np(thread, &attr);
+    status = pthread_getattr_np(thread, &attr);
 #else
 #error Dont know how to get thread attributes on this platform!
 #endif
-        _ASSERT_MSG(status == 0, "pthread_getattr_np call failed");
+    _ASSERT_MSG(status == 0, "pthread_getattr_np call failed");
 
-        status = pthread_attr_getstack(&attr, &m_stackLimit, &stackSize);
-        _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
+    status = pthread_attr_getstack(&attr, &stackLimit, &stackSize);
+    _ASSERT_MSG(status == 0, "pthread_attr_getstack call failed");
 
-        status = pthread_attr_destroy(&attr);
-        _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
+    status = pthread_attr_destroy(&attr);
+    _ASSERT_MSG(status == 0, "pthread_attr_destroy call failed");
 #endif
+
+    return stackLimit;
+}
+
+// Get cached base address of this thread's stack
+// Can be called only for the current thread.
+void *
+CPalThread::GetCachedStackBase()
+{
+    _ASSERT_MSG(this == InternalGetCurrentThread(), "CPalThread::GetStackBase called from foreign thread");
+
+    if (m_stackBase == NULL)
+    {
+        m_stackBase = GetStackBase();
+    }
+
+    return m_stackBase;
+}
+
+// Get cached limit address of this thread's stack.
+// Can be called only for the current thread.
+void *
+CPalThread::GetCachedStackLimit()
+{
+    _ASSERT_MSG(this == InternalGetCurrentThread(), "CPalThread::GetCachedStackLimit called from foreign thread");
+
+    if (m_stackLimit == NULL)
+    {
+        m_stackLimit = GetStackLimit();
     }
 
     return m_stackLimit;
@@ -2598,7 +2618,7 @@ PALAPI
 PAL_GetStackBase()
 {
     CPalThread* thread = InternalGetCurrentThread();
-    return thread->GetStackBase();
+    return thread->GetCachedStackBase();
 }
 
 void *
@@ -2606,7 +2626,7 @@ PALAPI
 PAL_GetStackLimit()
 {
     CPalThread* thread = InternalGetCurrentThread();
-    return thread->GetStackLimit();
+    return thread->GetCachedStackLimit();
 }
 
 PAL_ERROR InjectActivationInternal(CorUnix::CPalThread* pThread);