diff options
author | Jan Vorlicek <janvorli@microsoft.com> | 2016-04-06 20:41:51 +0200 |
---|---|---|
committer | Jan Vorlicek <janvorli@microsoft.com> | 2016-04-14 14:10:32 +0200 |
commit | c240f969e0f43ee25f9590f7e53676e4fd7bd85d (patch) | |
tree | 430186702a14181d67ef04f8d5a3a63d9a3bb51b /src/debug/debug-pal | |
parent | bd7ee6206f05fbc321e7f69383423f05d440e7b6 (diff) | |
download | coreclr-c240f969e0f43ee25f9590f7e53676e4fd7bd85d.tar.gz coreclr-c240f969e0f43ee25f9590f7e53676e4fd7bd85d.tar.bz2 coreclr-c240f969e0f43ee25f9590f7e53676e4fd7bd85d.zip |
Fix possibility of deadlock in the SIGSEGV handler
The SIGSEGV handler was transitively calling pthread_attr_get_np to
get current stack limit no matter where the exception happened. The
problem was that if the segmentation violation happened in a C
runtime function, this could lead to deadlock since pthread_attr_get_np
is not signal safe.
The fix is to add callback that will figure out if the failing instruction
was in managed code or in one of the jit helpers and if not, it would
not attempt to handle the signal.
I have also removed non signal safe calls from the signal handlers code
path where we don't know yet whether the interrupted code was in a function
that could collide with a function that we call from the signal handler
or not. This includes TRACE and ASSERT calls that were calling string
formatting functions.
Diffstat (limited to 'src/debug/debug-pal')
-rw-r--r-- | src/debug/debug-pal/unix/twowaypipe.cpp | 50 |
1 files changed, 20 insertions, 30 deletions
diff --git a/src/debug/debug-pal/unix/twowaypipe.cpp b/src/debug/debug-pal/unix/twowaypipe.cpp index 3eeb15bb78..2bf919feeb 100644 --- a/src/debug/debug-pal/unix/twowaypipe.cpp +++ b/src/debug/debug-pal/unix/twowaypipe.cpp @@ -15,7 +15,7 @@ static const char* PipeNameFormat = "/tmp/clr-debug-pipe-%d-%llu-%s"; -static void GetPipeName(char *name, DWORD id, const char *suffix) +void TwoWayPipe::GetPipeName(char *name, DWORD id, const char *suffix) { UINT64 disambiguationKey; BOOL ret = GetProcessIdDisambiguationKey(id, &disambiguationKey); @@ -25,8 +25,8 @@ static void GetPipeName(char *name, DWORD id, const char *suffix) // also try to use 0 as the value. _ASSERTE(ret == TRUE || disambiguationKey == 0); - int chars = _snprintf(name, PATH_MAX, PipeNameFormat, id, disambiguationKey, suffix); - _ASSERTE(chars > 0 && chars < PATH_MAX); + int chars = _snprintf(name, MaxPipeNameLength, PipeNameFormat, id, disambiguationKey, suffix); + _ASSERTE(chars > 0 && chars < MaxPipeNameLength); } // Creates a server side of the pipe. @@ -39,19 +39,17 @@ bool TwoWayPipe::CreateServer(DWORD id) return false; m_id = id; - char inPipeName[PATH_MAX]; - char outPipeName[PATH_MAX]; - GetPipeName(inPipeName, id, "in"); - GetPipeName(outPipeName, id, "out"); + GetPipeName(m_inPipeName, id, "in"); + GetPipeName(m_outPipeName, id, "out"); - if (mkfifo(inPipeName, S_IRWXU) == -1) + if (mkfifo(m_inPipeName, S_IRWXU) == -1) { return false; } - if (mkfifo(outPipeName, S_IRWXU) == -1) + if (mkfifo(m_outPipeName, S_IRWXU) == -1) { - unlink(inPipeName); + unlink(m_inPipeName); return false; } @@ -69,21 +67,19 @@ bool TwoWayPipe::Connect(DWORD id) return false; m_id = id; - char inPipeName[PATH_MAX]; - char outPipeName[PATH_MAX]; //"in" and "out" are switched deliberately, because we're on the client - GetPipeName(inPipeName, id, "out"); - GetPipeName(outPipeName, id, "in"); + GetPipeName(m_inPipeName, id, "out"); + GetPipeName(m_outPipeName, id, "in"); // Pipe opening order is reversed compared to WaitForConnection() // in order to avaid deadlock. - m_outboundPipe = open(outPipeName, O_WRONLY); + m_outboundPipe = open(m_outPipeName, O_WRONLY); if (m_outboundPipe == INVALID_PIPE) { return false; } - m_inboundPipe = open(inPipeName, O_RDONLY); + m_inboundPipe = open(m_inPipeName, O_RDONLY); if (m_inboundPipe == INVALID_PIPE) { close(m_outboundPipe); @@ -104,18 +100,13 @@ bool TwoWayPipe::WaitForConnection() if (m_state != Created) return false; - char inPipeName[PATH_MAX]; - char outPipeName[PATH_MAX]; - GetPipeName(inPipeName, m_id, "in"); - GetPipeName(outPipeName, m_id, "out"); - - m_inboundPipe = open(inPipeName, O_RDONLY); + m_inboundPipe = open(m_inPipeName, O_RDONLY); if (m_inboundPipe == INVALID_PIPE) { return false; } - m_outboundPipe = open(outPipeName, O_WRONLY); + m_outboundPipe = open(m_outPipeName, O_WRONLY); if (m_outboundPipe == INVALID_PIPE) { close(m_inboundPipe); @@ -185,6 +176,10 @@ int TwoWayPipe::Write(const void *data, DWORD dataSize) // true - success, false - failure (use GetLastError() for more details) bool TwoWayPipe::Disconnect() { + // IMPORTANT NOTE: This function must not call any signal unsafe functions + // since it is called from signal handlers. + // That includes ASSERT and TRACE macros. + if (m_outboundPipe != INVALID_PIPE && m_outboundPipe != 0) { close(m_outboundPipe); @@ -199,13 +194,8 @@ bool TwoWayPipe::Disconnect() if (m_state == ServerConnected || m_state == Created) { - char inPipeName[PATH_MAX]; - GetPipeName(inPipeName, m_id, "in"); - unlink(inPipeName); - - char outPipeName[PATH_MAX]; - GetPipeName(outPipeName, m_id, "out"); - unlink(outPipeName); + unlink(m_inPipeName); + unlink(m_outPipeName); } m_state = NotInitialized; |