systrap: handle stub thread crashes

If a stub thread crashes (exits), it means something wrong and the whole
subprocess has to be killed.

The sentry has to handle subprocess deaths and kills guest processes associated
with crashed suprocesses.

Reported-by: syzbot+8a8330333cd20c039a1b@syzkaller.appspotmail.com
PiperOrigin-RevId: 610296930

pkg/sentry/platform/systrap/filters.go

@@ -94,6 +94,12 @@ func (systrapSeccomp) SyscallFilters(vars precompiledseccomp.Values) seccomp.Sys
 		},
 		unix.SYS_TGKILL: seccomp.MatchAll{},
 		unix.SYS_WAIT4:  seccomp.MatchAll{},
+		unix.SYS_WAITID: seccomp.PerArg{
+			seccomp.EqualTo(unix.P_PID),
+			seccomp.AnyValue{},
+			seccomp.AnyValue{},
+			seccomp.EqualTo(unix.WEXITED | unix.WNOHANG | unix.WNOWAIT),
+		},
 		unix.SYS_SETPRIORITY: seccomp.PerArg{
 			seccomp.EqualTo(unix.PRIO_PROCESS),
 			seccomp.AnyValue{},

pkg/sentry/platform/systrap/shared_context.go

@@ -224,7 +224,9 @@ const (
 	stuckContextTimeout       = 30 * time.Second
 )
 
-func (sc *sharedContext) sleepOnState(state sysmsg.ContextState) {
+var errDeadSubprocess = fmt.Errorf("subprocess died")
+
+func (sc *sharedContext) sleepOnState(state sysmsg.ContextState) error {
 	timeout := unix.Timespec{
 		Sec:  0,
 		Nsec: contextPreemptTimeoutNsec,
@@ -239,6 +241,9 @@ func (sc *sharedContext) sleepOnState(state sysmsg.ContextState) {
 		if errno != unix.ETIMEDOUT {
 			panic(fmt.Sprintf("error waiting for state: %v", errno))
 		}
+		if !sc.subprocess.alive() {
+			return errDeadSubprocess
+		}
 		if time.Now().After(deadline) {
 			log.Warningf("Systrap task goroutine has been waiting on ThreadContext.State futex too long. ThreadContext: %v", sc)
 		}
@@ -255,6 +260,7 @@ func (sc *sharedContext) sleepOnState(state sysmsg.ContextState) {
 		timeout.Sec = contextCheckupTimeoutSec
 		timeout.Nsec = 0
 	}
+	return nil
 }
 
 type fastPathDispatcher struct {

pkg/sentry/platform/systrap/subprocess.go

@@ -23,6 +23,7 @@ import (
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/hostarch"
 	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/pool"
@@ -169,6 +170,9 @@ type subprocess struct {
 	// contextQueue is a queue of all contexts that are ready to switch back to
 	// user mode.
 	contextQueue *contextQueue
+
+	// dead indicates whether the subprocess is alive or not.
+	dead atomicbitops.Bool
 }
 
 func (s *subprocess) initSyscallThread(ptraceThread *thread) error {
@@ -452,13 +456,18 @@ func (s *subprocess) unmap() {
 // globalPool. This has the added benefit of reducing creation time for new
 // subprocesses.
 func (s *subprocess) Release() {
+	if !s.alive() {
+		return
+	}
 	s.unmap()
 	s.DecRef(s.release)
 }
 
 // release returns the subprocess to the global pool.
 func (s *subprocess) release() {
-	globalPool.markAvailable(s)
+	if s.alive() {
+		globalPool.markAvailable(s)
+	}
 }
 
 // attach attaches to the thread.
@@ -748,7 +757,10 @@ func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool
 	if err := s.contextQueue.add(ctx); err != nil {
 		return false, false, err
 	}
-	s.waitOnState(ctx)
+
+	if err := s.waitOnState(ctx); err != nil {
+		return false, false, corruptedSharedMemoryErr(err.Error())
+	}
 
 	// Check if there's been an error.
 	threadID := ctx.threadID()
@@ -786,7 +798,7 @@ func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool
 	return false, false, nil
 }
 
-func (s *subprocess) waitOnState(ctx *sharedContext) {
+func (s *subprocess) waitOnState(ctx *sharedContext) error {
 	ctx.kicked = false
 	slowPath := false
 	if !s.contextQueue.fastPathEnabled() || atomic.LoadUint32(&s.contextQueue.numActiveThreads) == 0 {
@@ -819,13 +831,17 @@ func (s *subprocess) waitOnState(ctx *sharedContext) {
 				ctx.kicked = s.kickSysmsgThread()
 			}
 
-			ctx.sleepOnState(curState)
+			if err := ctx.sleepOnState(curState); err != nil {
+				return err
+			}
 		}
 	}
 
 	ctx.recordLatency()
 	ctx.resetLatencyMeasures()
 	ctx.enableSentryFastPath()
+
+	return nil
 }
 
 // canKickSysmsgThread returns true if a new thread can be kicked.
@@ -935,7 +951,7 @@ func (s *subprocess) Unmap(addr hostarch.Addr, length uint64) {
 		unix.SYS_MUNMAP,
 		arch.SyscallArgument{Value: uintptr(addr)},
 		arch.SyscallArgument{Value: uintptr(length)})
-	if err != nil {
+	if err != nil && err != errDeadSubprocess {
 		// We never expect this to happen.
 		panic(fmt.Sprintf("munmap(%x, %x)) failed: %v", addr, length, err))
 	}

pkg/sentry/platform/systrap/subprocess_amd64.go

@@ -134,7 +134,6 @@ func (t *thread) adjustInitRegsRip() {
 
 // Pass the expected PPID to the child via R15 when creating stub process.
 func initChildProcessPPID(initregs *arch.Registers, ppid int32) {
-	initregs.R15 = uint64(ppid)
 	// Rbx has to be set to 1 when creating stub process.
 	initregs.Rbx = _NEW_STUB
 }

pkg/sentry/platform/systrap/subprocess_arm64.go

@@ -110,7 +110,6 @@ func (t *thread) adjustInitRegsRip() {
 
 // Pass the expected PPID to the child via X7 when creating stub process
 func initChildProcessPPID(initregs *arch.Registers, ppid int32) {
-	initregs.Regs[7] = uint64(ppid)
 	// R9 has to be set to 1 when creating stub process.
 	initregs.Regs[9] = _NEW_STUB
 }

pkg/sentry/platform/systrap/subprocess_linux.go

@@ -40,7 +40,7 @@ func createStub() (*thread, error) {
 	// transitively) will be killed as well. It's simply not possible to
 	// safely handle a single stub getting killed: the exact state of
 	// execution is unknown and not recoverable.
-	return attachedThread(uintptr(unix.SIGKILL)|unix.CLONE_FILES, linux.SECCOMP_RET_TRAP)
+	return attachedThread(unix.CLONE_FILES|uintptr(unix.SIGCHLD), linux.SECCOMP_RET_TRAP)
 }
 
 // attachedThread returns a new attached thread.
@@ -57,13 +57,15 @@ func attachedThread(flags uintptr, defaultAction linux.BPFAction) (*thread, erro
 			Rules: seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{
 				unix.SYS_CLONE: seccomp.Or{
 					// Allow creation of new subprocesses (used by the master).
-					seccomp.PerArg{seccomp.EqualTo(unix.CLONE_FILES | unix.SIGKILL)},
+					seccomp.PerArg{seccomp.EqualTo(unix.CLONE_FILES | unix.CLONE_PARENT | unix.SIGCHLD)},
+					seccomp.PerArg{seccomp.EqualTo(unix.CLONE_FILES | unix.SIGCHLD)},
 					// Allow creation of new sysmsg thread.
 					seccomp.PerArg{seccomp.EqualTo(
 						unix.CLONE_FILES |
 							unix.CLONE_FS |
 							unix.CLONE_VM |
-							unix.CLONE_PTRACE)},
+							unix.CLONE_PTRACE |
+							linux.SIGKILL)},
 					// Allow creation of new threads within a single address space (used by address spaces).
 					seccomp.PerArg{seccomp.EqualTo(
 						unix.CLONE_FILES |
@@ -256,7 +258,7 @@ func (t *thread) createStub() (*thread, error) {
 	pid, err := t.syscallIgnoreInterrupt(
 		&regs,
 		unix.SYS_CLONE,
-		arch.SyscallArgument{Value: uintptr(unix.SIGKILL | unix.CLONE_FILES)},
+		arch.SyscallArgument{Value: uintptr(unix.CLONE_FILES | unix.CLONE_PARENT | uintptr(unix.SIGCHLD))},
 		arch.SyscallArgument{Value: 0},
 		arch.SyscallArgument{Value: 0},
 		arch.SyscallArgument{Value: 0},
@@ -272,15 +274,7 @@ func (t *thread) createStub() (*thread, error) {
 	// We unfortunately don't have a handy part of memory to write the wait
 	// status. If the wait succeeds, we'll assume that it was the SIGSTOP.
 	// If the child actually exited, the attach below will fail.
-	_, err = t.syscallIgnoreInterrupt(
-		&t.initRegs,
-		unix.SYS_WAIT4,
-		arch.SyscallArgument{Value: uintptr(pid)},
-		arch.SyscallArgument{Value: 0},
-		arch.SyscallArgument{Value: unix.WALL | unix.WUNTRACED},
-		arch.SyscallArgument{Value: 0},
-		arch.SyscallArgument{Value: 0},
-		arch.SyscallArgument{Value: 0})
+	_, err = unix.Wait4(int(pid), nil, unix.WALL|unix.WUNTRACED, nil)
 	if err != nil {
 		return nil, fmt.Errorf("waiting on stub process: %v", err)
 	}

pkg/sentry/platform/systrap/subprocess_unsafe.go

@@ -26,6 +26,8 @@ import (
 	"unsafe"
 
 	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/sentry/arch"
 	"gvisor.dev/gvisor/pkg/sentry/memmap"
 	"gvisor.dev/gvisor/pkg/sentry/pgalloc"
@@ -97,3 +99,34 @@ func restoreFPState(ctx *sharedContext, c *context, ac *arch.Context64) {
 	dst := ctx.shared.FPState[:]
 	copy(dst, src)
 }
+
+// alive returns true if the subprocess is alive.
+func (s *subprocess) alive() bool {
+	if s.dead.Load() {
+		return false
+	}
+
+	// Wait4 doesn't support WNOWAIT, but here is no other way to find out
+	// whether a process exited or was stopped by ptrace.
+	siginfo := linux.SignalInfo{}
+	_, _, errno := unix.Syscall6(
+		unix.SYS_WAITID,
+		unix.P_PID,
+		uintptr(s.syscallThread.thread.tid),
+		uintptr(unsafe.Pointer(&siginfo)),
+		uintptr(unix.WEXITED|unix.WNOHANG|unix.WNOWAIT),
+		0, 0)
+	if errno == 0 && siginfo.PID() == 0 {
+		return true
+	}
+	if errno == 0 && siginfo.Code != linux.CLD_EXITED && siginfo.Code != linux.CLD_KILLED {
+		return true
+	}
+
+	// The process is dead, let's collect its zombie.
+	wstatus := unix.WaitStatus(0)
+	pid, err := unix.Wait4(int(s.syscallThread.thread.tid), &wstatus, unix.WNOHANG, nil)
+	log.Warningf("the subprocess %d exited (status: %s, err %s)", pid, wstatus, err)
+	s.dead.Store(true)
+	return false
+}

pkg/sentry/platform/systrap/syscall_thread.go

@@ -168,6 +168,9 @@ func (t *syscallThread) attach() error {
 }
 
 func (t *syscallThread) syscall(sysno uintptr, args ...arch.SyscallArgument) (uintptr, error) {
+	if t.subproc.dead.Load() {
+		return 0, errDeadSubprocess
+	}
 	sentryMsg := t.sentryMessage
 	stubMsg := t.stubMessage
 	sentryMsg.sysno = uint64(sysno)

pkg/sentry/platform/systrap/systrap_unsafe.go

@@ -98,7 +98,8 @@ func (t *thread) clone() (*thread, error) {
 		unix.CLONE_FILES |
 			unix.CLONE_FS |
 			unix.CLONE_PTRACE |
-			unix.CLONE_VM)
+			unix.CLONE_VM |
+			linux.SIGKILL)
 	rval, err := t.syscallIgnoreInterrupt(
 		&t.initRegs,
 		unix.SYS_CLONE,