Lock OS threads when exec'ing with Pdeathsig

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
https://github.com/golang/go/issues/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>
This commit is contained in:
Cory Snider 2022-09-28 13:33:53 -04:00
parent c3a6de9ec8
commit 1f22b15030
6 changed files with 90 additions and 12 deletions

View file

@ -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)

View file

@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"path/filepath"
"runtime"
"strconv"
"strings"
"time"
@ -200,18 +201,34 @@ func (r *remote) startContainerd() error {
cmd.Env = append(cmd.Env, e)
}
}
if err := cmd.Start(); err != nil {
return err
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{})
go func() {
// 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

View file

@ -6,6 +6,7 @@ import (
"net"
"os"
"os/exec"
"runtime"
"strconv"
"syscall"
"time"
@ -37,9 +38,10 @@ 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
}
@ -47,6 +49,7 @@ func newProxyCommand(proto string, hostIP net.IP, hostPort int, containerIP net.
// proxies as separate processes.
type proxyCommand struct {
cmd *exec.Cmd
wait chan error
}
func (p *proxyCommand) Start() error {
@ -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()
@ -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
}

View file

@ -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)
@ -188,15 +194,27 @@ func invokePack(srcPath string, options *archive.TarOptions, root string) (io.Re
return nil, errors.Wrap(err, "error getting options pipe for tar process")
}
if err := cmd.Start(); err != nil {
return nil, errors.Wrap(err, "tar error on re-exec cmd")
}
started := make(chan error)
go func() {
// 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 subprocess is not
// prematurely signaled. See https://go.dev/issue/27505 for more
// information.
runtime.LockOSThread()
defer runtime.UnlockOSThread()
if err := cmd.Start(); err != nil {
started <- err
return
}
close(started)
err := cmd.Wait()
err = errors.Wrapf(err, "error processing tar file: %s", errBuff)
tarW.CloseWithError(err)
}()
if err := <-started; err != nil {
return nil, errors.Wrap(err, "tar error on re-exec cmd")
}
if err := json.NewEncoder(stdin).Encode(options); err != nil {
stdin.Close()

View file

@ -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)
}

View file

@ -17,6 +17,11 @@ func Self() string {
// SysProcAttr.Pdeathsig to SIGTERM.
// This will use the in-memory version (/proc/self/exe) of the current binary,
// it is thus safe to delete or replace the on-disk binary (os.Args[0]).
//
// As SysProcAttr.Pdeathsig is set, the signal will be sent to the process when
// the OS thread which created the process dies. It is the caller's
// responsibility to ensure that the creating thread is not terminated
// prematurely. See https://go.dev/issue/27505 for more details.
func Command(args ...string) *exec.Cmd {
return &exec.Cmd{
Path: Self(),