go
/
gvisor
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Avoid reuse of pending SignalInfo objects 

runApp.execute -> Task.SendSignal -> sendSignalLocked -> sendSignalTimerLocked
-> pendingSignals.enqueue assumes that it owns the arch.SignalInfo returned
from platform.Context.Switch.

On the other hand, ptrace.context.Switch assumes that it owns the returned
SignalInfo and can safely reuse it on the next call to Switch. The KVM platform
always returns a unique SignalInfo.

This becomes a problem when the returned signal is not immediately delivered,
allowing a future signal in Switch to change the previous pending SignalInfo.

This is noticeable in #38 when external SIGINTs are delivered from the PTY
slave FD. Note that the ptrace stubs are in the same process group as the
sentry, so they are eligible to receive the PTY signals. This should probably
change, but is not the only possible cause of this bug.

Updates #38

Original change by newmanwang <wcs1011@gmail.com>, updated by Michael Pratt
<mpratt@google.com>.

Change-Id: I5383840272309df70a29f67b25e8221f933622cd
PiperOrigin-RevId: 213071072
This commit is contained in:
newmanwang 2018-09-14 17:38:16 -07:00 committed by Shentubot
parent 75c66f871b
commit de5a590ee2
2 changed files with 7 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pkg/sentry/platform/platform.go
View File

@ -133,7 +133,8 @@ type Context interface {
// - ErrContextSignal: The Context was interrupted by a signal. The // - ErrContextSignal: The Context was interrupted by a signal. The
// returned *arch.SignalInfo contains information about the signal. If // returned *arch.SignalInfo contains information about the signal. If
// arch.SignalInfo.Signo == SIGSEGV, the returned usermem.AccessType // arch.SignalInfo.Signo == SIGSEGV, the returned usermem.AccessType
// contains the access type of the triggering fault. // contains the access type of the triggering fault. The caller owns
// the returned SignalInfo.
// //
// - ErrContextInterrupt: The Context was interrupted by a call to // - ErrContextInterrupt: The Context was interrupted by a call to
// Interrupt(). Switch() may return ErrContextInterrupt spuriously. In // Interrupt(). Switch() may return ErrContextInterrupt spuriously. In

7
pkg/sentry/platform/ptrace/ptrace.go
View File

@ -142,9 +142,12 @@ func (c *context) Switch(as platform.AddressSpace, ac arch.Context, cpu int32) (
if isSyscall { if isSyscall {
return nil, usermem.NoAccess, nil return nil, usermem.NoAccess, nil
} }
si := c.signalInfo
if faultSP == nil { if faultSP == nil {
// Non-fault signal. // Non-fault signal.
return &c.signalInfo, usermem.NoAccess, platform.ErrContextSignal return &si, usermem.NoAccess, platform.ErrContextSignal
} }
// Got a page fault. Ideally, we'd get real fault type here, but ptrace // Got a page fault. Ideally, we'd get real fault type here, but ptrace
@ -168,7 +171,7 @@ func (c *context) Switch(as platform.AddressSpace, ac arch.Context, cpu int32) (
// here, in case this fault was generated by a CPUID exception. There // here, in case this fault was generated by a CPUID exception. There
// is no way to distinguish between CPUID-generated faults and regular // is no way to distinguish between CPUID-generated faults and regular
// page faults. // page faults.
return &c.signalInfo, at, platform.ErrContextSignalCPUID return &si, at, platform.ErrContextSignalCPUID
} }
// Interrupt interrupts the running guest application associated with this context. // Interrupt interrupts the running guest application associated with this context.