From fe66cae2edc7796c4b073e4fde0649adff566e33 Mon Sep 17 00:00:00 2001 From: Konstantin Bogomolov Date: Tue, 6 Feb 2024 13:17:11 -0800 Subject: [PATCH] Enumerate known systrap stub failures to exit process cleanly. This helps to rectify a long standing problem of Systrap panicking when encountering corrupted sysmsg stub memory. These errors specifically are easier to notice and debug since we check for them in the stub code and flag them to the sentry explicitly. They are now very grep-able to make finding their origin in the stub code easier. PiperOrigin-RevId: 604743496 --- pkg/sentry/kernel/task_syscall.go | 3 + pkg/sentry/platform/BUILD | 1 + pkg/sentry/platform/platform.go | 14 +++++ pkg/sentry/platform/systrap/subprocess.go | 5 +- pkg/sentry/platform/systrap/sysmsg/BUILD | 1 + .../systrap/sysmsg/sighandler_amd64.c | 10 +-- .../systrap/sysmsg/sighandler_arm64.c | 7 ++- pkg/sentry/platform/systrap/sysmsg/sysmsg.go | 61 +++++++++++++++++++ pkg/sentry/platform/systrap/sysmsg/sysmsg.h | 16 ++++- .../platform/systrap/sysmsg/sysmsg_lib.c | 10 +-- 10 files changed, 111 insertions(+), 17 deletions(-) diff --git a/pkg/sentry/kernel/task_syscall.go b/pkg/sentry/kernel/task_syscall.go index 87cf67bb5..7fc011f2c 100644 --- a/pkg/sentry/kernel/task_syscall.go +++ b/pkg/sentry/kernel/task_syscall.go @@ -29,6 +29,7 @@ import ( "gvisor.dev/gvisor/pkg/metric" "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/memmap" + "gvisor.dev/gvisor/pkg/sentry/platform" "gvisor.dev/gvisor/pkg/sentry/seccheck" pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto" ) @@ -477,6 +478,8 @@ func ExtractErrno(err error, sysno int) int { return ExtractErrno(err.Err, sysno) case *os.SyscallError: return ExtractErrno(err.Err, sysno) + case *platform.ContextError: + return int(err.Errno) default: if errno, ok := linuxerr.TranslateError(err); ok { return int(linuxerr.ToUnix(errno)) diff --git a/pkg/sentry/platform/BUILD b/pkg/sentry/platform/BUILD index bcb06d63a..5da0cae31 100644 --- a/pkg/sentry/platform/BUILD +++ b/pkg/sentry/platform/BUILD @@ -26,5 +26,6 @@ go_library( "//pkg/sentry/hostmm", "//pkg/sentry/memmap", "//pkg/usermem", + "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/sentry/platform/platform.go b/pkg/sentry/platform/platform.go index 29ddfb1b8..fd78372cf 100644 --- a/pkg/sentry/platform/platform.go +++ b/pkg/sentry/platform/platform.go @@ -21,6 +21,7 @@ import ( "fmt" "os" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/hostarch" @@ -277,6 +278,19 @@ type Context interface { PrepareSleep() } +// ContextError is one of the possible errors returned by Context.Switch(). +type ContextError struct { + // Err is the underlying error. + Err error + // Errno is an approximation of what type of error this is supposed to + // be as defined by the linux errnos. + Errno unix.Errno +} + +func (e *ContextError) Error() string { + return e.Err.Error() +} + var ( // ErrContextSignal is returned by Context.Switch() to indicate that the // Context was interrupted by a signal. diff --git a/pkg/sentry/platform/systrap/subprocess.go b/pkg/sentry/platform/systrap/subprocess.go index 7c3b8f2cd..7407f6b92 100644 --- a/pkg/sentry/platform/systrap/subprocess.go +++ b/pkg/sentry/platform/systrap/subprocess.go @@ -717,7 +717,7 @@ func (s *subprocess) decAwakeContexts() { // This function returns true on a system call, false on a signal. // The second return value is true if a syscall instruction can be replaced on // a function call. -func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool, shouldPatchSyscall bool, err error) { +func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool, shouldPatchSyscall bool, err *platform.ContextError) { // Reset necessary registers. regs := &ac.StateData().Regs s.resetSysemuRegs(regs) @@ -752,8 +752,7 @@ func (s *subprocess) switchToApp(c *context, ac *arch.Context64) (isSyscall bool threadID := ctx.threadID() if threadID != invalidThreadID { if sysThread, ok := s.sysmsgThreads[threadID]; ok && sysThread.msg.Err != 0 { - msg := sysThread.msg - panic(fmt.Sprintf("stub thread %d failed: err 0x%x line %d: %s", sysThread.thread.tid, msg.Err, msg.Line, msg)) + return false, false, sysThread.msg.ConvertSysmsgErr() } log.Warningf("systrap: found unexpected ThreadContext.ThreadID field, expected %d found %d", invalidThreadID, threadID) } diff --git a/pkg/sentry/platform/systrap/sysmsg/BUILD b/pkg/sentry/platform/systrap/sysmsg/BUILD index 145a752b6..418036e42 100644 --- a/pkg/sentry/platform/systrap/sysmsg/BUILD +++ b/pkg/sentry/platform/systrap/sysmsg/BUILD @@ -131,6 +131,7 @@ go_library( "//pkg/abi/linux", "//pkg/cpuid", "//pkg/hostarch", + "//pkg/sentry/platform", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c index 184c9cac5..65776c95a 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c @@ -138,7 +138,7 @@ static uint64_t get_fsbase(void) { int ret = __syscall(__NR_arch_prctl, ARCH_GET_FS, (long)&fsbase, 0, 0, 0, 0); if (ret) { - panic(ret); + panic(STUB_ERROR_ARCH_PRCTL, ret); } } return fsbase; @@ -151,7 +151,7 @@ static void set_fsbase(uint64_t fsbase) { } else { int ret = __syscall(__NR_arch_prctl, ARCH_SET_FS, fsbase, 0, 0, 0, 0); if (ret) { - panic(ret); + panic(STUB_ERROR_ARCH_PRCTL, ret); } } } @@ -198,7 +198,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { void *sp = sysmsg_sp(); struct sysmsg *sysmsg = sysmsg_addr(sp); - if (sysmsg != sysmsg->self) panic(0xdeaddead); + if (sysmsg != sysmsg->self) panic(STUB_ERROR_BAD_SYSMSG, 0); int32_t thread_state = atomic_load(&sysmsg->state); if (thread_state == THREAD_STATE_INITIALIZING) { // This thread was interrupted before it even had a context. @@ -340,7 +340,7 @@ void __syshandler() { // SYSMSG_STATE_PREP is set to postpone interrupts. Look at // __export_sighandler for more details. int state = atomic_load(&sysmsg->state); - if (state != THREAD_STATE_PREP) panic(state); + if (state != THREAD_STATE_PREP) panic(STUB_ERROR_BAD_THREAD_STATE, 0); struct thread_context *ctx = sysmsg->context; @@ -368,7 +368,7 @@ void __export_start(struct sysmsg *sysmsg, void *_ucontext) { asm volatile("movq %%gs:0, %0\n" : "=r"(sysmsg) : :); if (sysmsg->self != sysmsg) { - panic(0xdeaddead); + panic(STUB_ERROR_BAD_SYSMSG, 0); } struct thread_context *ctx = diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c index 2cb688a47..2470a755e 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c @@ -92,7 +92,7 @@ static void ptregs_to_gregs(ucontext_t *ucontext, } void __export_start(struct sysmsg *sysmsg, void *_ucontext) { - panic(0x11111111); + panic(0x11111111, 0); } void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { @@ -100,7 +100,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { void *sp = sysmsg_sp(); struct sysmsg *sysmsg = sysmsg_addr(sp); - if (sysmsg != sysmsg->self) panic(0xdeaddead); + if (sysmsg != sysmsg->self) panic(STUB_ERROR_BAD_SYSMSG, 0); int32_t thread_state = atomic_load(&sysmsg->state); uint32_t ctx_state = CONTEXT_STATE_INVALID; @@ -125,7 +125,8 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { const uint64_t kSigframeMagicHeaderLen = sizeof(struct _aarch64_ctx); // Verify the header. if (((uint32_t *)&ucontext->uc_mcontext.__reserved)[0] != FPSIMD_MAGIC) { - panic(0xbadf); + panic(STUB_ERROR_FPSTATE_BAD_HEADER, + ((uint32_t *)&ucontext->uc_mcontext.__reserved)[0]); } uint8_t *fpStatePointer = (uint8_t *)&ucontext->uc_mcontext.__reserved + kSigframeMagicHeaderLen; diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go index dd061d7c8..b8ab2a549 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.go +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.go @@ -25,8 +25,10 @@ import ( "strings" "sync/atomic" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/hostarch" + "gvisor.dev/gvisor/pkg/sentry/platform" ) // LINT.IfChange @@ -154,6 +156,9 @@ type Msg struct { // Err is the error value with which the {sig|sys}handler crashes the stub // thread (see sysmsg.h:__panic). Err int32 + // ErrAdditional is an error value that gives additional information + // about the panic. + ErrAdditional int32 // Line is the code line on which the {sig|sys}handler crashed the stub thread // (see sysmsg.h:panic). Line int32 @@ -266,11 +271,37 @@ type ThreadContext struct { Debug uint64 } +// StubError are values that represent known stub-thread failure modes. +// Since these errors originate from the stub threads, look at +// sysmsg.h:stub_error. +type StubError int32 + +const ( + // StubErrorBadSysmsg indicates sysmsg->self did not match sysmsg. + StubErrorBadSysmsg StubError = 0x0bad0000 + iota + // StubErrorBadThreadState indicates sysmsg->state was invalid. + StubErrorBadThreadState + // StubErrorBadSpinningQueueDecref indicates stubs removed more threads + // from spinning queue than were put in. + StubErrorBadSpinningQueueDecref + // StubErrorArchPrctl indicates an error when calling arch_prctl. + StubErrorArchPrctl + // StubErrorFutex indicates an error when calling futex. + StubErrorFutex + // StubErrorBadContextID indicates a context received from the context + // queue was of unexpected value. + StubErrorBadContextID + // StubErrorFpStateBadHeader indicates that the floating point state + // header did not match the expected value. + StubErrorFpStateBadHeader +) + // LINT.ThenChange(sysmsg.h) // Init initializes the message. func (m *Msg) Init(threadID uint32) { m.Err = 0 + m.ErrAdditional = 0 m.Line = -1 m.ThreadID = threadID m.Context = 0 @@ -286,6 +317,36 @@ func (c *ThreadContext) Init(initialThreadID uint32) { c.ThreadID = initialThreadID } +// ConvertSysmsgErr converts m.Err to platform.ContextError. +func (m *Msg) ConvertSysmsgErr() *platform.ContextError { + err := &platform.ContextError{ + Errno: unix.EPERM, + } + + const prefix = "systrap stub thread failure:" + suffix := fmt.Sprintf("(failed on line %d; %s)", atomic.LoadInt32(&m.Line), m.String()) + switch StubError(atomic.LoadInt32(&m.Err)) { + case StubErrorBadSysmsg: + err.Err = fmt.Errorf("%s sysmsg->self did not match sysmsg during sig/sys-handler %s", prefix, suffix) + case StubErrorBadThreadState: + err.Err = fmt.Errorf("%s sysmsg->state was invalid during sys-handler %s", prefix, suffix) + case StubErrorBadSpinningQueueDecref: + err.Err = fmt.Errorf("%s imbalanced use of spinning queue %s", prefix, suffix) + case StubErrorArchPrctl: + err.Err = fmt.Errorf("%s arch_prctl error=0x%x %s", prefix, atomic.LoadInt32(&m.ErrAdditional), suffix) + case StubErrorFutex: + err.Err = fmt.Errorf("%s futex error=0x%x %s", prefix, atomic.LoadInt32(&m.ErrAdditional), suffix) + case StubErrorBadContextID: + err.Err = fmt.Errorf("%s unexpected context ID (%d) from context queue %s", prefix, atomic.LoadInt32(&m.ErrAdditional), suffix) + case StubErrorFpStateBadHeader: + err.Err = fmt.Errorf("%s FP state context magic header (%d) does not match expected FPSIMD_MAGIC %s", prefix, atomic.LoadInt32(&m.ErrAdditional), suffix) + default: + err.Err = fmt.Errorf("%s unknown reason (0x%x) (possible shared memory corruption) %s", prefix, atomic.LoadInt32(&m.Err), suffix) + } + + return err +} + func (m *Msg) String() string { var b strings.Builder fmt.Fprintf(&b, "sysmsg.Msg{msg: %x state %d", m.Self, m.State) diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h index bcce52c5f..07ba6f8bd 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg.h +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg.h @@ -63,6 +63,7 @@ struct sysmsg { int32_t fault_jump; int32_t err; + int32_t err_additional; int32_t err_line; uint64_t debug; uint32_t thread_id; @@ -99,6 +100,16 @@ struct thread_context { uint64_t debug; }; +enum stub_error { + STUB_ERROR_BAD_SYSMSG = 0x0bad0000, + STUB_ERROR_BAD_THREAD_STATE, + STUB_ERROR_SPINNING_QUEUE_DECREF, + STUB_ERROR_ARCH_PRCTL, + STUB_ERROR_FUTEX, + STUB_ERROR_BAD_CONTEXT_ID, + STUB_ERROR_FPSTATE_BAD_HEADER, +}; + #ifndef PAGE_SIZE #define PAGE_SIZE 4096 #endif @@ -143,11 +154,12 @@ struct __kernel_timespec; long sys_futex(uint32_t *addr, int op, int val, struct __kernel_timespec *tv, uint32_t *addr2, int val3); -static void __panic(int err, long line) { +static void __panic(int err, int err_additional, long line) { void *sp = sysmsg_sp(); struct sysmsg *sysmsg = sysmsg_addr(sp); struct thread_context *ctx = sysmsg->context; sysmsg->err = err; + sysmsg->err_additional = err_additional; sysmsg->err_line = line; // Wake up the goroutine waiting on the current context. __atomic_store_n(&ctx->state, CONTEXT_STATE_FAULT, __ATOMIC_RELEASE); @@ -177,7 +189,7 @@ struct thread_context *switch_context(struct sysmsg *sysmsg, int wait_state(struct sysmsg *sysmsg, enum thread_state new_thread_state); void init_new_thread(void); -#define panic(err) __panic(err, __LINE__) +#define panic(err, err_additional) __panic(err, err_additional, __LINE__) // NOLINTEND(runtime/int) #endif // THIRD_PARTY_GVISOR_PKG_SENTRY_PLATFORM_SYSTRAP_SYSMSG_SYSMSG_H_ diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c index 69fc85f9d..204b642fd 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c @@ -224,7 +224,7 @@ struct thread_context *queue_get_context(struct sysmsg *sysmsg) { atomic_add(&queue->start, 1); if (context_id > MAX_GUEST_CONTEXTS) { - panic(context_id); + panic(STUB_ERROR_BAD_CONTEXT_ID, context_id); } struct thread_context *ctx = thread_context_addr(context_id); sysmsg->context = ctx; @@ -254,7 +254,8 @@ static struct thread_context *get_context_fast(struct sysmsg *sysmsg, } if (atomic_load(&queue->fast_path_disabled) != 0) { - if (!spinning_queue_remove_first(0)) panic(0); + if (!spinning_queue_remove_first(0)) + panic(STUB_ERROR_SPINNING_QUEUE_DECREF, 0); break; } @@ -265,7 +266,8 @@ static struct thread_context *get_context_fast(struct sysmsg *sysmsg, if (atomic_compare_exchange(&queue->num_active_threads, &nr_active_threads, nr_active_threads - 1)) { nr_active_threads -= 1; - if (!spinning_queue_remove_first(0)) panic(0); + if (!spinning_queue_remove_first(0)) + panic(STUB_ERROR_SPINNING_QUEUE_DECREF, 0); *nr_active_threads_p = nr_active_threads; break; } @@ -380,7 +382,7 @@ struct thread_context *switch_context(struct sysmsg *sysmsg, if (atomic_load(&ctx->sentry_fast_path) == 0) { int ret = sys_futex(&ctx->state, FUTEX_WAKE, 1, NULL, NULL, 0); if (ret < 0) { - panic(ret); + panic(STUB_ERROR_FUTEX, ret); } } }