diff --git a/pkg/sentry/platform/systrap/shared_context.go b/pkg/sentry/platform/systrap/shared_context.go index 41ffb996b..a3b295d0e 100644 --- a/pkg/sentry/platform/systrap/shared_context.go +++ b/pkg/sentry/platform/systrap/shared_context.go @@ -40,7 +40,7 @@ type sharedContext struct { subprocess *subprocess // contextID is the ID corresponding to the sysmsg.ThreadContext memory slot // that is used for this sharedContext. - contextID uint64 + contextID uint32 // shared is the handle to the shared memory that the sentry task goroutine // reads from and writes to. // NOTE: Using this handle directly without a getter from this function should @@ -59,7 +59,7 @@ func (s *subprocess) getSharedContext() (*sharedContext, error) { s.IncRef() sc := sharedContext{ subprocess: s, - contextID: id, + contextID: uint32(id), shared: s.getThreadContextFromID(id), } sc.shared.Init(invalidThreadID) @@ -71,7 +71,7 @@ func (sc *sharedContext) release() { if sc == nil { return } - sc.subprocess.threadContextPool.Put(sc.contextID) + sc.subprocess.threadContextPool.Put(uint64(sc.contextID)) sc.subprocess.DecRef(sc.subprocess.release) } @@ -102,7 +102,6 @@ func (sc *sharedContext) NotifyInterrupt() { } t := sysmsgThread.thread - atomic.StoreUint64(&sysmsgThread.msg.InterruptedContextID, sc.contextID) 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)) } diff --git a/pkg/sentry/platform/systrap/subprocess.go b/pkg/sentry/platform/systrap/subprocess.go index 70ebfe760..a2901f951 100644 --- a/pkg/sentry/platform/systrap/subprocess.go +++ b/pkg/sentry/platform/systrap/subprocess.go @@ -714,7 +714,10 @@ func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool c.signalInfo = linux.SignalInfo{Signo: int32(platform.SignalInterrupt)} return false, false, nil } - defer c.interrupt.Disable() + defer func() { + ctx.clearInterrupt() + c.interrupt.Disable() + }() if contextDecouplingExp { restoreFPState(nil, ctx, 0, c, ac) @@ -1036,7 +1039,7 @@ func (s *subprocess) createSysmsgThread(tregs *arch.Registers, c *context, ac *a sysThread.setMsg(sysmsg.StackAddrToMsg(sentryStackAddr)) sysThread.msg.Init(threadID) if contextDecouplingExp { - sysThread.msg.ContextID = uint64(invalidContextID) + sysThread.msg.ContextID = invalidContextID } else { c.sharedContext.setThreadID(threadID) sysThread.msg.ContextID = c.sharedContext.contextID diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c index 1161f89ad..193fe86f3 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c @@ -211,23 +211,8 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { // If the current thread is in syshandler, an interrupt has to be postponed, // because sysmsg can't be changed. if (thread_state != THREAD_STATE_NONE) { - // 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)) { + if (__atomic_load_n(&ctx->interrupt, __ATOMIC_ACQUIRE)) __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) { @@ -376,7 +361,6 @@ void __syshandler() { ctx->siginfo.si_addr = 0; ctx->siginfo.si_syscall = ctx->ptregs.rax; ctx->ptregs.rax = (unsigned long)-ENOSYS; - __atomic_store_n(&sysmsg->interrupt, 0, __ATOMIC_RELAXED); switch_context_amd64(sysmsg, ctx, THREAD_STATE_EVENT, ctx_state); } diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go index 7c1e8fa8f..a103454b5 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go @@ -152,16 +152,14 @@ type Msg struct { // State indicates to the sentry what the sysmsg thread is doing at a given // moment. State ThreadState - // ContextID is the ID of the ThreadContext struct that the current - // sysmsg thread is is processing. This ID is used in the {sig|sys}handler - // to find the offset to the correct ThreadContext struct location. - ContextID uint64 // ContextRegion defines the ThreadContext memory region start within // the sysmsg thread address space. ContextRegion uint64 + // ContextID is the ID of the ThreadContext struct that the current + // sysmsg thread is is processing. This ID is used in the {sig|sys}handler + // to find the offset to the correct ThreadContext struct location. + ContextID uint32 - // InterruptedContextID is the target of the interrupt sent to sysmsg thread. - InterruptedContextID uint64 // FaultJump is the size of a faulted instruction. FaultJump int32 // Err is the error value with which the {sig|sys}handler crashes the stub diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h index 16d973282..b1def2359 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h @@ -57,12 +57,11 @@ struct sysmsg { uint64_t app_stack; uint32_t interrupt; uint32_t state; - uint64_t context_id; uint64_t context_region; + uint32_t context_id; // The fields above have offsets defined in sysmsg_offsets*.h - uint64_t interrupted_context_id; int32_t fault_jump; int32_t err; int32_t err_line; diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c index 8be2c9b95..ec5328d60 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c @@ -234,6 +234,7 @@ struct thread_context *switch_context(struct sysmsg *sysmsg, panic(ret); } } + uint32_t old_ctx_id = sysmsg->context_id; ctx = get_context(sysmsg); diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg_offsets.h b/pkg/sentry/platform/systrap/sysmsg/sysmsg_offsets.h index 7d6862393..fe02d4a68 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg_offsets.h +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg_offsets.h @@ -39,8 +39,8 @@ #define offsetof_sysmsg_app_stack 0x20 #define offsetof_sysmsg_interrupt 0x28 #define offsetof_sysmsg_state 0x2c -#define offsetof_sysmsg_context_id 0x30 -#define offsetof_sysmsg_context_region 0x38 +#define offsetof_sysmsg_context_region 0x30 +#define offsetof_sysmsg_context_id 0x38 #define offsetof_thread_context_fpstate 0x0 #define offsetof_thread_context_fpstate_changed MAX_FPSTATE_LEN