From 0380bcb3a4125723dc5248f70174ff64fb1942a2 Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Fri, 14 Sep 2018 11:09:41 -0700 Subject: Fix interaction between rt_sigtimedwait and ignored signals. PiperOrigin-RevId: 213011782 Change-Id: I716c6ea3c586b0c6c5a892b6390d2d11478bc5af --- pkg/sentry/kernel/task.go | 9 +++-- pkg/sentry/kernel/task_signals.go | 68 ++++++++++++++++++++++++--------- pkg/sentry/syscalls/linux/sys_signal.go | 55 +++----------------------- 3 files changed, 61 insertions(+), 71 deletions(-) diff --git a/pkg/sentry/kernel/task.go b/pkg/sentry/kernel/task.go index ae4fd7817..2f6f825ac 100644 --- a/pkg/sentry/kernel/task.go +++ b/pkg/sentry/kernel/task.go @@ -108,9 +108,12 @@ type Task struct { // goroutine. signalMask linux.SignalSet - // FIXME: An equivalent to task_struct::real_blocked is needed - // to prevent signals that are ignored, but transiently unblocked by - // sigtimedwait(2), from being dropped in Task.sendSignalTimerLocked. + // If the task goroutine is currently executing Task.sigtimedwait, + // realSignalMask is the previous value of signalMask, which has temporarily + // been replaced by Task.sigtimedwait. Otherwise, realSignalMask is 0. + // + // realSignalMask is exclusive to the task goroutine. + realSignalMask linux.SignalSet // If haveSavedSignalMask is true, savedSignalMask is the signal mask that // should be applied after the task has either delivered one signal to a diff --git a/pkg/sentry/kernel/task_signals.go b/pkg/sentry/kernel/task_signals.go index 58a1bc0bd..afb010f60 100644 --- a/pkg/sentry/kernel/task_signals.go +++ b/pkg/sentry/kernel/task_signals.go @@ -19,6 +19,7 @@ package kernel import ( "fmt" "sync/atomic" + "time" "gvisor.googlesource.com/gvisor/pkg/abi/linux" "gvisor.googlesource.com/gvisor/pkg/sentry/arch" @@ -119,25 +120,11 @@ var UnblockableSignals = linux.MakeSignalSet(linux.SIGKILL, linux.SIGSTOP) // StopSignals is the set of signals whose default action is SignalActionStop. var StopSignals = linux.MakeSignalSet(linux.SIGSTOP, linux.SIGTSTP, linux.SIGTTIN, linux.SIGTTOU) -// dequeueSignalLocked returns a pending unmasked signal. If there are no -// pending unmasked signals, dequeueSignalLocked returns nil. +// dequeueSignalLocked returns a pending signal that is *not* included in mask. +// If there are no pending unmasked signals, dequeueSignalLocked returns nil. // // Preconditions: t.tg.signalHandlers.mu must be locked. -func (t *Task) dequeueSignalLocked() *arch.SignalInfo { - if info := t.pendingSignals.dequeue(t.signalMask); info != nil { - return info - } - return t.tg.pendingSignals.dequeue(t.signalMask) -} - -// TakeSignal returns a pending signal not blocked by mask. Signal handlers are -// not affected. If there are no pending signals not blocked by mask, -// TakeSignal returns a nil SignalInfo. -func (t *Task) TakeSignal(mask linux.SignalSet) *arch.SignalInfo { - t.tg.pidns.owner.mu.RLock() - defer t.tg.pidns.owner.mu.RUnlock() - t.tg.signalHandlers.mu.Lock() - defer t.tg.signalHandlers.mu.Unlock() +func (t *Task) dequeueSignalLocked(mask linux.SignalSet) *arch.SignalInfo { if info := t.pendingSignals.dequeue(mask); info != nil { return info } @@ -294,6 +281,49 @@ func (t *Task) SignalReturn(rt bool) (*SyscallControl, error) { return ctrlResume, nil } +// Sigtimedwait implements the semantics of sigtimedwait(2). +// +// Preconditions: The caller must be running on the task goroutine. t.exitState +// < TaskExitZombie. +func (t *Task) Sigtimedwait(set linux.SignalSet, timeout time.Duration) (*arch.SignalInfo, error) { + // set is the set of signals we're interested in; invert it to get the set + // of signals to block. + mask := ^set &^ UnblockableSignals + + t.tg.signalHandlers.mu.Lock() + defer t.tg.signalHandlers.mu.Unlock() + if info := t.dequeueSignalLocked(mask); info != nil { + return info, nil + } + + if timeout == 0 { + return nil, syserror.EAGAIN + } + + // Unblock signals we're waiting for. Remember the original signal mask so + // that Task.sendSignalTimerLocked doesn't discard ignored signals that + // we're temporarily unblocking. + t.realSignalMask = t.signalMask + t.setSignalMaskLocked(t.signalMask & mask) + + // Wait for a timeout or new signal. + t.tg.signalHandlers.mu.Unlock() + _, err := t.BlockWithTimeout(nil, true, timeout) + t.tg.signalHandlers.mu.Lock() + + // Restore the original signal mask. + t.setSignalMaskLocked(t.realSignalMask) + t.realSignalMask = 0 + + if info := t.dequeueSignalLocked(mask); info != nil { + return info, nil + } + if err == syserror.ETIMEDOUT { + return nil, syserror.EAGAIN + } + return nil, err +} + // SendSignal sends the given signal to t. // // The following errors may be returned: @@ -431,7 +461,7 @@ func (t *Task) sendSignalTimerLocked(info *arch.SignalInfo, group bool, timer *I // Linux's kernel/signal.c:__send_signal() => prepare_signal() => // sig_ignored(). ignored := computeAction(sig, t.tg.signalHandlers.actions[sig]) == SignalActionIgnore - if linux.SignalSetOf(sig)&t.signalMask == 0 && ignored && !t.hasTracer() { + if sigset := linux.SignalSetOf(sig); sigset&t.signalMask == 0 && sigset&t.realSignalMask == 0 && ignored && !t.hasTracer() { t.Debugf("Discarding ignored signal %d", sig) if timer != nil { timer.signalRejectedLocked() @@ -1010,7 +1040,7 @@ func (*runInterrupt) execute(t *Task) taskRunState { } // Are there signals pending? - if info := t.dequeueSignalLocked(); info != nil { + if info := t.dequeueSignalLocked(t.signalMask); info != nil { if linux.SignalSetOf(linux.Signal(info.Signo))&StopSignals != 0 && t.tg.groupStopPhase == groupStopNone { // Indicate that we've dequeued a stop signal before // unlocking the signal mutex; initiateGroupStop will check diff --git a/pkg/sentry/syscalls/linux/sys_signal.go b/pkg/sentry/syscalls/linux/sys_signal.go index 66ecb1299..ecdec5d3a 100644 --- a/pkg/sentry/syscalls/linux/sys_signal.go +++ b/pkg/sentry/syscalls/linux/sys_signal.go @@ -343,44 +343,6 @@ func Pause(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall return 0, nil, syserror.ConvertIntr(t.Block(nil), kernel.ERESTARTNOHAND) } -func sigtimedwait(t *kernel.Task, mask linux.SignalSet, timeout time.Duration) (*arch.SignalInfo, error) { - // Is it already pending? - if info := t.TakeSignal(^mask); info != nil { - return info, nil - } - - // No signals available immediately and asked not to wait. - if timeout == 0 { - return nil, syserror.EAGAIN - } - - // No signals available yet. Temporarily unblock the ones we are interested - // in then wait for either a timeout or a new signal. - oldmask := t.SignalMask() - t.SetSignalMask(oldmask &^ mask) - _, err := t.BlockWithTimeout(nil, true, timeout) - t.SetSignalMask(oldmask) - - // How did the wait go? - switch err { - case syserror.ErrInterrupted: - if info := t.TakeSignal(^mask); info != nil { - // Got one of the signals we were waiting for. - return info, nil - } - // Got a signal we weren't waiting for. - return nil, syserror.EINTR - case syserror.ETIMEDOUT: - // Timed out and still no signals. - return nil, syserror.EAGAIN - default: - // Some other error? Shouldn't be possible. The event channel - // passed to BlockWithTimeout was nil, so the only two ways the - // block could've ended are a timeout or an interrupt. - panic("unreachable") - } -} - // RtSigpending implements linux syscall rt_sigpending(2). func RtSigpending(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { addr := args[0].Pointer() @@ -415,23 +377,18 @@ func RtSigtimedwait(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kerne timeout = time.Duration(math.MaxInt64) } - si, err := sigtimedwait(t, mask, timeout) + si, err := t.Sigtimedwait(mask, timeout) if err != nil { return 0, nil, err } - if si != nil { - if siginfo != 0 { - si.FixSignalCodeForUser() - if _, err := t.CopyOut(siginfo, si); err != nil { - return 0, nil, err - } + if siginfo != 0 { + si.FixSignalCodeForUser() + if _, err := t.CopyOut(siginfo, si); err != nil { + return 0, nil, err } - return uintptr(si.Signo), nil, nil } - - // sigtimedwait's not supposed to return nil si and err... - return 0, nil, nil + return uintptr(si.Signo), nil, nil } // RtSigqueueinfo implements linux syscall rt_sigqueueinfo(2). -- cgit v1.2.3