From 6728f09910bd9f7633f277fafe6945cfaa2abf42 Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Fri, 8 Jun 2018 15:00:29 -0700 Subject: Fix sigaltstack semantics. Walking off the bottom of the sigaltstack, for example with recursive faults, results in forced signal delivery, not resetting the stack or pushing signal stack to whatever happens to lie below the signal stack. PiperOrigin-RevId: 199856085 Change-Id: I0004d2523f0df35d18714de2685b3eaa147837e0 --- pkg/sentry/arch/arch.go | 2 +- pkg/sentry/arch/signal_amd64.go | 16 ++++++++++++---- pkg/sentry/arch/signal_stack.go | 11 +++++++++-- 3 files changed, 22 insertions(+), 7 deletions(-) (limited to 'pkg/sentry/arch') diff --git a/pkg/sentry/arch/arch.go b/pkg/sentry/arch/arch.go index 021789e4b..0189e958d 100644 --- a/pkg/sentry/arch/arch.go +++ b/pkg/sentry/arch/arch.go @@ -158,7 +158,7 @@ type Context interface { // rt is true if SignalRestore is being entered from rt_sigreturn and // false if SignalRestore is being entered from sigreturn. // SignalRestore returns the thread's new signal mask. - SignalRestore(st *Stack, rt bool) (linux.SignalSet, error) + SignalRestore(st *Stack, rt bool) (linux.SignalSet, SignalStack, error) // CPUIDEmulate emulates a CPUID instruction according to current register state. CPUIDEmulate(l log.Logger) diff --git a/pkg/sentry/arch/signal_amd64.go b/pkg/sentry/arch/signal_amd64.go index 4040b530f..c1d743f38 100644 --- a/pkg/sentry/arch/signal_amd64.go +++ b/pkg/sentry/arch/signal_amd64.go @@ -377,6 +377,14 @@ func (c *context64) SignalSetup(st *Stack, act *SignalAct, info *SignalInfo, alt sp = frameBottom + usermem.Addr(frameSize) st.Bottom = sp + // Prior to proceeding, figure out if the frame will exhaust the range + // for the signal stack. This is not allowed, and should immediately + // force signal delivery (reverting to the default handler). + if act.IsOnStack() && alt.IsEnabled() && !alt.Contains(frameBottom) { + return syscall.EFAULT + } + + // Adjust the code. info.FixSignalCodeForUser() // Set up the stack frame. @@ -422,15 +430,15 @@ func (c *context64) SignalSetup(st *Stack, act *SignalAct, info *SignalInfo, alt // SignalRestore implements Context.SignalRestore. (Compare to Linux's // arch/x86/kernel/signal.c:sys_rt_sigreturn().) -func (c *context64) SignalRestore(st *Stack, rt bool) (linux.SignalSet, error) { +func (c *context64) SignalRestore(st *Stack, rt bool) (linux.SignalSet, SignalStack, error) { // Copy out the stack frame. var uc UContext64 if _, err := st.Pop(&uc); err != nil { - return 0, err + return 0, SignalStack{}, err } var info SignalInfo if _, err := st.Pop(&info); err != nil { - return 0, err + return 0, SignalStack{}, err } // Restore registers. @@ -472,5 +480,5 @@ func (c *context64) SignalRestore(st *Stack, rt bool) (linux.SignalSet, error) { log.Infof("sigreturn unable to restore application fpstate") } - return uc.Sigset, nil + return uc.Sigset, uc.Stack, nil } diff --git a/pkg/sentry/arch/signal_stack.go b/pkg/sentry/arch/signal_stack.go index 7c6531d79..ba43dd1d4 100644 --- a/pkg/sentry/arch/signal_stack.go +++ b/pkg/sentry/arch/signal_stack.go @@ -39,12 +39,19 @@ func (s SignalStack) Top() usermem.Addr { return usermem.Addr(s.Addr + s.Size) } -// SetOnStack marks this signal stack as in use. (This is only called on copies -// sent to user applications, so there's no corresponding ClearOnStack.) +// SetOnStack marks this signal stack as in use. +// +// Note that there is no corresponding ClearOnStack, and that this should only +// be called on copies that are serialized to userspace. func (s *SignalStack) SetOnStack() { s.Flags |= SignalStackFlagOnStack } +// Contains checks if the stack pointer is within this stack. +func (s *SignalStack) Contains(sp usermem.Addr) bool { + return usermem.Addr(s.Addr) < sp && sp <= usermem.Addr(s.Addr+s.Size) +} + // NativeSignalStack is a type that is equivalent to stack_t in the guest // architecture. type NativeSignalStack interface { -- cgit v1.2.3