From 9b379d7e04750bbf6615cdfc1783db53c3d9bdc9 Mon Sep 17 00:00:00 2001 From: Andrew Williams Date: Sun, 25 Jan 2015 12:53:34 -0600 Subject: [PATCH] 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 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/syscall/exec_linux.go | 40 ++++----- src/syscall/syscall_linux_test.go | 140 ++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+), 20 deletions(-) create mode 100644 src/syscall/syscall_linux_test.go diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go index ced2ca862d..3aa30c7364 100644 --- a/src/syscall/exec_linux.go +++ b/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 { diff --git a/src/syscall/syscall_linux_test.go b/src/syscall/syscall_linux_test.go new file mode 100644 index 0000000000..40fce6d68c --- /dev/null +++ b/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) +}