]> Cypherpunks repositories - gostls13.git/commitdiff
runtime, syscall: workaround for bug in Linux's execve
authorJohn R. Lenton <jlenton@gmail.com>
Sat, 20 May 2017 16:22:36 +0000 (17:22 +0100)
committerIan Lance Taylor <iant@golang.org>
Tue, 20 Jun 2017 23:53:17 +0000 (23:53 +0000)
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 <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/runtime/proc.go
src/syscall/exec_unix.go

index f6e07f8ec0c3e7d4fbbe96125e66ff8b07bf3e0a..b360f20ee776075410ce383401438540211d53ff 100644 (file)
@@ -1615,6 +1615,12 @@ func unlockextra(mp *m) {
        atomic.Storeuintptr(&extram, uintptr(unsafe.Pointer(mp)))
 }
 
+// execLock serializes exec and clone to avoid bugs or unspecified behaviour
+// around exec'ing while creating/destroying threads.  See issue #19546.
+//
+// TODO: look into using a rwmutex, to avoid serializing thread creation.
+var execLock mutex
+
 // Create a new m. It will start off with a call to fn, or else the scheduler.
 // fn needs to be static and not a heap allocated closure.
 // May run with m.p==nil, so write barriers are not allowed.
@@ -1634,10 +1640,14 @@ func newm(fn func(), _p_ *p) {
                if msanenabled {
                        msanwrite(unsafe.Pointer(&ts), unsafe.Sizeof(ts))
                }
+               lock(&execLock)
                asmcgocall(_cgo_thread_start, unsafe.Pointer(&ts))
+               unlock(&execLock)
                return
        }
+       lock(&execLock)
        newosproc(mp, unsafe.Pointer(mp.g0.stack.hi))
+       unlock(&execLock)
 }
 
 // Stops execution of the current m until new work is available.
@@ -2857,6 +2867,18 @@ func syscall_runtime_AfterForkInChild() {
        msigrestore(getg().m.sigmask)
 }
 
+// Called from syscall package before Exec.
+//go:linkname syscall_runtime_BeforeExec syscall.runtime_BeforeExec
+func syscall_runtime_BeforeExec() {
+       lock(&execLock)
+}
+
+// Called from syscall package after Exec.
+//go:linkname syscall_runtime_AfterExec syscall.runtime_AfterExec
+func syscall_runtime_AfterExec() {
+       unlock(&execLock)
+}
+
 // Allocate a new g, with a stack big enough for stacksize bytes.
 func malg(stacksize int32) *g {
        newg := new(g)
index e4f047f3f4e6b076fe10899a8cbbc22ca2380f8d..e8ca29ab40389f318e2e9bd5d79c6931682ddd05 100644 (file)
@@ -242,6 +242,10 @@ func StartProcess(argv0 string, argv []string, attr *ProcAttr) (pid int, handle
        return pid, 0, err
 }
 
+// Implemented in runtime package.
+func runtime_BeforeExec()
+func runtime_AfterExec()
+
 // Exec invokes the execve(2) system call.
 func Exec(argv0 string, argv []string, envv []string) (err error) {
        argv0p, err := BytePtrFromString(argv0)
@@ -256,9 +260,11 @@ func Exec(argv0 string, argv []string, envv []string) (err error) {
        if err != nil {
                return err
        }
+       runtime_BeforeExec()
        _, _, err1 := RawSyscall(SYS_EXECVE,
                uintptr(unsafe.Pointer(argv0p)),
                uintptr(unsafe.Pointer(&argvp[0])),
                uintptr(unsafe.Pointer(&envvp[0])))
+       runtime_AfterExec()
        return Errno(err1)
 }