syscall: relocate linux death signal code

Fix bug on Linux SysProcAttr handling: setting both Pdeathsig and
Credential caused Pdeathsig to be ignored. This is because the kernel
clears the deathsignal field when performing a setuid/setgid
system call.

Avoid this by moving Pdeathsig handling after Credential handling.

Fixes #9686

Change-Id: Id01896ad4e979b8c448e0061f00aa8762ca0ac94
Reviewed-on: https://go-review.googlesource.com/3290
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/syscall/exec_linux.go

@@ -132,26 +132,6 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 		}
 	}
 
-	// Parent death signal
-	if sys.Pdeathsig != 0 {
-		_, _, err1 = RawSyscall6(SYS_PRCTL, PR_SET_PDEATHSIG, uintptr(sys.Pdeathsig), 0, 0, 0, 0)
-		if err1 != 0 {
-			goto childerror
-		}
-
-		// Signal self if parent is already dead. This might cause a
-		// duplicate signal in rare cases, but it won't matter when
-		// using SIGKILL.
-		r1, _, _ = RawSyscall(SYS_GETPPID, 0, 0, 0)
-		if r1 != ppid {
-			pid, _, _ := RawSyscall(SYS_GETPID, 0, 0, 0)
-			_, _, err1 := RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
-			if err1 != 0 {
-				goto childerror
-			}
-		}
-	}
-
 	// Enable tracing if requested.
 	if sys.Ptrace {
 		_, _, err1 = RawSyscall(SYS_PTRACE, uintptr(PTRACE_TRACEME), 0, 0)
@@ -232,6 +212,26 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
 		}
 	}
 
+	// Parent death signal
+	if sys.Pdeathsig != 0 {
+		_, _, err1 = RawSyscall6(SYS_PRCTL, PR_SET_PDEATHSIG, uintptr(sys.Pdeathsig), 0, 0, 0, 0)
+		if err1 != 0 {
+			goto childerror
+		}
+
+		// Signal self if parent is already dead. This might cause a
+		// duplicate signal in rare cases, but it won't matter when
+		// using SIGKILL.
+		r1, _, _ = RawSyscall(SYS_GETPPID, 0, 0, 0)
+		if r1 != ppid {
+			pid, _, _ := RawSyscall(SYS_GETPID, 0, 0, 0)
+			_, _, err1 := RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
+			if err1 != 0 {
+				goto childerror
+			}
+		}
+	}
+
 	// Pass 1: look for fd[i] < i and move those up above len(fd)
 	// so that pass 2 won't stomp on an fd it needs later.
 	if pipe < nextfd {

src/syscall/syscall_linux_test.go

@@ -0,0 +1,140 @@
+// Copyright 2015 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package syscall_test
+
+import (
+	"bufio"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"os/signal"
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+)
+
+func TestMain(m *testing.M) {
+	if os.Getenv("GO_DEATHSIG_PARENT") == "1" {
+		deathSignalParent()
+	} else if os.Getenv("GO_DEATHSIG_CHILD") == "1" {
+		deathSignalChild()
+	}
+
+	os.Exit(m.Run())
+}
+
+func TestLinuxDeathSignal(t *testing.T) {
+	if os.Getuid() != 0 {
+		t.Skip("skipping root only test")
+	}
+
+	// Copy the test binary to a location that a non-root user can read/execute
+	// after we drop privileges
+	tempDir, err := ioutil.TempDir("", "TestDeathSignal")
+	if err != nil {
+		t.Fatalf("cannot create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+	os.Chmod(tempDir, 0755)
+
+	tmpBinary := filepath.Join(tempDir, filepath.Base(os.Args[0]))
+
+	src, err := os.Open(os.Args[0])
+	if err != nil {
+		t.Fatalf("cannot open binary %q, %v", os.Args[0], err)
+	}
+	defer src.Close()
+
+	dst, err := os.OpenFile(tmpBinary, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0755)
+	if err != nil {
+		t.Fatalf("cannot create temporary binary %q, %v", tmpBinary, err)
+	}
+	if _, err := io.Copy(dst, src); err != nil {
+		t.Fatalf("failed to copy test binary to %q, %v", tmpBinary, err)
+	}
+	err = dst.Close()
+	if err != nil {
+		t.Fatalf("failed to close test binary %q, %v", tmpBinary, err)
+	}
+
+	cmd := exec.Command(tmpBinary)
+	cmd.Env = []string{"GO_DEATHSIG_PARENT=1"}
+	chldStdin, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatal("failed to create new stdin pipe: %v", err)
+	}
+	chldStdout, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatal("failed to create new stdout pipe: %v", err)
+	}
+	cmd.Stderr = os.Stderr
+
+	err = cmd.Start()
+	defer cmd.Wait()
+	if err != nil {
+		t.Fatalf("failed to start first child process: %v", err)
+	}
+
+	chldPipe := bufio.NewReader(chldStdout)
+
+	if got, err := chldPipe.ReadString('\n'); got == "start\n" {
+		syscall.Kill(cmd.Process.Pid, syscall.SIGTERM)
+
+		go func() {
+			time.Sleep(5 * time.Second)
+			chldStdin.Close()
+		}()
+
+		want := "ok\n"
+		if got, err = chldPipe.ReadString('\n'); got != want {
+			t.Fatalf("expected %q, received %q, %v", want, got, err)
+		}
+	} else {
+		t.Fatalf("did not receive start from child, received %q, %v", got, err)
+	}
+}
+
+func deathSignalParent() {
+	cmd := exec.Command(os.Args[0])
+	cmd.Env = []string{"GO_DEATHSIG_CHILD=1"}
+	cmd.Stdin = os.Stdin
+	cmd.Stdout = os.Stdout
+	attrs := syscall.SysProcAttr{
+		Pdeathsig: syscall.SIGUSR1,
+		// UID/GID 99 is the user/group "nobody" on RHEL/Fedora and is
+		// unused on Ubuntu
+		Credential: &syscall.Credential{Uid: 99, Gid: 99},
+	}
+	cmd.SysProcAttr = &attrs
+
+	err := cmd.Start()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "death signal parent error: %v\n")
+		os.Exit(1)
+	}
+	cmd.Wait()
+	os.Exit(0)
+}
+
+func deathSignalChild() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, syscall.SIGUSR1)
+	go func() {
+		<-c
+		fmt.Println("ok")
+		os.Exit(0)
+	}()
+	fmt.Println("start")
+
+	buf := make([]byte, 32)
+	os.Stdin.Read(buf)
+
+	// We expected to be signaled before stdin closed
+	fmt.Println("not ok")
+	os.Exit(1)
+}