platform/kvm: use per-thread signal stack to run bluepill handler

The user-space bluepill introduced the issue. With that change, we use a
per-vcpu stack to run bluepillHandler. BluepillHanlder releases a vcpu before
calling sigreturn, so it is still running on the vcpu stack. The race looks
like this: one thread releases a vcpu, another thread takes it and  starts
using its stack, the first thread calls sigreturn with a corrupted signal
frame.



PiperOrigin-RevId: 684992950

nogo.yaml

@@ -205,6 +205,7 @@ analyzers:
         - pkg/sentry/platform/safecopy/safecopy_unsafe.go          # Special case.
         - pkg/sentry/usage/memory_unsafe.go                        # Special case.
         - pkg/sentry/vfs/mount_unsafe.go                           # Special case.
+        - pkg/sigframe/sigframe_amd64_unsafe.go                    # Special case.
         - pkg/state/decode_unsafe.go                               # Special case.
   unusedresult:
     external: # Enabled.

pkg/sentry/platform/kvm/bluepill_amd64.s

@@ -91,3 +91,13 @@ TEXT ·currentCPU(SB), $0-8
 	MOVQ ENTRY_CPU_SELF(GS), AX
 	MOVQ AX, ret+0(FP)
 	RET
+
+TEXT ·rdfsbase(SB), $0-8
+	BYTE $0xf3; BYTE $0x48; BYTE $0x0f; BYTE $0xae; BYTE $0xc0;
+	MOVQ AX, ret+0(FP)
+	RET
+
+TEXT ·rdgsbase(SB), $0-8
+	BYTE $0xf3; BYTE $0x48; BYTE $0x0f; BYTE $0xae; BYTE $0xc8;
+	MOVQ AX, ret+0(FP)
+	RET

pkg/sentry/platform/kvm/bluepill_amd64_unsafe.go

@@ -18,13 +18,10 @@
 package kvm
 
 import (
-	"fmt"
 	"unsafe"
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
-	"gvisor.dev/gvisor/pkg/cpuid"
-	"gvisor.dev/gvisor/pkg/hostarch"
 	"gvisor.dev/gvisor/pkg/hostsyscall"
 	"gvisor.dev/gvisor/pkg/ring0"
 	"gvisor.dev/gvisor/pkg/sentry/arch"
@@ -170,10 +167,9 @@ func bluepill(c *vCPU) {
 
 	// Block all signals.
 	sigmask := linux.SignalSet(^uint64(0))
-	c.bluepillSigframe.MContext.Rax = uint64(uintptr(unsafe.Pointer(c)))
 	if err := sigframe.CallWithSignalFrame(
-		c.bluepillStack, addrOfBluepillUserHandler(),
-		c.bluepillSigframe, c.bluepillSigframeFPState, &sigmask); err != nil {
+		&c.signalStack, addrOfBluepillUserHandler(),
+		&sigmask, uint64(uintptr(unsafe.Pointer(c)))); err != nil {
 		throw("failed to swallow the bluepill")
 	}
 }
@@ -182,33 +178,6 @@ func bluepill(c *vCPU) {
 //
 //go:nosplit
 func bluepillUserHandler(frame uintptr) {
-	// Sanitize the registers; interrupts must always be disabled.
-	context := bluepillArchContext(unsafe.Pointer(frame))
-	c := vCPUPtr(uintptr(context.Rax))
 	bluepillHandler(unsafe.Pointer(frame))
-	sigframe.Sigreturn(c.bluepillSigframe)
-}
-
-func (c *vCPU) initBluepillHandler() error {
-	stackSize := uintptr(hostarch.PageSize)
-
-	maxFPUSizeUint, fpuAlignmentUint := cpuid.HostFeatureSet().ExtendedStateSize()
-	fpuAlignment := uintptr(fpuAlignmentUint)
-	maxFPUSize := uintptr(maxFPUSizeUint)
-
-	fpuOffset := (unsafe.Sizeof(arch.UContext64{}) + fpuAlignment - 1) / fpuAlignment * fpuAlignment
-	mappingSize, _ := hostarch.PageRoundUp(stackSize + maxFPUSize + fpuOffset)
-
-	addr, _, errno := unix.Syscall6(unix.SYS_MMAP, 0, mappingSize,
-		uintptr(unix.PROT_READ|unix.PROT_WRITE),
-		uintptr(unix.MAP_PRIVATE|unix.MAP_ANONYMOUS),
-		0, 0)
-	if errno != 0 {
-		return fmt.Errorf("mmap failed: %d", errno)
-	}
-	c.bluepillStack = addr + stackSize
-	c.bluepillSigframe = (*arch.UContext64)(unsafe.Pointer(addr + stackSize))
-	c.bluepillSigframeFPState = addr + stackSize + fpuOffset
-
-	return nil
+	sigframe.Sigreturn((*arch.UContext64)(unsafe.Pointer(frame)))
 }

pkg/sentry/platform/kvm/kvm_amd64.go

@@ -218,9 +218,13 @@ func (c *cpuidEntries) Set(in cpuid.In, out cpuid.Out) {
 	}
 }
 
+var hasFSGSBASE bool
+
 // updateGlobalOnce does global initialization. It has to be called only once.
 func updateGlobalOnce(fd int) error {
+	hasFSGSBASE = cpuid.HostFeatureSet().UseFSGSBASE()
 	fpu.InitHostState()
+
 	bitsForScaling = getBitsForScaling()
 	if err := updateSystemValues(int(fd)); err != nil {
 		return err

pkg/sentry/platform/kvm/machine_amd64.go

@@ -31,7 +31,6 @@ import (
 	"gvisor.dev/gvisor/pkg/hostsyscall"
 	"gvisor.dev/gvisor/pkg/ring0"
 	"gvisor.dev/gvisor/pkg/ring0/pagetables"
-	"gvisor.dev/gvisor/pkg/sentry/arch"
 	"gvisor.dev/gvisor/pkg/sentry/platform"
 	ktime "gvisor.dev/gvisor/pkg/sentry/time"
 )
@@ -80,9 +79,8 @@ type vCPUArchState struct {
 	// This starts above fixedKernelPCID.
 	PCIDs *pagetables.PCIDs
 
-	bluepillStack           uintptr
-	bluepillSigframe        *arch.UContext64
-	bluepillSigframeFPState uintptr
+	// signalStack is the signal stack of the last thread bound to this vCPU.
+	signalStack linux.SignalStack
 }
 
 const (
@@ -101,10 +99,6 @@ const (
 
 // initArchState initializes architecture-specific state.
 func (c *vCPU) initArchState() error {
-	if err := c.initBluepillHandler(); err != nil {
-		return err
-	}
-
 	var (
 		kernelSystemRegs systemRegs
 		kernelUserRegs   userRegs

pkg/sentry/platform/kvm/machine_amd64_unsafe.go

@@ -26,25 +26,36 @@ import (
 	"gvisor.dev/gvisor/pkg/hostsyscall"
 )
 
+func rdfsbase() uint64
+func rdgsbase() uint64
+
 // loadSegments copies the current segments.
 //
 // This may be called from within the signal context and throws on error.
 //
 //go:nosplit
 func (c *vCPU) loadSegments(tid uint64) {
-	if errno := hostsyscall.RawSyscallErrno(
-		unix.SYS_ARCH_PRCTL,
-		linux.ARCH_GET_FS,
-		uintptr(unsafe.Pointer(&c.CPU.Registers().Fs_base)),
-		0); errno != 0 {
-		throw("getting FS segment")
+	if errno := hostsyscall.RawSyscallErrno(unix.SYS_SIGALTSTACK, 0, uintptr(unsafe.Pointer(&c.signalStack)), 0); errno != 0 {
+		throw("sigaltstack")
 	}
-	if errno := hostsyscall.RawSyscallErrno(
-		unix.SYS_ARCH_PRCTL,
-		linux.ARCH_GET_GS,
-		uintptr(unsafe.Pointer(&c.CPU.Registers().Gs_base)),
-		0); errno != 0 {
-		throw("getting GS segment")
+	if hasFSGSBASE {
+		c.CPU.Registers().Fs_base = rdfsbase()
+		c.CPU.Registers().Gs_base = rdgsbase()
+	} else {
+		if errno := hostsyscall.RawSyscallErrno(
+			unix.SYS_ARCH_PRCTL,
+			linux.ARCH_GET_FS,
+			uintptr(unsafe.Pointer(&c.CPU.Registers().Fs_base)),
+			0); errno != 0 {
+			throw("getting FS segment")
+		}
+		if errno := hostsyscall.RawSyscallErrno(
+			unix.SYS_ARCH_PRCTL,
+			linux.ARCH_GET_GS,
+			uintptr(unsafe.Pointer(&c.CPU.Registers().Gs_base)),
+			0); errno != 0 {
+			throw("getting GS segment")
+		}
 	}
 	c.tid.Store(tid)
 }

pkg/sigframe/BUILD

@@ -14,6 +14,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/abi/linux",
+        "//pkg/hostarch",
         "//pkg/hostsyscall",
         "//pkg/sentry/arch",
         "@org_golang_x_sys//unix:go_default_library",

pkg/sigframe/sigframe_amd64.s

@@ -48,14 +48,11 @@
 // Caller-Save: RAX, RCX, RDX, RSI, RDI, and R8-R11 are free to be used by
 // the called function and may be overwritten.
 // retjmp has to be updated when the stack frame size is changed.
-TEXT ·callWithSignalFrame(SB),NOSPLIT,$8-32
+TEXT ·callWithSignalFrame(SB),NOSPLIT,$8-24
 	MOVQ stack+0(FP), DI
 	MOVQ handler+8(FP), AX
 	MOVQ sigframe+16(FP), R8
 
-	MOVQ fpstate+24(FP), R9
-	MOVQ R9, SIGCTX_FPSTATE(R8)
-
 	MOVQ BX, SIGCTX_RBX(R8)
 	MOVQ BP, SIGCTX_RBP(R8)
 	MOVQ R12, SIGCTX_R12(R8)

pkg/sigframe/sigframe_amd64_test.go

@@ -19,7 +19,6 @@ package sigframe
 
 import (
 	"testing"
-	"unsafe"
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
@@ -31,19 +30,25 @@ func TestUserSignalHandler(t *testing.T) {
 	addr, _, _ := unix.Syscall6(unix.SYS_MMAP, 0, hostarch.PageSize*4, uintptr(unix.PROT_READ|unix.PROT_WRITE),
 		uintptr(unix.MAP_PRIVATE|unix.MAP_ANONYMOUS),
 		0, 0)
+	stack := linux.SignalStack{
+		Addr: uint64(addr),
+		Size: hostarch.PageSize * 4,
+	}
 	set := linux.MakeSignalSet(linux.SIGURG)
-	sigframe := (*arch.UContext64)(unsafe.Pointer(addr + hostarch.PageSize))
-	CallWithSignalFrame(addr+hostarch.PageSize, addrOfUserHandler(), sigframe, 0, &set)
+	CallWithSignalFrame(&stack, addrOfUserHandler(), &set, 0)
 }
 
 func BenchmarkUserSigHandler(b *testing.B) {
 	addr, _, _ := unix.Syscall6(unix.SYS_MMAP, 0, hostarch.PageSize*4, uintptr(unix.PROT_READ|unix.PROT_WRITE),
 		uintptr(unix.MAP_PRIVATE|unix.MAP_ANONYMOUS),
 		0, 0)
+	stack := linux.SignalStack{
+		Addr: uint64(addr),
+		Size: hostarch.PageSize * 4,
+	}
 	set := linux.MakeSignalSet(linux.SIGURG)
-	sigframe := (*arch.UContext64)(unsafe.Pointer(addr + hostarch.PageSize))
 	for i := 0; i < b.N; i++ {
-		CallWithSignalFrame(addr+hostarch.PageSize, addrOfUserHandler(), sigframe, 0, &set)
+		CallWithSignalFrame(&stack, addrOfUserHandler(), &set, 0)
 	}
 }
 

pkg/sigframe/sigframe_amd64_unsafe.go

@@ -22,11 +22,12 @@ import (
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/hostarch"
 	"gvisor.dev/gvisor/pkg/hostsyscall"
 	"gvisor.dev/gvisor/pkg/sentry/arch"
 )
 
-func callWithSignalFrame(stack uintptr, handler uintptr, sigframe *arch.UContext64, fpstate uintptr)
+func callWithSignalFrame(stack uintptr, handler uintptr, sigframe *arch.UContext64)
 
 //go:linkname throw runtime.throw
 func throw(s string)
@@ -38,17 +39,33 @@ func throw(s string)
 // These registers must be pre-set within the signal frame.
 //
 //go:nosplit
-func CallWithSignalFrame(stack uintptr, handlerAddr uintptr, sigframe *arch.UContext64, fpstate uintptr, sigmask *linux.SignalSet) error {
+func CallWithSignalFrame(signalStack *linux.SignalStack, handlerAddr uintptr, sigmask *linux.SignalSet, rax uint64) error {
+	var oldSigMask linux.SignalSet
 	errno := hostsyscall.RawSyscallErrno6(
 		unix.SYS_RT_SIGPROCMASK, linux.SIG_BLOCK,
 		uintptr(unsafe.Pointer(sigmask)),
-		uintptr(unsafe.Pointer(&sigframe.Sigset)),
+		uintptr(unsafe.Pointer(&oldSigMask)),
 		linux.SignalSetSize,
 		0, 0)
 	if errno != 0 {
 		return errno
 	}
-	callWithSignalFrame(stack, handlerAddr, sigframe, fpstate)
+	const minStackSize = uintptr(hostarch.PageSize)
+	ctxOffset := (unsafe.Sizeof(arch.UContext64{}) + 7) &^ 7
+
+	if ctxOffset+minStackSize > uintptr(signalStack.Size) {
+		return unix.ENOMEM
+	}
+
+	p := uintptr(signalStack.Addr + signalStack.Size)
+	stack := p - ctxOffset
+	sigframe := (*arch.UContext64)(unsafe.Pointer(stack))
+	sigframe.MContext.Rax = rax
+	sigframe.Stack = *signalStack
+	sigframe.Sigset = oldSigMask
+	sigframe.MContext.Fpstate = 0
+
+	callWithSignalFrame(stack, handlerAddr, sigframe)
 	return nil
 }