Skip to content

Commit

Permalink
Lock OS threads when exec'ing with Pdeathsig
Browse files Browse the repository at this point in the history
On Linux, when (os/exec.Cmd).SysProcAttr.Pdeathsig is set, the signal
will be sent to the process when the OS thread on which cmd.Start() was
executed dies. The runtime terminates an OS thread when a goroutine
exits after being wired to the thread with runtime.LockOSThread(). If
other goroutines are allowed to be scheduled onto a thread which called
cmd.Start(), an unrelated goroutine could cause the thread to be
terminated and prematurely signal the command. See
golang/go#27505 for more information.

Prevent started subprocesses with Pdeathsig from getting signaled
prematurely by wiring the starting goroutine to the OS thread until the
subprocess has exited. No other goroutines can be scheduled onto a
locked thread so it will remain alive until unlocked or the daemon
process exits.

Signed-off-by: Cory Snider <csnider@mirantis.com>
  • Loading branch information
corhere committed Sep 28, 2022
1 parent c3a6de9 commit 65cdbad
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 8 deletions.
7 changes: 7 additions & 0 deletions daemon/graphdriver/overlay2/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ func mountFrom(dir, device, target, mType string, flags uintptr, label string) e
output := bytes.NewBuffer(nil)
cmd.Stdout = output
cmd.Stderr = output

// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
// causes the started process to be signaled when the creating OS thread
// dies. Ensure that the reexec is not prematurely signaled. See
// https://go.dev/issue/27505 for more information.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if err := cmd.Start(); err != nil {
w.Close()
return fmt.Errorf("mountfrom error on re-exec cmd: %v", err)
Expand Down
25 changes: 21 additions & 4 deletions libcontainerd/supervisor/remote_daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -200,18 +201,34 @@ func (r *remote) startContainerd() error {
cmd.Env = append(cmd.Env, e)
}
}
if err := cmd.Start(); err != nil {
return err
}

r.daemonWaitCh = make(chan struct{})
startedCh := make(chan error)
go func() {
// On Linux, when cmd.SysProcAttr.Pdeathsig is set,
// the signal is sent to the subprocess when the creating thread
// terminates. The runtime terminates a thread if a goroutine
// exits while locked to it. Prevent the containerd process
// from getting killed prematurely by ensuring that the thread
// used to start it remains alive until it or the daemon process
// exits. See https://go.dev/issue/27505 for more details.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
err := cmd.Start()
startedCh <- err
if err != nil {
return
}

r.daemonWaitCh = make(chan struct{})
// Reap our child when needed
if err := cmd.Wait(); err != nil {
r.logger.WithError(err).Errorf("containerd did not exit successfully")
}
close(r.daemonWaitCh)
}()
if err := <-startedCh; err != nil {
return err
}

r.daemonPid = cmd.Process.Pid

Expand Down
33 changes: 29 additions & 4 deletions libnetwork/portmapper/proxy_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net"
"os"
"os/exec"
"runtime"
"strconv"
"syscall"
"time"
Expand Down Expand Up @@ -37,16 +38,18 @@ func newProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
Path: path,
Args: args,
SysProcAttr: &syscall.SysProcAttr{
Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the daemon process dies
Pdeathsig: syscall.SIGTERM, // send a sigterm to the proxy if the creating thread in the daemon process dies (https://go.dev/issue/27505)
},
},
wait: make(chan error, 1),
}, nil
}

// proxyCommand wraps an exec.Cmd to run the userland TCP and UDP
// proxies as separate processes.
type proxyCommand struct {
cmd *exec.Cmd
cmd *exec.Cmd
wait chan error
}

func (p *proxyCommand) Start() error {
Expand All @@ -56,7 +59,29 @@ func (p *proxyCommand) Start() error {
}
defer r.Close()
p.cmd.ExtraFiles = []*os.File{w}
if err := p.cmd.Start(); err != nil {

// As p.cmd.SysProcAttr.Pdeathsig is set, the signal will be sent to the
// process when the OS thread on which p.cmd.Start() was executed dies.
// If the thread is allowed to be released back into the goroutine
// thread pool, the thread could get terminated at any time if a
// goroutine gets scheduled onto it which calls runtime.LockOSThread()
// and exits without a matching number of runtime.UnlockOSThread()
// calls. Ensure that the thread from which Start() is called stays
// alive until the proxy or the daemon process exits to prevent the
// proxy from getting terminated early. See https://go.dev/issue/27505
// for more details.
started := make(chan error)
go func() {
runtime.LockOSThread()
defer runtime.UnlockOSThread()
err := p.cmd.Start()
started <- err
if err != nil {
return
}
p.wait <- p.cmd.Wait()
}()
if err := <-started; err != nil {
return err
}
w.Close()
Expand Down Expand Up @@ -92,7 +117,7 @@ func (p *proxyCommand) Stop() error {
if err := p.cmd.Process.Signal(os.Interrupt); err != nil {
return err
}
return p.cmd.Wait()
return <-p.wait
}
return nil
}
12 changes: 12 additions & 0 deletions pkg/chrootarchive/archive_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T
cmd.Stdout = output
cmd.Stderr = output

// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
// causes the started process to be signaled when the creating OS thread
// dies. Ensure that the reexec is not prematurely signaled. See
// https://go.dev/issue/27505 for more information.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if err := cmd.Start(); err != nil {
w.Close()
return fmt.Errorf("Untar error on re-exec cmd: %v", err)
Expand Down Expand Up @@ -188,6 +194,12 @@ func invokePack(srcPath string, options *archive.TarOptions, root string) (io.Re
return nil, errors.Wrap(err, "error getting options pipe for tar process")
}

// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
// causes the started process to be signaled when the creating OS thread
// dies. Ensure that the reexec is not prematurely signaled. See
// https://go.dev/issue/27505 for more information.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if err := cmd.Start(); err != nil {
return nil, errors.Wrap(err, "tar error on re-exec cmd")
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/chrootarchive/diff_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ func applyLayerHandler(dest string, layer io.Reader, options *archive.TarOptions
outBuf, errBuf := new(bytes.Buffer), new(bytes.Buffer)
cmd.Stdout, cmd.Stderr = outBuf, errBuf

// reexec.Command() sets cmd.SysProcAttr.Pdeathsig on Linux, which
// causes the started process to be signaled when the creating OS thread
// dies. Ensure that the reexec is not prematurely signaled. See
// https://go.dev/issue/27505 for more information.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if err = cmd.Run(); err != nil {
return 0, fmt.Errorf("ApplyLayer %s stdout: %s stderr: %s", err, outBuf, errBuf)
}
Expand Down

0 comments on commit 65cdbad

Please sign in to comment.