diff --git a/pkg/sentry/platform/systrap/subprocess.go b/pkg/sentry/platform/systrap/subprocess.go index 9d2e6fcb3..cd6a3dbd7 100644 --- a/pkg/sentry/platform/systrap/subprocess.go +++ b/pkg/sentry/platform/systrap/subprocess.go @@ -107,6 +107,9 @@ const ( maxGuestContexts = 4096 // invalidContextID specifies an invalid ID. invalidContextID = maxGuestContexts + 1 + // invalidThreadID is used to indicate that a context is not being worked on by + // any sysmsg thread. + invalidThreadID uint32 = uint32(maxGuestContexts) + 1 ) // subprocess is a collection of threads being traced. @@ -154,6 +157,9 @@ type subprocess struct { syscallThreadMu sync.Mutex syscallThread *syscallThread + + sysmsgThreadsMu sync.Mutex + sysmsgThreads map[uint32]*sysmsgThread } func (s *subprocess) initSyscallThread(ptraceThread *thread) error { @@ -263,6 +269,7 @@ func newSubprocess(create func() (*thread, error), memoryFile *pgalloc.MemoryFil sysmsgStackPool: pool.Pool{Start: 0, Limit: maxSystemThreads}, threadContextPool: pool.Pool{Start: 0, Limit: maxGuestContexts}, memoryFile: memoryFile, + sysmsgThreads: make(map[uint32]*sysmsgThread), } runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -643,9 +650,10 @@ func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool ctx.Regs = regs.PtraceRegs restoreArchSpecificState(regs, t, sysThread, msg, ac) - // Check for interrupts, and ensure that future interrupts will signal t. - if !c.interrupt.Enable(sysThread) { + // Check for interrupts, and ensure that future interrupts signal the context. + if !c.interrupt.Enable(c) { // Pending interrupt; simulate. + ctx.Interrupt = 0 c.signalInfo = linux.SignalInfo{Signo: int32(platform.SignalInterrupt)} return false, false, nil } @@ -806,6 +814,7 @@ func (s *subprocess) getSysmsgThread(tregs *arch.Registers, c *context, ac *arch subproc: s, stackRange: fr, } + tid := uint32(p.tid) // Map the stack into the sentry. sentryStackAddr, _, errno := unix.RawSyscall6( @@ -848,11 +857,12 @@ func (s *subprocess) getSysmsgThread(tregs *arch.Registers, c *context, ac *arch } sysThread.setMsg(sysmsg.StackAddrToMsg(sentryStackAddr)) - sysThread.msg.Init() + sysThread.msg.Init(tid) + s.getThreadContextFromID(c.cid).ThreadID = tid + sysThread.msg.ContextID = c.cid sysThread.msg.Self = uint64(sysmsgStackAddr + sysmsg.MsgOffsetFromSharedStack) sysThread.msg.SyshandlerStack = uint64(sysmsg.StackAddrToSyshandlerStack(sysThread.sysmsgPerThreadMemAddr())) sysThread.msg.ContextRegion = uint64(stubContextRegion) - sysThread.msg.ContextID = c.cid sysThread.msg.Syshandler = uint64(stubSysmsgStart + uintptr(sysmsg.Sighandler_blob_offset____export_syshandler)) sysThread.msg.State.Set(sysmsg.ThreadStateDone) @@ -908,6 +918,10 @@ func (s *subprocess) getSysmsgThread(tregs *arch.Registers, c *context, ac *arch c.sysmsgThread = sysThread + s.sysmsgThreadsMu.Lock() + s.sysmsgThreads[tid] = sysThread + s.sysmsgThreadsMu.Unlock() + return sysThread, nil } @@ -928,11 +942,13 @@ func (s *subprocess) PostFork() { // subprocess. func (s *subprocess) registerContext(c *context) error { s.mu.Lock() + c.mu.Lock() // Unlock manually for the sake of not holding the lock while initializing // context memory. locked := true unlock := func() { if locked { + c.mu.Unlock() s.mu.Unlock() locked = false } @@ -953,7 +969,7 @@ func (s *subprocess) registerContext(c *context) error { unlock() threadContext := s.getThreadContextFromID(id) - threadContext.Init() + threadContext.Init(invalidThreadID) return nil } @@ -964,9 +980,15 @@ func (s *subprocess) unregisterContext(c *context) { if s == nil { return } + c.mu.Lock() + cid := c.cid + c.cid = invalidContextID + c.subprocess = nil + c.mu.Unlock() + s.mu.Lock() delete(s.faultedContexts, c) - s.threadContextPool.Put(c.cid) + s.threadContextPool.Put(cid) s.numContexts.Add(-1) released := s.released s.mu.Unlock() diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c index 2b7c0a2ac..c42d12399 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c @@ -178,7 +178,23 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { int32_t thread_state; thread_state = __atomic_load_n(&sysmsg->state, __ATOMIC_ACQUIRE); if (thread_state != THREAD_STATE_NONE) { - __atomic_store_n(&sysmsg->interrupt, 1, __ATOMIC_RELEASE); + // There are two possibilities for when we received the interrupt: + // 1. Before syshandler switched to the sentry. + // In this case we do not need to postpone the interrupt because it + // will be handled as soon as the Task returns to the sentry kernel. + // 2. After syshandler has received a response from the sentry. + // This is an interrupt most likely targeted at whatever context is + // bound to the sysmsg right now, but there is an unlikely case that + // an interrupt takes a while to reach the stub and the context has + // changed. For this reason we write which context ID the interrupt + // was meant for in sysmsg and check against that. + uint64_t interrupted_tid = + __atomic_load_n(&sysmsg->interrupted_context_id, __ATOMIC_ACQUIRE); + if (thread_state == THREAD_STATE_DONE && + (interrupted_tid == sysmsg->context_id)) { + __atomic_store_n(&sysmsg->interrupt, 1, __ATOMIC_RELEASE); + __atomic_store_n(&ctx->interrupt, 1, __ATOMIC_RELAXED); + } return; } } else if (signo == SIGILL && sysmsg->state == THREAD_STATE_INTERRUPT) { @@ -186,6 +202,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { signo = SIGCHLD; siginfo->si_signo = SIGCHLD; __atomic_store_n(&sysmsg->interrupt, 0, __ATOMIC_RELAXED); + __atomic_store_n(&ctx->interrupt, 0, __ATOMIC_RELAXED); // Skip the fault instruction. ucontext->uc_mcontext.gregs[REG_RIP] = sysmsg->ret_addr; // If we're skipping the fault instruction and going straight to the user diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go index 0bab82026..d2ec4accb 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go @@ -183,6 +183,8 @@ type Msg struct { stubFastPath uint32 sentryFastPath uint32 AckedEvents uint32 + // ThreadID is the ID of the sysmsg thread. + ThreadID uint32 } // ContextState defines the reason the context has exited back to the sentry, @@ -260,6 +262,9 @@ type ThreadContext struct { State ContextState // Interrupt is set to indicate that this context has been interrupted. Interrupt uint32 + // ThreadID is the ID of the sysmsg thread that's currently working on the + // context. + ThreadID uint32 // Debug is a variable to use to get visibility into the stub from the sentry. Debug uint64 } @@ -267,7 +272,7 @@ type ThreadContext struct { // LINT.ThenChange(sysmsg.h) // Init initializes the message. -func (m *Msg) Init() { +func (m *Msg) Init(threadID uint32) { m.Err = 0 m.Line = -1 m.stubFastPath = 0 @@ -275,12 +280,13 @@ func (m *Msg) Init() { } // Init initializes the ThreadContext instance. -func (c *ThreadContext) Init() { +func (c *ThreadContext) Init(initialThreadID uint32) { c.FPStateChanged = 1 c.Regs = linux.PtraceRegs{} c.Signo = 0 c.SignalInfo = linux.SignalInfo{} c.State = ContextStateNone + c.ThreadID = initialThreadID } // StubFastPath returns true if the stub thread in the polling mode. diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h index 465f43ac1..3dd62bbd9 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h @@ -70,6 +70,7 @@ struct sysmsg { uint32_t stub_fast_path; uint32_t sentry_fast_path; uint32_t acked_events; + uint32_t thread_id; }; enum context_state { @@ -94,6 +95,7 @@ struct thread_context { int64_t signo; uint32_t state; uint32_t interrupt; + uint32_t thread_id; uint64_t debug; }; diff --git a/pkg/sentry/platform/systrap/sysmsg_thread.go b/pkg/sentry/platform/systrap/sysmsg_thread.go index 74d4ea570..c5a133404 100644 --- a/pkg/sentry/platform/systrap/sysmsg_thread.go +++ b/pkg/sentry/platform/systrap/sysmsg_thread.go @@ -24,7 +24,6 @@ import ( "gvisor.dev/gvisor/pkg/seccomp" "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/memmap" - "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/pkg/sentry/platform/systrap/sysmsg" ) @@ -137,14 +136,6 @@ func (p *sysmsgThread) waitEvent(switchToState sysmsg.ThreadState) { } } -// NotifyInterrupt implements interrupt.Receiver.NotifyInterrupt. -func (p *sysmsgThread) NotifyInterrupt() { - t := p.thread - if _, _, e := unix.RawSyscall(unix.SYS_TGKILL, uintptr(t.tgid), uintptr(t.tid), uintptr(platform.SignalInterrupt)); e != 0 { - panic(fmt.Sprintf("failed to interrupt the child process %d: %v", t.tid, e)) - } -} - func (p *sysmsgThread) Debugf(format string, v ...any) { if !log.IsLogging(log.Debug) { return diff --git a/pkg/sentry/platform/systrap/systrap.go b/pkg/sentry/platform/systrap/systrap.go index 374379e57..085608833 100644 --- a/pkg/sentry/platform/systrap/systrap.go +++ b/pkg/sentry/platform/systrap/systrap.go @@ -52,7 +52,9 @@ import ( "fmt" "os" "sync" + "sync/atomic" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" pkgcontext "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/cpuid" @@ -105,18 +107,16 @@ type context struct { // interrupt is the interrupt context. interrupt interrupt.Forwarder + // mu protects the following fields. + mu sync.Mutex + // subprocess is the current subprocess used to execute the context. - // It is only updated in Switch, and used in Release, so only on the task - // goroutine. subprocess *subprocess // cid is the ID of the context in the address space of the current // subprocess used to run it. cid uint64 - // mu protects the following fields. - mu sync.Mutex - // If lastFaultSP is non-nil, the last context switch was due to a fault // received while executing lastFaultSP. Only context.Switch may set // lastFaultSP to a non-nil value. @@ -278,6 +278,46 @@ func (c *context) Interrupt() { c.interrupt.NotifyInterrupt() } +// NotifyInterrupt implements interrupt.Receiver.NotifyInterrupt. +// +// Another reasonable existing object to implement NotifyInterrupt would be +// sysmsg.ThreadContext, because it already has the correct tid written into it +// to know which thread to send the signal to. However we cannot do that because +// it is in shared memory, which means that one subprocess can overwrite it to +// have the sentry send an interrupt to a completely different subprocess. +// For this reason we use systrap.context and check that the target thread +// is actually valid within the subprocess. +func (c *context) NotifyInterrupt() { + c.mu.Lock() + s := c.subprocess + cid := c.cid + c.mu.Unlock() + + if s == nil || cid == invalidContextID { + return + } + + threadContext := s.getThreadContextFromID(cid) + atomic.StoreUint32(&threadContext.Interrupt, 1) + threadID := atomic.LoadUint32(&threadContext.ThreadID) + + s.sysmsgThreadsMu.Lock() + defer s.sysmsgThreadsMu.Unlock() + + sysmsgThread, ok := s.sysmsgThreads[threadID] + if !ok { + // This is either an invalidThreadID or another garbage value; either way we + // don't know which thread to interrupt; best we can do is mark the context. + return + } + + t := sysmsgThread.thread + atomic.StoreUint64(&sysmsgThread.msg.InterruptedContextID, cid) + if _, _, e := unix.RawSyscall(unix.SYS_TGKILL, uintptr(t.tgid), uintptr(t.tid), uintptr(platform.SignalInterrupt)); e != 0 { + panic(fmt.Sprintf("failed to interrupt the child process %d: %v", t.tid, e)) + } +} + // Release releases all platform resources used by the context. func (c *context) Release() { if c.sysmsgThread != nil {