From 6aab418ece3649119d9b654bf3732af0a89e03ad Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 22 Oct 2016 03:33:36 +0300 Subject: [PATCH 1/2] Fix backtraces on Windows/GNU --- src/libbacktrace/fileline.c | 28 ++++++++++++++++++++++++ src/test/run-pass/backtrace-debuginfo.rs | 1 - src/test/run-pass/backtrace.rs | 4 ---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/src/libbacktrace/fileline.c b/src/libbacktrace/fileline.c index 27ebbedc21ccf..47f5db996a4c8 100644 --- a/src/libbacktrace/fileline.c +++ b/src/libbacktrace/fileline.c @@ -38,6 +38,11 @@ POSSIBILITY OF SUCH DAMAGE. */ #include #include +#define USE_WIN_EXECNAME (defined(_WIN32_WINNT) && (_WIN32_WINNT >= 0x0600)) +#if USE_WIN_EXECNAME +#include +#endif /* USE_WIN_EXECNAME */ + #include "backtrace.h" #include "internal.h" @@ -45,6 +50,29 @@ POSSIBILITY OF SUCH DAMAGE. */ #define getexecname() NULL #endif +/* Targeted fix for Rust backtraces on Windows/GNU. + We can't set the executable name once during during creation + of backtrace state for security reasons, see issue #21889 for + details. So we recalculate it each time the file is accessed. + We also use QueryFullProcessImageName instead of APIs like + GetModuleFileName because it's correctly updated when the + executable is renamed. */ +#if USE_WIN_EXECNAME +#undef getexecname +static const char *getexecname(void) { + /* Accesses to backtrace functionality from Rust are serialized, + so having a single static unsyncronized buffer is enough. */ + static char buf[MAX_PATH]; + /* The returned name is later passed to `open`, so it needs to + be encoded in the current locale, so we use `QueryFullProcessImageNameA`. + As a result paths not representable in the current locale are not supported. */ + DWORD buf_size = MAX_PATH; + HANDLE process_handle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, GetCurrentProcessId()); + return QueryFullProcessImageNameA(process_handle, 0, buf, &buf_size) ? buf : NULL; +} +} +#endif /* USE_WIN_EXECNAME */ + /* Initialize the fileline information from the executable. Returns 1 on success, 0 on failure. */ diff --git a/src/test/run-pass/backtrace-debuginfo.rs b/src/test/run-pass/backtrace-debuginfo.rs index 72cf109fd5974..41b387eb0b9d2 100644 --- a/src/test/run-pass/backtrace-debuginfo.rs +++ b/src/test/run-pass/backtrace-debuginfo.rs @@ -37,7 +37,6 @@ macro_rules! dump_and_die { target_os = "ios", target_os = "android", all(target_os = "linux", target_arch = "arm"), - target_os = "windows", target_os = "freebsd", target_os = "dragonfly", target_os = "bitrig", diff --git a/src/test/run-pass/backtrace.rs b/src/test/run-pass/backtrace.rs index c438c17f51e3a..3759070a1a4c1 100644 --- a/src/test/run-pass/backtrace.rs +++ b/src/test/run-pass/backtrace.rs @@ -103,10 +103,6 @@ fn runtest(me: &str) { } fn main() { - if cfg!(windows) && cfg!(target_env = "gnu") { - return - } - let args: Vec = env::args().collect(); if args.len() >= 2 && args[1] == "fail" { foo(); From c46c256c9c6a2e3b76018e16f57556209feeed27 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 22 Oct 2016 03:33:36 +0300 Subject: [PATCH 2/2] libbacktrace: Validate opened executable file on Windows --- src/libbacktrace/fileline.c | 44 ++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/src/libbacktrace/fileline.c b/src/libbacktrace/fileline.c index 47f5db996a4c8..c4931ed4cce8c 100644 --- a/src/libbacktrace/fileline.c +++ b/src/libbacktrace/fileline.c @@ -56,8 +56,10 @@ POSSIBILITY OF SUCH DAMAGE. */ details. So we recalculate it each time the file is accessed. We also use QueryFullProcessImageName instead of APIs like GetModuleFileName because it's correctly updated when the - executable is renamed. */ + executable is renamed. We alse validate that the opened file + is indeed our executable by using NtAreMappedFilesTheSame. */ #if USE_WIN_EXECNAME + #undef getexecname static const char *getexecname(void) { /* Accesses to backtrace functionality from Rust are serialized, @@ -70,7 +72,42 @@ static const char *getexecname(void) { HANDLE process_handle = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, GetCurrentProcessId()); return QueryFullProcessImageNameA(process_handle, 0, buf, &buf_size) ? buf : NULL; } + +static int validate_descriptor(int descriptor) { + FARPROC NtAreMappedFilesTheSame = GetProcAddress(GetModuleHandleW(L"ntdll.dll"), + "NtAreMappedFilesTheSame"); + if (!NtAreMappedFilesTheSame) { + return 0; + } + HANDLE file_handle = (HANDLE) _get_osfhandle(descriptor); + if (file_handle == INVALID_HANDLE_VALUE) { + return 0; + } + /* Map the opened file into memory */ + HANDLE file_mapping = CreateFileMappingW(file_handle, NULL, PAGE_READONLY | SEC_IMAGE, + 0, 0, NULL); + if (!file_mapping) { + return 0; + } + LPVOID mapped_view = MapViewOfFile(file_mapping, FILE_MAP_READ, 0, 0, 0); + if (!mapped_view) { + CloseHandle(file_mapping); + return 0; + } + /* Now "compare" memory at which the opened file is mapped (mapped_view) with memory + at which the current executable is mapped (returned by GetModuleHandleW) */ + NTSTATUS status = NtAreMappedFilesTheSame(GetModuleHandleW(NULL), mapped_view); + UnmapViewOfFile(mapped_view); + CloseHandle(file_mapping); + return status == 0; } + +#else /* USE_WIN_EXECNAME */ + +static int validate_descriptor(int descriptor) { + return 1; +} + #endif /* USE_WIN_EXECNAME */ /* Initialize the fileline information from the executable. Returns 1 @@ -141,6 +178,11 @@ fileline_initialize (struct backtrace_state *state, called_error_callback = 1; break; } + if (!validate_descriptor(descriptor)) { + close(descriptor); + descriptor = -1; + break; + } if (descriptor >= 0) break; }