From 263dad62587c435154236e7dfe2d9fa4dd185006 Mon Sep 17 00:00:00 2001 From: Konstantin Bogomolov Date: Wed, 8 Mar 2023 09:53:50 -0800 Subject: [PATCH] Handle context interrupts based on syshandler state. Also add interrupt handling for context decoupling. Previously syshandler interrupts would be retriggered no matter if the interrupt arrived before the switch to sentry or after. We only need to handle the case of it arriving after. Additionally this CL introduces interrupt handling for the decoupled context mode, by making interrupts target task contexts rather than sysmsg threads. PiperOrigin-RevId: 515065101 --- pkg/sentry/platform/systrap/subprocess.go | 34 ++++++++++--- .../systrap/sysmsg/sighandler_amd64.c | 19 ++++++- pkg/sentry/platform/systrap/sysmsg/sysmsg.go | 10 +++- pkg/sentry/platform/systrap/sysmsg/sysmsg.h | 2 + pkg/sentry/platform/systrap/sysmsg_thread.go | 9 ---- pkg/sentry/platform/systrap/systrap.go | 50 +++++++++++++++++-- 6 files changed, 101 insertions(+), 23 deletions(-) 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 {