diff --git a/pkg/sentry/platform/systrap/filters.go b/pkg/sentry/platform/systrap/filters.go index a9f61322e..7dc2cd5a1 100644 --- a/pkg/sentry/platform/systrap/filters.go +++ b/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{}, diff --git a/pkg/sentry/platform/systrap/shared_context.go b/pkg/sentry/platform/systrap/shared_context.go index 8ba1e79ee..232d7b64b 100644 --- a/pkg/sentry/platform/systrap/shared_context.go +++ b/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 { diff --git a/pkg/sentry/platform/systrap/subprocess.go b/pkg/sentry/platform/systrap/subprocess.go index 102b6361f..965eca2e2 100644 --- a/pkg/sentry/platform/systrap/subprocess.go +++ b/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)) } diff --git a/pkg/sentry/platform/systrap/subprocess_amd64.go b/pkg/sentry/platform/systrap/subprocess_amd64.go index 82ca7a649..4a19ac59a 100644 --- a/pkg/sentry/platform/systrap/subprocess_amd64.go +++ b/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 } diff --git a/pkg/sentry/platform/systrap/subprocess_arm64.go b/pkg/sentry/platform/systrap/subprocess_arm64.go index c055f8e08..f43c394d5 100644 --- a/pkg/sentry/platform/systrap/subprocess_arm64.go +++ b/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 } diff --git a/pkg/sentry/platform/systrap/subprocess_linux.go b/pkg/sentry/platform/systrap/subprocess_linux.go index 97bd16872..4e551823c 100644 --- a/pkg/sentry/platform/systrap/subprocess_linux.go +++ b/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( ®s, 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) } diff --git a/pkg/sentry/platform/systrap/subprocess_unsafe.go b/pkg/sentry/platform/systrap/subprocess_unsafe.go index a3588381f..a09d25785 100644 --- a/pkg/sentry/platform/systrap/subprocess_unsafe.go +++ b/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 +} diff --git a/pkg/sentry/platform/systrap/syscall_thread.go b/pkg/sentry/platform/systrap/syscall_thread.go index 6dd7ac1b4..dd9b80af0 100644 --- a/pkg/sentry/platform/systrap/syscall_thread.go +++ b/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) diff --git a/pkg/sentry/platform/systrap/systrap_unsafe.go b/pkg/sentry/platform/systrap/systrap_unsafe.go index 5fd679da5..eba22e920 100644 --- a/pkg/sentry/platform/systrap/systrap_unsafe.go +++ b/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,