From 82ad26307c7a208fddfe85dac8f90c69bb9e32df Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 9 Dec 2022 15:39:22 -0800 Subject: [PATCH 1/2] Dispatcher: Calculate REG_ERR correctly using ARM ESR_EL1 On Fault then ARM will return information about the fault in ESR_EL1 to the user. We need to decode what ESR_EL1 means in the context of the fault to get the flags we care about. The flags we care about specifically are PF_USER and PF_WRITE. PF_PROT would have been interesting but I didn't see when this gets returned to the user. ARM makes the difference if the page is unmapped or "mapped" with PROT_NONE. x86 doesn't make the distinction here. This should fix an issue that a user was hitting. --- .../Interface/Core/ArchHelpers/MContext.h | 67 ++++++++++++++++++- .../Interface/Core/Dispatcher/Dispatcher.cpp | 9 ++- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/External/FEXCore/Source/Interface/Core/ArchHelpers/MContext.h b/External/FEXCore/Source/Interface/Core/ArchHelpers/MContext.h index f8a1d3686..eb656bce0 100644 --- a/External/FEXCore/Source/Interface/Core/ArchHelpers/MContext.h +++ b/External/FEXCore/Source/Interface/Core/ArchHelpers/MContext.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -10,7 +11,6 @@ #include #include - namespace FEXCore::ArchHelpers::Context { enum ContextFlags : uint32_t { @@ -76,6 +76,7 @@ static inline mcontext_t* GetMContext(void* ucontext) { #ifdef _M_ARM_64 constexpr uint32_t FPR_MAGIC = 0x46508001U; +constexpr uint32_t ESR1_MAGIC = 0x45535201U; struct HostCTXHeader { uint32_t Magic; @@ -89,6 +90,11 @@ struct HostFPRState { __uint128_t FPRs[32]; }; +struct HostESRState { + HostCTXHeader Head; + uint64_t ESR; +}; + static inline uint64_t GetSp(void* ucontext) { return GetMContext(ucontext)->sp; } @@ -129,6 +135,61 @@ static inline __uint128_t GetArmFPR(void* ucontext, uint32_t id) { return HostState->FPRs[id]; } +static inline uint64_t GetArmESR(void* ucontext) { + auto MContext = GetMContext(ucontext); + + size_t i = 0; + auto HostState = reinterpret_cast(&MContext->__reserved[i]); + do { + if (HostState->Magic == ESR1_MAGIC) { + auto ESR = reinterpret_cast(HostState); + return ESR->ESR; + } + i += HostState->Size; + HostState = reinterpret_cast(&MContext->__reserved[i]); + } while (HostState->Size != 0); + + return 0; +} + +constexpr static uint64_t ESR1_EC = 0b111111U << 26; +constexpr static uint64_t ESR1_EC_DataAbort = 0b100100U << 26; + +// Write-Not-Read flag +// When set - Abort is due to a write +constexpr static uint64_t ESR1_WNR = 1 << 6; + +// DFSC - Default Status Code +// Translation fault - No page mapped +// Permissions fault - Page mapped but with incorrect permission from access. +constexpr static uint64_t ESR1_DataAbort_DFSC = 0b111111; +constexpr static uint64_t ESR1_DataAbort_TranslationFault_EL0 = 0b000111; +constexpr static uint64_t ESR1_DataAbort_PermissionFault_EL0 = 0b001111; +constexpr static uint64_t ESR1_DataAbort_Level = 0b11; +constexpr static uint64_t ESR1_DataAbort_Level_EL3 = 0b00; +constexpr static uint64_t ESR1_DataAbort_Level_EL2 = 0b01; +constexpr static uint64_t ESR1_DataAbort_Level_EL1 = 0b10; +constexpr static uint64_t ESR1_DataAbort_Level_EL0 = 0b11; + +static inline uint32_t GetProtectFlags(void* ucontext) { + uint64_t ESR = GetArmESR(ucontext); + LOGMAN_THROW_A_FMT((ESR & ESR1_EC) == ESR1_EC_DataAbort, "Unknown ESR1 EC type: 0x{:x} != 0x{:x}", ESR & ESR1_EC, ESR1_EC_DataAbort); + + uint32_t ProtectFlags{}; + if ((ESR & ESR1_DataAbort_Level) == ESR1_DataAbort_Level_EL0) { + // Always a user error for us. + ProtectFlags |= X86State::X86_PF_USER; + } + + if (ESR & ESR1_WNR) { + // Fault was due to a write + ProtectFlags |= X86State::X86_PF_WRITE; + } + + // PF_PROT is not returned to user on x86, so don't return the difference between permission fault and translation fault. + return ProtectFlags; +} + using ContextBackup = ArmContextBackup; template static inline void BackupContext(void* ucontext, T *Backup) { @@ -222,6 +283,10 @@ static inline __uint128_t GetArmFPR(void* ucontext, uint32_t id) { ERROR_AND_DIE_FMT("Not implemented for x86 host"); } +static inline uint32_t GetProtectFlags(void* ucontext) { + return GetMContext(ucontext)->gregs[REG_ERR]; +} + using ContextBackup = X86ContextBackup; template static inline void BackupContext(void* ucontext, T *Backup) { diff --git a/External/FEXCore/Source/Interface/Core/Dispatcher/Dispatcher.cpp b/External/FEXCore/Source/Interface/Core/Dispatcher/Dispatcher.cpp index 7f6d02e4d..74cdfeeeb 100644 --- a/External/FEXCore/Source/Interface/Core/Dispatcher/Dispatcher.cpp +++ b/External/FEXCore/Source/Interface/Core/Dispatcher/Dispatcher.cpp @@ -288,15 +288,14 @@ static uint32_t ConvertSignalToTrapNo(int Signal, siginfo_t *HostSigInfo) { return Signal; } -static uint32_t ConvertSignalToError(int Signal, siginfo_t *HostSigInfo) { +static uint32_t ConvertSignalToError(void *ucontext, int Signal, siginfo_t *HostSigInfo) { switch (Signal) { case SIGSEGV: if (HostSigInfo->si_code == SEGV_MAPERR || HostSigInfo->si_code == SEGV_ACCERR) { // Protection fault // Always a user fault for us - // XXX: PF_PROT and PF_WRITE - return X86State::X86_PF_USER; + return ArchHelpers::Context::GetProtectFlags(ucontext); } break; } @@ -477,7 +476,7 @@ bool Dispatcher::HandleGuestSignal(FEXCore::Core::InternalThreadState *Thread, i } else { guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_TRAPNO] = ConvertSignalToTrapNo(Signal, HostSigInfo); - guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_ERR] = ConvertSignalToError(Signal, HostSigInfo); + guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_ERR] = ConvertSignalToError(ucontext, Signal, HostSigInfo); } guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_OLDMASK] = 0; guest_uctx->uc_mcontext.gregs[FEXCore::x86_64::FEX_REG_CR2] = 0; @@ -589,7 +588,7 @@ bool Dispatcher::HandleGuestSignal(FEXCore::Core::InternalThreadState *Thread, i else { guest_uctx->uc_mcontext.gregs[FEXCore::x86::FEX_REG_TRAPNO] = ConvertSignalToTrapNo(Signal, HostSigInfo); guest_siginfo->si_code = HostSigInfo->si_code; - guest_uctx->uc_mcontext.gregs[FEXCore::x86::FEX_REG_ERR] = ConvertSignalToError(Signal, HostSigInfo); + guest_uctx->uc_mcontext.gregs[FEXCore::x86::FEX_REG_ERR] = ConvertSignalToError(ucontext, Signal, HostSigInfo); } guest_uctx->uc_mcontext.gregs[FEXCore::x86::FEX_REG_EIP] = Frame->State.rip; guest_uctx->uc_mcontext.gregs[FEXCore::x86::FEX_REG_EFL] = 0; From 3afd5691a4284433156169bf684f85beb8ecfd55 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Fri, 9 Dec 2022 15:44:22 -0800 Subject: [PATCH 2/2] unittests: Adds unit test to test ERR --- .../tests/signal/synchronous-signal-block.cpp | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/unittests/FEXLinuxTests/tests/signal/synchronous-signal-block.cpp b/unittests/FEXLinuxTests/tests/signal/synchronous-signal-block.cpp index 84444d1c4..9da4050d9 100644 --- a/unittests/FEXLinuxTests/tests/signal/synchronous-signal-block.cpp +++ b/unittests/FEXLinuxTests/tests/signal/synchronous-signal-block.cpp @@ -9,7 +9,7 @@ #include #include - +#include #include #include #include @@ -28,7 +28,13 @@ struct HandledSignal { uintptr_t addr; }; +struct HandledSignal_ERR { + int signal; + size_t ERR; +}; + static std::optional handled_signal; +static std::optional handled_signal_err; static void handler(int sig, siginfo_t *si, void *context) { printf("Got %d at address: 0x%lx\n", sig, (long)si->si_addr); @@ -97,6 +103,67 @@ std::optional CheckIfSignalHandlerCalled(F&& f) { return handled_signal; } +static void handler_read(int sig, siginfo_t *si, void *context) { + ucontext_t* _context = (ucontext_t*)context; + auto mcontext = &_context->uc_mcontext; + printf("Got %d at address: 0x%lx with 0x%zx\n", sig, (long)si->si_addr, (size_t)mcontext->gregs[REG_ERR]); + handled_signal_err = { sig, (size_t)mcontext->gregs[REG_ERR] }; + siglongjmp(jmpbuf, 1); +} + +template +std::optional CheckIfSignalHandlerCalledWithRegERR(F&& f) { + handled_signal = {}; + struct sigaction oldsa[4]; + + if (!sigsetjmp(jmpbuf, 1)) { + // Handle all signals by the test handler + struct sigaction sa; + sa.sa_flags = SA_SIGINFO; + sigemptyset(&sa.sa_mask); + sa.sa_sigaction = handler_read; + sigaction(SIGSEGV, &sa, &oldsa[0]); + sigaction(SIGBUS, &sa, &oldsa[1]); + sigaction(SIGILL, &sa, &oldsa[2]); + sigaction(SIGFPE, &sa, &oldsa[3]); + + // Mask signals and run given callback + std::forward(f)(); + } + + // Restore previous signal handlers + sigaction(SIGSEGV, &oldsa[0], nullptr); + sigaction(SIGBUS, &oldsa[1], nullptr); + sigaction(SIGILL, &oldsa[2], nullptr); + sigaction(SIGFPE, &oldsa[3], nullptr); + + return handled_signal; +} + +TEST_CASE("Signals: Error Flag - Read") { + // Check that the signal handler is delayed until unmasking. + auto handled_signal = CheckIfSignalHandlerCalledWithRegERR([&]() { + uint8_t *Code = (uint8_t*)mmap(nullptr, 4096, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + printf("Read: %d\n", Code[0]); + }); + REQUIRE(handled_signal_err.has_value()); + CHECK(handled_signal_err->signal == SIGSEGV); + constexpr size_t Expected = 0x4; + CHECK(handled_signal_err->ERR == Expected); // USER +} + +TEST_CASE("Signals: Error Flag - Write") { + // Check that the signal handler is delayed until unmasking. + auto handled_signal = CheckIfSignalHandlerCalledWithRegERR([&]() { + uint8_t *Code = (uint8_t*)mmap(nullptr, 4096, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + Code[0] = 1; + }); + REQUIRE(handled_signal_err.has_value()); + CHECK(handled_signal_err->signal == SIGSEGV); + constexpr size_t Expected = 0x6; + CHECK(handled_signal_err->ERR == Expected); // USER + WRITE +} + // For ssegv, we fail to do default signal catching behaviour TEST_CASE("Signals: ssegv") { auto status = CheckIfExitsFromSignal([]() { *(int*)0x32 = 0x64; });