runtime: crash with C sigaltstacks

[aclements]

What version of Go are you using (go version)?

go version devel +0c14345c96 Wed Nov 29 16:54:25 2017 +0000 linux/amd64

Does this issue reproduce with the latest release?

No. This started with commit 44d9e96.

What operating system and processor architecture are you using (go env)?

linux/amd64

What did you do?

https://play.golang.org/p/JkOKEJ9kQk (not runnable on the playground)

What did you expect to see?

Successful exit with no output.

What did you see instead?

Segfault.

The problem is that the sigaltstack syscall is failing (which we handle unceremoniously with a segfault) because we're passing stack bounds (0, 0). The happens because I incorrectly made minit/unminit assume they would each only be called once per M. This is true for regular Go threads, but not true for calls from C threads into Go, or signals delivered on C threads.

Here's a specific way this can happen:

1. oneNewExtraM creates an M with a Go-allocated gsignal stack and put it on the extram list.
2. A C-created thread with a C-created alternate signal stack calls a Go function. (Or a signal handled by the runtime arrives on such a thread.)
   1. cgocallback_gofunc (or badsignal) calls needm. needm gets the extra M created above off the list and calls minit -> minitSignals -> minitSignalStack.
   2. minitSignalStack sees that a signal stack is already set for this thread and sets m.newSigstack = false.
   3. The Go signal handler returns, which calls dropm -> unminit -> unminitSignals.
   4. unminitSignals sees that m.newSigstack is false, so it sets m.gsignal.stack = stack{}
   5. dropm returns the M to the extram list.
3. A C-created thread without an alternate signal stack calls a Go function (or gets a signal)
   1. needm gets the same M from the extra M list. But this time minitSignalStack sees that there is no signal stack for the current thread, so it calls signalstack(&_g_.m.gsignal.stack).
   2. Since we cleared m.gsignal.stack in step 2.4 above, the syscall fails and we crash.

[aclements]

I think this same problem has existed in an even worse form for many releases. Prior to 44d9e96, we didn't clear m.gsignal.stack, so it didn't crash when it tried to use that gsignal stack. Instead, step 3(i) would reuse the signal stack from the C thread in step 2, meaning that both C-created threads could potentially be running with the same signal stack.

It seems like we must track the Go-allocated gsignal stack and the "active" signal stack separately so that when we use an extra M we can re-instate the Go-allocated gsignal stack when we dropm.

/cc @ianlancetaylor

[ianlancetaylor]

Yikes, I think you're right. When the signal handler changes signal stacks it calls restoreGsignalStack, but nothing does that when needm calls minit.

[gopherbot]

Change https://golang.org/cl/81476 mentions this issue: runtime: restore the Go-allocated signal stack in unminit

[aclements]

Re-opening for 1.9. I have a cherry-pick locally that resolves the conflicts if it's helpful.

[gopherbot]

Change https://golang.org/cl/81615 mentions this issue: runtime: use MAP_ANON in sigstack check

[ianlancetaylor]

Really reopening.