Skip to content

Commit

Permalink
Merge pull request #1250 from Sonicadvance1/fix_signal_nodefer
Browse files Browse the repository at this point in the history 
Linux: Setup signal mask correctly to block signal-in-signal situations
  • Loading branch information
@Sonicadvance1
Sonicadvance1 authored Sep 3, 2021
2 parents ad34cdd + 4f93259 commit 70931bf
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
25 changes: 24 additions & 1 deletion External/FEXCore/Source/Interface/Core/ArchHelpers/MContext.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct X86ContextBackup {
// RIP and RSP is stored in GPRs here
uint64_t GPRs[23];
FEXCore::x86_64::_libc_fpstate FPRState;
uint64_t sa_mask;

// Guest state
int Signal;
Expand All @@ -35,6 +36,7 @@ struct ArmContextBackup {
uint32_t FPSR;
uint32_t FPCR;
__uint128_t FPRs[32];
uint64_t sa_mask;

// Guest state
int Signal;
Expand All @@ -44,6 +46,11 @@ struct ArmContextBackup {
static constexpr int RedZoneSize = 0;
};

static inline ucontext_t* GetUContext(void* ucontext) {
ucontext_t* _context = (ucontext_t*)ucontext;
return _context;
}

static inline mcontext_t* GetMContext(void* ucontext) {
ucontext_t* _context = (ucontext_t*)ucontext;
return &_context->uc_mcontext;
Expand Down Expand Up @@ -102,6 +109,7 @@ using ContextBackup = ArmContextBackup;
template <typename T>
static inline void BackupContext(void* ucontext, T *Backup) {
if constexpr (std::is_same<T, ArmContextBackup>::value) {
auto _ucontext = GetUContext(ucontext);
auto _mcontext = GetMContext(ucontext);

memcpy(&Backup->GPRs[0], &_mcontext->regs[0], 31 * sizeof(uint64_t));
Expand All @@ -115,6 +123,9 @@ static inline void BackupContext(void* ucontext, T *Backup) {
Backup->FPSR = HostState->FPSR;
Backup->FPCR = HostState->FPCR;
memcpy(&Backup->FPRs[0], &HostState->FPRs[0], 32 * sizeof(__uint128_t));

// Save the signal mask so we can restore it
memcpy(&Backup->sa_mask, &_ucontext->uc_sigmask, sizeof(uint64_t));
} else {
ERROR_AND_DIE("Wrong context type"); // This must be a runtime error
}
Expand All @@ -123,6 +134,7 @@ static inline void BackupContext(void* ucontext, T *Backup) {
template <typename T>
static inline void RestoreContext(void* ucontext, T *Backup) {
if constexpr (std::is_same<T, ArmContextBackup>::value) {
auto _ucontext = GetUContext(ucontext);
auto _mcontext = GetMContext(ucontext);

HostFPRState *HostState = reinterpret_cast<HostFPRState*>(&_mcontext->__reserved[0]);
Expand All @@ -136,6 +148,9 @@ static inline void RestoreContext(void* ucontext, T *Backup) {
ArchHelpers::Context::SetPc(ucontext, Backup->PrevPC);
ArchHelpers::Context::SetSp(ucontext, Backup->PrevSP);
memcpy(&_mcontext->regs[0], &Backup->GPRs[0], 31 * sizeof(uint64_t));

// Restore the signal mask now
memcpy(&_ucontext->uc_sigmask, &Backup->sa_mask, sizeof(uint64_t));
} else {
ERROR_AND_DIE("Wrong context type"); // This must be a runtime error
}
Expand Down Expand Up @@ -181,13 +196,17 @@ using ContextBackup = X86ContextBackup;
template <typename T>
static inline void BackupContext(void* ucontext, T *Backup) {
if constexpr (std::is_same<T, X86ContextBackup>::value) {
auto _ucontext = GetUContext(ucontext);
auto _mcontext = GetMContext(ucontext);

// Copy the GPRs
memcpy(&Backup->GPRs[0], &_mcontext->gregs[0], sizeof(X86ContextBackup::GPRs));
// Copy the FPRState
memcpy(&Backup->FPRState, _mcontext->fpregs, sizeof(X86ContextBackup::FPRState));
// XXX: Save 256bit and 512bit AVX register state

// Save the signal mask so we can restore it
memcpy(&Backup->sa_mask, &_ucontext->uc_sigmask, sizeof(uint64_t));
} else {
ERROR_AND_DIE("Wrong context type"); // This must be a runtime error
}
Expand All @@ -196,17 +215,21 @@ static inline void BackupContext(void* ucontext, T *Backup) {
template <typename T>
static inline void RestoreContext(void* ucontext, T *Backup) {
if constexpr (std::is_same<T, X86ContextBackup>::value) {
auto _ucontext = GetUContext(ucontext);
auto _mcontext = GetMContext(ucontext);

// Copy the GPRs
memcpy(&_mcontext->gregs[0], &Backup->GPRs[0], sizeof(X86ContextBackup::GPRs));
// Copy the FPRState
memcpy(_mcontext->fpregs, &Backup->FPRState, sizeof(X86ContextBackup::FPRState));

// Restore the signal mask now
memcpy(&_ucontext->uc_sigmask, &Backup->sa_mask, sizeof(uint64_t));
} else {
ERROR_AND_DIE("Wrong context type"); // This must be a runtime error
}
}

#endif

} // namespace FEXCore::ArchHelpers::Context
} // namespace FEXCore::ArchHelpers::Context
58 changes: 44 additions & 14 deletions Source/Tests/LinuxSyscalls/SignalDelegator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,6 @@ namespace FEX::HLE {
.ss_flags = SS_DISABLE, // By default the guest alt stack is disabled
.ss_size = 0,
};
// Guest signal sa_mask is per thread!
// This is the sa_mask from sigaction which is orr'd to the current signal mask
FEXCore::GuestSAMask Guest_sa_mask[SignalDelegator::MAX_SIGNALS]{};
// This is the thread's current signal mask
FEXCore::GuestSAMask CurrentSignalMask{};
// The mask prior to a suspend
Expand Down Expand Up @@ -109,6 +106,8 @@ namespace FEX::HLE {
}
}

ucontext_t* _context = (ucontext_t*)UContext;

// Remove the pending signal
ThreadData.PendingSignals &= ~(1ULL << (Signal - 1));

Expand All @@ -126,6 +125,28 @@ namespace FEX::HLE {
else {
if (Handler.GuestHandler &&
Handler.GuestHandler(Thread, Signal, Info, UContext, &Handler.GuestAction, &ThreadData.GuestAltStack)) {

// Set up a new mask based on this signals signal mask
uint64_t NewMask = Handler.GuestAction.sa_mask.Val;

// If NODEFER then the new signal mask includes this signal
if (!(Handler.GuestAction.sa_flags & SA_NODEFER)) {
NewMask |= (1ULL << (Signal - 1));
}

// Walk our required signals and stop masking them if requested
for (size_t i = 0; i < MAX_SIGNALS; ++i) {
if (HostHandlers[i + 1].Required.load(std::memory_order_relaxed)) {
// Never mask our required signals
NewMask &= ~(1ULL << i);
}
}

// Update our host signal mask so we don't hit race conditions with signals
// This allows us to maintain the expected signal mask through the guest signal handling and then all the way back again
memcpy(&_context->uc_sigmask, &NewMask, sizeof(uint64_t));

// We handled this signal, continue running
return;
}
ERROR_AND_DIE("Unhandled guest exception");
Expand Down Expand Up @@ -174,17 +195,27 @@ namespace FEX::HLE {
// Now install the thunk handler
SignalHandler.HostAction.sigaction = SignalHandlerThunk;

if ((SignalHandler.HostAction.sa_flags ^ SignalHandler.GuestAction.sa_flags) & SA_NODEFER) {
// If the guest is using SA_NODEFER then make sure to set it for the host as well
SignalHandler.HostAction.sa_flags &= ~SA_NODEFER;
SignalHandler.HostAction.sa_flags |= SignalHandler.GuestAction.sa_flags & SA_NODEFER;
}
auto CheckAndAddFlags = [](uint64_t HostFlags, uint64_t GuestFlags, uint64_t Flags) {
// If any of the flags don't match then update to the newest set
if ((HostFlags ^ GuestFlags) & Flags) {
// Remove all the flags from the host that we are testing for
HostFlags &= ~Flags;
// Copy over the guest flags being set
HostFlags |= GuestFlags & Flags;
}

if ((SignalHandler.HostAction.sa_flags ^ SignalHandler.GuestAction.sa_flags) & SA_RESTART) {
// If the guest is using SA_RESTART then make sure to set it for the host as well
SignalHandler.HostAction.sa_flags &= ~SA_RESTART;
SignalHandler.HostAction.sa_flags |= SignalHandler.GuestAction.sa_flags & SA_RESTART;
}
return HostFlags;
};

// Don't allow the guest to override flags for
// SA_SIGINFO : Host always needs SA_SIGINFO
// SA_ONSTACK : Host always needs the altstack
// SA_RESETHAND : We don't support one shot handlers
// SA_RESTORER : We always need our host side restorer on x86-64, Couldn't use guest restorer anyway
SignalHandler.HostAction.sa_flags = CheckAndAddFlags(
SignalHandler.HostAction.sa_flags,
SignalHandler.GuestAction.sa_flags,
SA_NOCLDSTOP | SA_NOCLDWAIT | SA_NODEFER | SA_RESTART);

#ifdef _M_X86_64
#define SA_RESTORER 0x04000000
Expand Down Expand Up @@ -359,7 +390,6 @@ namespace FEX::HLE {
}

HostHandlers[Signal].GuestAction = *Action;
ThreadData.Guest_sa_mask[Signal] = Action->sa_mask;
// Only attempt to install a new thunk handler if we were installing a new guest action
if (!InstallHostThunk(Signal)) {
UpdateHostThunk(Signal);
Expand Down

0 comments on commit 70931bf

Please sign in to comment.