From 62a1da372a2bfeb29644246ff0f61dc048bb912c Mon Sep 17 00:00:00 2001 From: Daniel Morsing Date: Thu, 27 Nov 2025 14:49:09 +0000 Subject: [PATCH] runtime: do signal stack clearing when parking Ms Clearing signal stacks during STW meant an added latency that was linear with respect to the number of Ms, parked or running. Since that number be quite high, we need an alternate scheme. Instead of clearing in the GC cycle, clear the signal stack whenever an M is parked and has the potential to stay parked until the end of the next GC cycle. This implements the wanted behavior for runtime/secret at the cost of a little bit of extra latency that will be dwarfed by the time it takes to perform a syscall. Change-Id: Ieb9618f46736c34486e17a3d6185661a98756d0d Reviewed-on: https://go-review.googlesource.com/c/go/+/724282 Auto-Submit: Daniel Morsing LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Reviewed-by: Michael Pratt --- src/runtime/mgc.go | 30 ++++++---------------------- src/runtime/proc.go | 29 +++++++++++++++++++++++++++ src/runtime/runtime2.go | 21 +++++++++++--------- src/runtime/secret.go | 36 +++++++++++++++++++++++++--------- src/runtime/secret_nosecret.go | 2 +- src/runtime/signal_unix.go | 2 +- 6 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index b8f99d1ed4..97a0ce8b98 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -839,30 +839,12 @@ func gcStart(trigger gcTrigger) { work.cpuStats.accumulateGCPauseTime(stw.stoppingCPUTime, 1) if goexperiment.RuntimeSecret { - // The world is stopped. Every M is either parked - // or in a syscall, or running some non-go code which can't run in secret mode. - // To get to a parked or a syscall state - // they have to transition through a point where we erase any - // confidential information in the registers. Making them - // handle a signal now would clobber the signal stack - // with non-confidential information. - // - // TODO(dmo): this is linear with respect to the number of Ms. - // Investigate just how long this takes and whether we can somehow - // loop over just the Ms that have secret info on their signal stack, - // or cooperatively have the Ms send signals to themselves just - // after they erase their registers, but before they enter a syscall - for mp := allm; mp != nil; mp = mp.alllink { - // even through the world is stopped, the kernel can still - // invoke our signal handlers. No confidential information can be spilled - // (because it's been erased by this time), but we can avoid - // sending additional signals by atomically inspecting this variable - if atomic.Xchg(&mp.signalSecret, 0) != 0 { - noopSignal(mp) - } - // TODO: syncronize with the signal handler to ensure that the signal - // was actually delivered. - } + // The world is stopped, which means every M is either idle, blocked + // in a syscall or this M that we are running on now. + // The blocked Ms had any secret spill on their signal stacks erased + // when they entered their respective states. Now we have to handle + // this one. + eraseSecretsSignalStk() } // Finish sweep before we start concurrent scan. diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 8027ac5bee..db4febace9 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1968,6 +1968,11 @@ func mstartm0() { //go:nosplit func mPark() { gp := getg() + // This M might stay parked through an entire GC cycle. + // Erase any leftovers on the signal stack. + if goexperiment.RuntimeSecret { + eraseSecretsSignalStk() + } notesleep(&gp.m.park) noteclear(&gp.m.park) } @@ -4635,6 +4640,30 @@ func reentersyscall(pc, sp, bp uintptr) { // but can have inconsistent g->sched, do not let GC observe it. gp.m.locks++ + // This M may have a signal stack that is dirtied with secret information + // (see package "runtime/secret"). Since it's about to go into a syscall for + // an arbitrary amount of time and the G that put the secret info there + // might have returned from secret.Do, we have to zero it out now, lest we + // break the guarantee that secrets are purged by the next GC after a return + // to secret.Do. + // + // It might be tempting to think that we only need to zero out this if we're + // not running in secret mode anymore, but that leaves an ABA problem. The G + // that put the secrets onto our signal stack may not be the one that is + // currently executing. + // + // Logically, we should erase this when we lose our P, not when we enter the + // syscall. This would avoid a zeroing in the case where the call returns + // almost immediately. Since we use this path for cgo calls as well, these + // fast "syscalls" are quite common. However, since we only erase the signal + // stack if we were delivered a signal in secret mode and considering the + // cross-thread synchronization cost for the P, it hardly seems worth it. + // + // TODO(dmo): can we encode the goid into mp.signalSecret and avoid the ABA problem? + if goexperiment.RuntimeSecret { + eraseSecretsSignalStk() + } + // Entersyscall must not call any function that might split/grow the stack. // (See details in comment above.) // Catch calls that might, by replacing the stack guard with something that diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 578458afa0..95dc834717 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -620,15 +620,18 @@ type m struct { // Fields whose offsets are not known to debuggers. - procid uint64 // for debuggers, but offset not hard-coded - gsignal *g // signal-handling g - goSigStack gsignalStack // Go-allocated signal handling stack - sigmask sigset // storage for saved signal mask - tls [tlsSlots]uintptr // thread-local storage (for x86 extern register) - mstartfn func() - curg *g // current running goroutine - caughtsig guintptr // goroutine running during fatal signal - signalSecret uint32 // whether we have secret information in our signal stack + procid uint64 // for debuggers, but offset not hard-coded + gsignal *g // signal-handling g + goSigStack gsignalStack // Go-allocated signal handling stack + sigmask sigset // storage for saved signal mask + tls [tlsSlots]uintptr // thread-local storage (for x86 extern register) + mstartfn func() + curg *g // current running goroutine + caughtsig guintptr // goroutine running during fatal signal + + // Indicates whether we've received a signal while + // running in secret mode. + signalSecret bool // p is the currently attached P for executing Go code, nil if not executing user Go code. // diff --git a/src/runtime/secret.go b/src/runtime/secret.go index 8aad63b54f..8942df7051 100644 --- a/src/runtime/secret.go +++ b/src/runtime/secret.go @@ -71,15 +71,6 @@ func addSecret(p unsafe.Pointer, size uintptr) { addspecial(p, &s.special, false) } -// send a no-op signal to an M for the purposes of -// clobbering the signal stack -// -// Use sigpreempt. If we don't have a preemption queued, this just -// turns into a no-op -func noopSignal(mp *m) { - signalM(mp, sigPreempt) -} - // secret_getStack returns the memory range of the // current goroutine's stack. // For testing only. @@ -93,6 +84,33 @@ func secret_getStack() (uintptr, uintptr) { return gp.stack.lo, gp.stack.hi } +// erase any secrets that may have been spilled onto the signal stack during +// signal handling. Must be called on g0 or inside STW to make sure we don't +// get rescheduled onto a different M. +// +//go:nosplit +func eraseSecretsSignalStk() { + mp := getg().m + if mp.signalSecret { + mp.signalSecret = false + // signal handlers get invoked atomically + // so it's fine for us to zero out the stack while a signal + // might get delivered. Worst case is we are currently running + // in secret mode and the signal spills fresh secret info onto + // the stack, but since we haven't returned from the secret.Do + // yet, we make no guarantees about that information. + // + // It might be tempting to only erase the part of the signal + // stack that has the context, but when running with forwarded + // signals, they might pull arbitrary data out of the context and + // store it elsewhere on the stack. We can't stop them from storing + // the data in arbitrary places, but we can erase the stack where + // they are likely to put it in cases of a register spill. + size := mp.gsignal.stack.hi - mp.gsignal.stack.lo + memclrNoHeapPointers(unsafe.Pointer(mp.gsignal.stack.lo), size) + } +} + // return a slice of all Ms signal stacks // For testing only. // diff --git a/src/runtime/secret_nosecret.go b/src/runtime/secret_nosecret.go index 0692d6bf70..581a092948 100644 --- a/src/runtime/secret_nosecret.go +++ b/src/runtime/secret_nosecret.go @@ -27,4 +27,4 @@ func addSecret(p unsafe.Pointer, size uintptr) {} //go:linkname secret_getStack runtime/secret.getStack func secret_getStack() (uintptr, uintptr) { return 0, 0 } -func noopSignal(mp *m) {} +func eraseSecretsSignalStk() {} diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index f352cb3c02..5269e2aa7f 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -491,7 +491,7 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) { sighandler(sig, info, ctx, gp) if goexperiment.RuntimeSecret && gp.secret > 0 { - atomic.Store(&gp.m.signalSecret, 1) + gp.m.signalSecret = true } setg(gp)