From 108410638aa8480e82933870ba8279133f543d2b Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Wed, 14 Apr 2021 14:12:08 -0700 Subject: Use assembly stub to take the address of assembly functions Go 1.17 is adding a new register-based calling convention [1] ("ABIInternal"), which used is when calling between Go functions. Assembly functions are still written using the old ABI ("ABI0"). That is, they still accept arguments on the stack, and pass arguments to other functions on the stack. The call rules look approximately like this: 1. Direct call from Go function to Go function: compiler emits direct ABIInternal call. 2. Indirect call from Go function to Go function: compiler emits indirect ABIInternal call. 3. Direct call from Go function to assembly function: compiler emits direct ABI0 call. 4. Indirect call from Go function to assembly function: compiler emits indirect ABIInternal call to ABI conversion wrapper function. 5. Direct or indirect call from assembly function to assembly function: assembly/linker emits call to original ABI0 function. 6. Direct or indirect call from assembly function to Go function: assembly/linker emits ABI0 call to ABI conversion wrapper function. Case 4 is the interesting one here. Since the compiler can't know the ABI of an indirect call, all indirect calls are made with ABIInternal. In order to support indirect ABI0 assembly function calls, a wrapper is generated that translates ABIInternal arguments to ABI0 arguments, calls the target function, and then converts results back. When the address of an ABI0 function is taken from Go code, it evaluates to the address of this wrapper function rather than the target function so that later indirect calls will work as expected. This is normally fine, but gVisor does more than just call some of the assembly functions we take the address of: either noting the start and end address for future reference from a signal handler (safecopy), or copying the function text to a new mapping (platforms). Both of these fail with wrappers enabled (currently, this is Go tip with GOEXPERIMENT=regabiwrappers) because these operations end up operating on the wrapper instead of the target function. We work around this issue by taking advantage of case 5: references to assembly symbols from other assembly functions resolve directly to the desired target symbol. Thus, rather than using reflect to get the address of a Go reference to the functions, we create assembly stubs that return the address of the function. This approach works just as well on current versions of Go, so the change can be made immediately and doesn't require any build tags. [1] https://go.googlesource.com/go/+/refs/heads/master/src/cmd/compile/abi-internal.md PiperOrigin-RevId: 368505655 --- pkg/safecopy/atomic_amd64.s | 24 ++++++++++++++++++++++++ pkg/safecopy/atomic_arm64.s | 24 ++++++++++++++++++++++++ pkg/safecopy/memclr_amd64.s | 6 ++++++ pkg/safecopy/memclr_arm64.s | 6 ++++++ pkg/safecopy/memcpy_amd64.s | 6 ++++++ pkg/safecopy/memcpy_arm64.s | 6 ++++++ pkg/safecopy/safecopy.go | 22 +++++++++++++--------- pkg/safecopy/safecopy_unsafe.go | 12 ++++++++++++ pkg/safecopy/sighandler_amd64.s | 6 ++++++ pkg/safecopy/sighandler_arm64.s | 6 ++++++ pkg/sentry/platform/kvm/bluepill.go | 13 ++++++++++--- pkg/sentry/platform/kvm/bluepill_amd64.s | 12 ++++++++++++ pkg/sentry/platform/kvm/bluepill_arm64.s | 12 ++++++++++++ pkg/sentry/platform/ptrace/stub_amd64.s | 6 ++++++ pkg/sentry/platform/ptrace/stub_arm64.s | 6 ++++++ pkg/sentry/platform/ptrace/stub_unsafe.go | 9 ++++++++- 16 files changed, 163 insertions(+), 13 deletions(-) (limited to 'pkg') diff --git a/pkg/safecopy/atomic_amd64.s b/pkg/safecopy/atomic_amd64.s index a0cd78f33..290579e53 100644 --- a/pkg/safecopy/atomic_amd64.s +++ b/pkg/safecopy/atomic_amd64.s @@ -44,6 +44,12 @@ TEXT ·swapUint32(SB), NOSPLIT, $0-24 MOVL AX, old+16(FP) RET +// func addrOfSwapUint32() uintptr +TEXT ·addrOfSwapUint32(SB), $0-8 + MOVQ $·swapUint32(SB), AX + MOVQ AX, ret+0(FP) + RET + // handleSwapUint64Fault returns the value stored in DI. Control is transferred // to it when swapUint64 below receives SIGSEGV or SIGBUS, with the signal // number stored in DI. @@ -74,6 +80,12 @@ TEXT ·swapUint64(SB), NOSPLIT, $0-28 MOVQ AX, old+16(FP) RET +// func addrOfSwapUint64() uintptr +TEXT ·addrOfSwapUint64(SB), $0-8 + MOVQ $·swapUint64(SB), AX + MOVQ AX, ret+0(FP) + RET + // handleCompareAndSwapUint32Fault returns the value stored in DI. Control is // transferred to it when swapUint64 below receives SIGSEGV or SIGBUS, with the // signal number stored in DI. @@ -107,6 +119,12 @@ TEXT ·compareAndSwapUint32(SB), NOSPLIT, $0-24 MOVL AX, prev+16(FP) RET +// func addrOfCompareAndSwapUint32() uintptr +TEXT ·addrOfCompareAndSwapUint32(SB), $0-8 + MOVQ $·compareAndSwapUint32(SB), AX + MOVQ AX, ret+0(FP) + RET + // handleLoadUint32Fault returns the value stored in DI. Control is transferred // to it when LoadUint32 below receives SIGSEGV or SIGBUS, with the signal // number stored in DI. @@ -134,3 +152,9 @@ TEXT ·loadUint32(SB), NOSPLIT, $0-16 MOVL (AX), BX MOVL BX, val+8(FP) RET + +// func addrOfLoadUint32() uintptr +TEXT ·addrOfLoadUint32(SB), $0-8 + MOVQ $·loadUint32(SB), AX + MOVQ AX, ret+0(FP) + RET diff --git a/pkg/safecopy/atomic_arm64.s b/pkg/safecopy/atomic_arm64.s index d58ed71f7..55c031a3c 100644 --- a/pkg/safecopy/atomic_arm64.s +++ b/pkg/safecopy/atomic_arm64.s @@ -33,6 +33,12 @@ again: MOVW R2, old+16(FP) RET +// func addrOfSwapUint32() uintptr +TEXT ·addrOfSwapUint32(SB), $0-8 + MOVD $·swapUint32(SB), R0 + MOVD R0, ret+0(FP) + RET + // handleSwapUint64Fault returns the value stored in R1. Control is transferred // to it when swapUint64 below receives SIGSEGV or SIGBUS, with the signal // number stored in R1. @@ -62,6 +68,12 @@ again: MOVD R2, old+16(FP) RET +// func addrOfSwapUint64() uintptr +TEXT ·addrOfSwapUint64(SB), $0-8 + MOVD $·swapUint64(SB), R0 + MOVD R0, ret+0(FP) + RET + // handleCompareAndSwapUint32Fault returns the value stored in R1. Control is // transferred to it when compareAndSwapUint32 below receives SIGSEGV or SIGBUS, // with the signal number stored in R1. @@ -97,6 +109,12 @@ done: MOVW R3, prev+16(FP) RET +// func addrOfCompareAndSwapUint32() uintptr +TEXT ·addrOfCompareAndSwapUint32(SB), $0-8 + MOVD $·compareAndSwapUint32(SB), R0 + MOVD R0, ret+0(FP) + RET + // handleLoadUint32Fault returns the value stored in DI. Control is transferred // to it when LoadUint32 below receives SIGSEGV or SIGBUS, with the signal // number stored in DI. @@ -124,3 +142,9 @@ TEXT ·loadUint32(SB), NOSPLIT, $0-16 LDARW (R0), R1 MOVW R1, val+8(FP) RET + +// func addrOfLoadUint32() uintptr +TEXT ·addrOfLoadUint32(SB), $0-8 + MOVD $·loadUint32(SB), R0 + MOVD R0, ret+0(FP) + RET diff --git a/pkg/safecopy/memclr_amd64.s b/pkg/safecopy/memclr_amd64.s index 64cf32f05..4abaecaff 100644 --- a/pkg/safecopy/memclr_amd64.s +++ b/pkg/safecopy/memclr_amd64.s @@ -145,3 +145,9 @@ _129through256: MOVOU X0, -32(DI)(BX*1) MOVOU X0, -16(DI)(BX*1) RET + +// func addrOfMemclr() uintptr +TEXT ·addrOfMemclr(SB), $0-8 + MOVQ $·memclr(SB), AX + MOVQ AX, ret+0(FP) + RET diff --git a/pkg/safecopy/memclr_arm64.s b/pkg/safecopy/memclr_arm64.s index 7361b9067..c789bfeb3 100644 --- a/pkg/safecopy/memclr_arm64.s +++ b/pkg/safecopy/memclr_arm64.s @@ -72,3 +72,9 @@ head_loop: CMP $16, R1 BLT tail_zero B aligned_to_16 + +// func addrOfMemclr() uintptr +TEXT ·addrOfMemclr(SB), $0-8 + MOVD $·memclr(SB), R0 + MOVD R0, ret+0(FP) + RET diff --git a/pkg/safecopy/memcpy_amd64.s b/pkg/safecopy/memcpy_amd64.s index 00b46c18f..1d63ca1fd 100644 --- a/pkg/safecopy/memcpy_amd64.s +++ b/pkg/safecopy/memcpy_amd64.s @@ -217,3 +217,9 @@ move_129through256: MOVOU -16(SI)(BX*1), X15 MOVOU X15, -16(DI)(BX*1) RET + +// func addrOfMemcpy() uintptr +TEXT ·addrOfMemcpy(SB), $0-8 + MOVQ $·memcpy(SB), AX + MOVQ AX, ret+0(FP) + RET diff --git a/pkg/safecopy/memcpy_arm64.s b/pkg/safecopy/memcpy_arm64.s index e7e541565..7b3f50aa5 100644 --- a/pkg/safecopy/memcpy_arm64.s +++ b/pkg/safecopy/memcpy_arm64.s @@ -76,3 +76,9 @@ forwardtailloop: CMP R3, R9 BNE forwardtailloop RET + +// func addrOfMemcpy() uintptr +TEXT ·addrOfMemcpy(SB), $0-8 + MOVD $·memcpy(SB), R0 + MOVD R0, ret+0(FP) + RET diff --git a/pkg/safecopy/safecopy.go b/pkg/safecopy/safecopy.go index 1e0af5889..df63dd5f1 100644 --- a/pkg/safecopy/safecopy.go +++ b/pkg/safecopy/safecopy.go @@ -18,7 +18,6 @@ package safecopy import ( "fmt" - "reflect" "runtime" "golang.org/x/sys/unix" @@ -91,6 +90,11 @@ var ( // signals. func signalHandler() +// addrOfSignalHandler returns the start address of signalHandler. +// +// See comment on addrOfMemcpy for more details. +func addrOfSignalHandler() uintptr + // FindEndAddress returns the end address (one byte beyond the last) of the // function that contains the specified address (begin). func FindEndAddress(begin uintptr) uintptr { @@ -111,26 +115,26 @@ func initializeAddresses() { // The following functions are written in assembly language, so they won't // be inlined by the existing compiler/linker. Tests will fail if this // assumption is violated. - memcpyBegin = reflect.ValueOf(memcpy).Pointer() + memcpyBegin = addrOfMemcpy() memcpyEnd = FindEndAddress(memcpyBegin) - memclrBegin = reflect.ValueOf(memclr).Pointer() + memclrBegin = addrOfMemclr() memclrEnd = FindEndAddress(memclrBegin) - swapUint32Begin = reflect.ValueOf(swapUint32).Pointer() + swapUint32Begin = addrOfSwapUint32() swapUint32End = FindEndAddress(swapUint32Begin) - swapUint64Begin = reflect.ValueOf(swapUint64).Pointer() + swapUint64Begin = addrOfSwapUint64() swapUint64End = FindEndAddress(swapUint64Begin) - compareAndSwapUint32Begin = reflect.ValueOf(compareAndSwapUint32).Pointer() + compareAndSwapUint32Begin = addrOfCompareAndSwapUint32() compareAndSwapUint32End = FindEndAddress(compareAndSwapUint32Begin) - loadUint32Begin = reflect.ValueOf(loadUint32).Pointer() + loadUint32Begin = addrOfLoadUint32() loadUint32End = FindEndAddress(loadUint32Begin) } func init() { initializeAddresses() - if err := ReplaceSignalHandler(unix.SIGSEGV, reflect.ValueOf(signalHandler).Pointer(), &savedSigSegVHandler); err != nil { + if err := ReplaceSignalHandler(unix.SIGSEGV, addrOfSignalHandler(), &savedSigSegVHandler); err != nil { panic(fmt.Sprintf("Unable to set handler for SIGSEGV: %v", err)) } - if err := ReplaceSignalHandler(unix.SIGBUS, reflect.ValueOf(signalHandler).Pointer(), &savedSigBusHandler); err != nil { + if err := ReplaceSignalHandler(unix.SIGBUS, addrOfSignalHandler(), &savedSigBusHandler); err != nil { panic(fmt.Sprintf("Unable to set handler for SIGBUS: %v", err)) } syserror.AddErrorUnwrapper(func(e error) (unix.Errno, bool) { diff --git a/pkg/safecopy/safecopy_unsafe.go b/pkg/safecopy/safecopy_unsafe.go index a075cf88e..efbc2ddc1 100644 --- a/pkg/safecopy/safecopy_unsafe.go +++ b/pkg/safecopy/safecopy_unsafe.go @@ -89,6 +89,18 @@ func compareAndSwapUint32(ptr unsafe.Pointer, old, new uint32) (prev uint32, sig //go:noescape func loadUint32(ptr unsafe.Pointer) (val uint32, sig int32) +// Return the start address of the functions above. +// +// In Go 1.17+, Go references to assembly functions resolve to an ABIInternal +// wrapper function rather than the function itself. We must reference from +// assembly to get the ABI0 (i.e., primary) address. +func addrOfMemcpy() uintptr +func addrOfMemclr() uintptr +func addrOfSwapUint32() uintptr +func addrOfSwapUint64() uintptr +func addrOfCompareAndSwapUint32() uintptr +func addrOfLoadUint32() uintptr + // CopyIn copies len(dst) bytes from src to dst. It returns the number of bytes // copied and an error if SIGSEGV or SIGBUS is received while reading from src. func CopyIn(dst []byte, src unsafe.Pointer) (int, error) { diff --git a/pkg/safecopy/sighandler_amd64.s b/pkg/safecopy/sighandler_amd64.s index 475ae48e9..0b5e8df66 100644 --- a/pkg/safecopy/sighandler_amd64.s +++ b/pkg/safecopy/sighandler_amd64.s @@ -131,3 +131,9 @@ handle_fault: MOVL DI, REG_RDI(DX) RET + +// func addrOfSignalHandler() uintptr +TEXT ·addrOfSignalHandler(SB), $0-8 + MOVQ $·signalHandler(SB), AX + MOVQ AX, ret+0(FP) + RET diff --git a/pkg/safecopy/sighandler_arm64.s b/pkg/safecopy/sighandler_arm64.s index 53e4ac2c1..41ed70ff9 100644 --- a/pkg/safecopy/sighandler_arm64.s +++ b/pkg/safecopy/sighandler_arm64.s @@ -141,3 +141,9 @@ handle_fault: MOVW R0, REG_R1(R2) RET + +// func addrOfSignalHandler() uintptr +TEXT ·addrOfSignalHandler(SB), $0-8 + MOVD $·signalHandler(SB), R0 + MOVD R0, ret+0(FP) + RET diff --git a/pkg/sentry/platform/kvm/bluepill.go b/pkg/sentry/platform/kvm/bluepill.go index fd1131638..bb9967b9f 100644 --- a/pkg/sentry/platform/kvm/bluepill.go +++ b/pkg/sentry/platform/kvm/bluepill.go @@ -16,7 +16,6 @@ package kvm import ( "fmt" - "reflect" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/ring0" @@ -36,6 +35,14 @@ func sighandler() // dieArchSetup and the assembly implementation for dieTrampoline. func dieTrampoline() +// Return the start address of the functions above. +// +// In Go 1.17+, Go references to assembly functions resolve to an ABIInternal +// wrapper function rather than the function itself. We must reference from +// assembly to get the ABI0 (i.e., primary) address. +func addrOfSighandler() uintptr +func addrOfDieTrampoline() uintptr + var ( // bounceSignal is the signal used for bouncing KVM. // @@ -87,10 +94,10 @@ func (c *vCPU) die(context *arch.SignalContext64, msg string) { func init() { // Install the handler. - if err := safecopy.ReplaceSignalHandler(bluepillSignal, reflect.ValueOf(sighandler).Pointer(), &savedHandler); err != nil { + if err := safecopy.ReplaceSignalHandler(bluepillSignal, addrOfSighandler(), &savedHandler); err != nil { panic(fmt.Sprintf("Unable to set handler for signal %d: %v", bluepillSignal, err)) } // Extract the address for the trampoline. - dieTrampolineAddr = reflect.ValueOf(dieTrampoline).Pointer() + dieTrampolineAddr = addrOfDieTrampoline() } diff --git a/pkg/sentry/platform/kvm/bluepill_amd64.s b/pkg/sentry/platform/kvm/bluepill_amd64.s index 025ea93b5..953024600 100644 --- a/pkg/sentry/platform/kvm/bluepill_amd64.s +++ b/pkg/sentry/platform/kvm/bluepill_amd64.s @@ -81,8 +81,20 @@ fallback: MOVQ ·savedHandler(SB), AX JMP AX +// func addrOfSighandler() uintptr +TEXT ·addrOfSighandler(SB), $0-8 + MOVQ $·sighandler(SB), AX + MOVQ AX, ret+0(FP) + RET + // dieTrampoline: see bluepill.go, bluepill_amd64_unsafe.go for documentation. TEXT ·dieTrampoline(SB),NOSPLIT,$0 PUSHQ BX // First argument (vCPU). PUSHQ AX // Fake the old RIP as caller. JMP ·dieHandler(SB) + +// func addrOfDieTrampoline() uintptr +TEXT ·addrOfDieTrampoline(SB), $0-8 + MOVQ $·dieTrampoline(SB), AX + MOVQ AX, ret+0(FP) + RET diff --git a/pkg/sentry/platform/kvm/bluepill_arm64.s b/pkg/sentry/platform/kvm/bluepill_arm64.s index 09c7e88e5..308f2a951 100644 --- a/pkg/sentry/platform/kvm/bluepill_arm64.s +++ b/pkg/sentry/platform/kvm/bluepill_arm64.s @@ -92,6 +92,12 @@ fallback: MOVD ·savedHandler(SB), R7 B (R7) +// func addrOfSighandler() uintptr +TEXT ·addrOfSighandler(SB), $0-8 + MOVD $·sighandler(SB), R0 + MOVD R0, ret+0(FP) + RET + // dieTrampoline: see bluepill.go, bluepill_arm64_unsafe.go for documentation. TEXT ·dieTrampoline(SB),NOSPLIT,$0 // R0: Fake the old PC as caller @@ -99,3 +105,9 @@ TEXT ·dieTrampoline(SB),NOSPLIT,$0 MOVD.P R1, 8(RSP) // R1: First argument (vCPU) MOVD.P R0, 8(RSP) // R0: Fake the old PC as caller B ·dieHandler(SB) + +// func addrOfDieTrampoline() uintptr +TEXT ·addrOfDieTrampoline(SB), $0-8 + MOVD $·dieTrampoline(SB), R0 + MOVD R0, ret+0(FP) + RET diff --git a/pkg/sentry/platform/ptrace/stub_amd64.s b/pkg/sentry/platform/ptrace/stub_amd64.s index 16f9c523e..d5c3f901f 100644 --- a/pkg/sentry/platform/ptrace/stub_amd64.s +++ b/pkg/sentry/platform/ptrace/stub_amd64.s @@ -109,6 +109,12 @@ parent_dead: SYSCALL HLT +// func addrOfStub() uintptr +TEXT ·addrOfStub(SB), $0-8 + MOVQ $·stub(SB), AX + MOVQ AX, ret+0(FP) + RET + // stubCall calls the stub function at the given address with the given PPID. // // This is a distinct function because stub, above, may be mapped at any diff --git a/pkg/sentry/platform/ptrace/stub_arm64.s b/pkg/sentry/platform/ptrace/stub_arm64.s index 6162df02a..4664cd4ad 100644 --- a/pkg/sentry/platform/ptrace/stub_arm64.s +++ b/pkg/sentry/platform/ptrace/stub_arm64.s @@ -102,6 +102,12 @@ parent_dead: SVC HLT +// func addrOfStub() uintptr +TEXT ·addrOfStub(SB), $0-8 + MOVD $·stub(SB), R0 + MOVD R0, ret+0(FP) + RET + // stubCall calls the stub function at the given address with the given PPID. // // This is a distinct function because stub, above, may be mapped at any diff --git a/pkg/sentry/platform/ptrace/stub_unsafe.go b/pkg/sentry/platform/ptrace/stub_unsafe.go index 5c9b7784f..1fbdea898 100644 --- a/pkg/sentry/platform/ptrace/stub_unsafe.go +++ b/pkg/sentry/platform/ptrace/stub_unsafe.go @@ -26,6 +26,13 @@ import ( // stub is defined in arch-specific assembly. func stub() +// addrOfStub returns the start address of stub. +// +// In Go 1.17+, Go references to assembly functions resolve to an ABIInternal +// wrapper function rather than the function itself. We must reference from +// assembly to get the ABI0 (i.e., primary) address. +func addrOfStub() uintptr + // stubCall calls the stub at the given address with the given pid. func stubCall(addr, pid uintptr) @@ -41,7 +48,7 @@ func unsafeSlice(addr uintptr, length int) (slice []byte) { // stubInit initializes the stub. func stubInit() { // Grab the existing stub. - stubBegin := reflect.ValueOf(stub).Pointer() + stubBegin := addrOfStub() stubLen := int(safecopy.FindEndAddress(stubBegin) - stubBegin) stubSlice := unsafeSlice(stubBegin, stubLen) mapLen := uintptr(stubLen) -- cgit v1.2.3