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).