runtime: C.setuid/C.setgid smashes Go stack #9400
Comments
|
I vote to fix it for all supported architectures on linux. What about other Unixes? Do they also have pre-thread (e)u/gid settings? |
|
Yes, of course we are going to fix it for all systems (Austin is working on that). The question is whether the fix for the non-power64 systems is important enough to include in Go 1.4.1. Since the bug has existed since Go 1.1 and seems not to be hurting anyone, I believe the answer is no. |
|
While it was obviously idiotic of me not to consider this problem, the signal handler for SIGSETXID in glibc is quite short and has very few local variables. The main driver for stack usage will be the saved registers, which is of course quite a bit more on PPC than any other architecture. |
|
Assuming we're interpreting the results of the test I wrote correctly, even On Friday, December 19, 2014, Ian Lance Taylor <[email protected]
|
|
It's fixed by CL 1887. (But that CL mentioned #9600, which does not exist yet.) |
|
CL https://golang.org/cl/23683 mentions this issue. |
The test for #9400 relies on an assembler function that manipulates the stack pointer. Meanwile, it uses a global variable for synchronization. However, position independent code on 386 use a function call to fetch the base address for global variables. That function call in turn overwrites the Go stack. Fix that by fetching the global variable address once before the stack register manipulation. Fixes the android/386 builder. Change-Id: Ib77bd80affaa12f09d582d09d8b84a73bd021b60 Reviewed-on: https://go-review.googlesource.com/23683 Run-TryBot: Elias Naur <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: David Crawshaw <[email protected]>
|
CL https://golang.org/cl/34510 mentions this issue. |
Change-Id: I7d0bc5093943b0744d865e91517ff6292f3b2f89 Reviewed-on: https://go-review.googlesource.com/34510 Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
For #3871 we change the runtime not to register a signal handler for SIGSETXID, so that we don't overwrite libpthread's handler, which would cause C.setuid and C.setgid to hang. We now let libpthread's handler run when that signal comes in.
Unfortunately, libpthread's handler does not have SA_ONSTACK set, so when SIGSETXID does come in, it runs on Go's stack. There may well not be enough room for this, causing the OS to write beyond the bottom of Go's stack, scribbling over adjacent memory. On linux/power64, this happens reliably in the misc/cgo/test call to C.setgid (the power64 signal information pushed onto the stack is a couple kilobytes).
The fix is to introduce a new bit in the signal handler table (_SigSetStack) and then set it for signals 32 and 33 (SIGCANCEL and SIGSETXID). The signal initialization would see _SigSetStack and call sigaction to load the handler info, set SA_ONSTACK, and write the handler info back. That is, it's leaving the handler in place, just forcing it to run on the signal stack, where there will be enough room.
It's unclear to me how much of a problem this is on linux/amd64, linux/386, and linux/arm as well. I'm sure the same thing happens there, the only question is quite how large the amount of data being scribbled is.
The bug has existed since #3871 was fixed, which happened on 2012-07-27 (and then was in Go 1.1 released on 2013-05-13). Since the bug has existed for so long with no one complaining, it's probably not worth putting in a point release, but we should fix it for Go 1.5 (and we have to fix it for power64 to pass all.bash).
@ianlancetaylor @aclements
The text was updated successfully, but these errors were encountered: