Skip to content

Commit 91139b8

Browse files
chipacaianlancetaylor
authored andcommitted
runtime, syscall: workaround for bug in Linux's execve
Linux's execve has (at the time of writing, and since v2.6.30) a bug when it ran concurrently with clone, in that it would fail to set up some datastructures if the thread count before and after some steps differed. This is described better and in more detail by Colin King in Launchpad¹ and kernel² bugs. When a program written in Go runtime.Exec's a setuid binary, this issue may cause the resulting process to not have the expected uid. This patch works around the issue by using a mutex to serialize exec and clone. 1. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1672819 2. https://bugzilla.kernel.org/show_bug.cgi?id=195453 Fixes #19546 Change-Id: I126e87d1d9ce3be5ea4ec9c7ffe13f92e087903d Reviewed-on: https://go-review.googlesource.com/43713 Reviewed-by: Ian Lance Taylor <[email protected]> Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 3ca8ee1 commit 91139b8

File tree

2 files changed

+28
-0
lines changed

2 files changed

+28
-0
lines changed

src/runtime/proc.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1615,6 +1615,12 @@ func unlockextra(mp *m) {
16151615
atomic.Storeuintptr(&extram, uintptr(unsafe.Pointer(mp)))
16161616
}
16171617

1618+
// execLock serializes exec and clone to avoid bugs or unspecified behaviour
1619+
// around exec'ing while creating/destroying threads. See issue #19546.
1620+
//
1621+
// TODO: look into using a rwmutex, to avoid serializing thread creation.
1622+
var execLock mutex
1623+
16181624
// Create a new m. It will start off with a call to fn, or else the scheduler.
16191625
// fn needs to be static and not a heap allocated closure.
16201626
// May run with m.p==nil, so write barriers are not allowed.
@@ -1634,10 +1640,14 @@ func newm(fn func(), _p_ *p) {
16341640
if msanenabled {
16351641
msanwrite(unsafe.Pointer(&ts), unsafe.Sizeof(ts))
16361642
}
1643+
lock(&execLock)
16371644
asmcgocall(_cgo_thread_start, unsafe.Pointer(&ts))
1645+
unlock(&execLock)
16381646
return
16391647
}
1648+
lock(&execLock)
16401649
newosproc(mp, unsafe.Pointer(mp.g0.stack.hi))
1650+
unlock(&execLock)
16411651
}
16421652

16431653
// Stops execution of the current m until new work is available.
@@ -2857,6 +2867,18 @@ func syscall_runtime_AfterForkInChild() {
28572867
msigrestore(getg().m.sigmask)
28582868
}
28592869

2870+
// Called from syscall package before Exec.
2871+
//go:linkname syscall_runtime_BeforeExec syscall.runtime_BeforeExec
2872+
func syscall_runtime_BeforeExec() {
2873+
lock(&execLock)
2874+
}
2875+
2876+
// Called from syscall package after Exec.
2877+
//go:linkname syscall_runtime_AfterExec syscall.runtime_AfterExec
2878+
func syscall_runtime_AfterExec() {
2879+
unlock(&execLock)
2880+
}
2881+
28602882
// Allocate a new g, with a stack big enough for stacksize bytes.
28612883
func malg(stacksize int32) *g {
28622884
newg := new(g)

src/syscall/exec_unix.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
242242
return pid, 0, err
243243
}
244244

245+
// Implemented in runtime package.
246+
func runtime_BeforeExec()
247+
func runtime_AfterExec()
248+
245249
// Exec invokes the execve(2) system call.
246250
func Exec(argv0 string, argv []string, envv []string) (err error) {
247251
argv0p, err := BytePtrFromString(argv0)
@@ -256,9 +260,11 @@ func Exec(argv0 string, argv []string, envv []string) (err error) {
256260
if err != nil {
257261
return err
258262
}
263+
runtime_BeforeExec()
259264
_, _, err1 := RawSyscall(SYS_EXECVE,
260265
uintptr(unsafe.Pointer(argv0p)),
261266
uintptr(unsafe.Pointer(&argvp[0])),
262267
uintptr(unsafe.Pointer(&envvp[0])))
268+
runtime_AfterExec()
263269
return Errno(err1)
264270
}

0 commit comments

Comments
 (0)