Skip to content

Commit

Permalink
Safeguard pdeathsig usage in shim w/ os thread lock.
Browse files Browse the repository at this point in the history
When Pdeathsig is used, the child process will receive the signal
whenever the parent *thread* is killed. In go, an os thread can be
killed if it is locked to a goroutine and left locked when the
goroutine finishes execution. The go runtime will schedule goroutines to
OS threads as it sees fit, so this can result in random unexpected
signals being sent to child processes with pdeathsig set.

There's no currently *known* case where the shim code leaves goroutines
locked to a thread, but given this could happen now or in the future in
our code or in any dependency code, it's best to safeguard against this.

The fix is to keep the goroutine that spawns and waits on the child
process locked to its os thread. That way, we can be sure it will never
be killed by the go runtime.

See also: golang/go#27505

Signed-off-by: Erik Sipsma <erik@dagger.io>
  • Loading branch information
sipsma committed Apr 3, 2023
1 parent bf4e0d4 commit d6cc49d
Showing 1 changed file with 42 additions and 26 deletions.
68 changes: 42 additions & 26 deletions cmd/shim/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"os/exec"
"os/signal"
"path/filepath"
"runtime"
"strings"
"syscall"
"time"
Expand Down Expand Up @@ -387,36 +388,51 @@ func setupBundle() int {
}

// Run the actual runc binary as a child process with the (possibly updated) config
// #nosec G204
cmd := exec.Command(runcPath, os.Args[1:]...)
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.SysProcAttr = &syscall.SysProcAttr{
Pdeathsig: syscall.SIGKILL,
}

sigCh := make(chan os.Signal, 32)
signal.Notify(sigCh)
if err := cmd.Start(); err != nil {
fmt.Printf("Error starting runc: %v", err)
return 1
}
// Run it in a separate goroutine locked to the OS thread to ensure that Pdeathsig
// is never sent incorrectly: https://github.com/golang/go/issues/27505
exitCodeCh := make(chan int)
go func() {
for sig := range sigCh {
cmd.Process.Signal(sig)
runtime.LockOSThread()
defer runtime.UnlockOSThread()
defer close(exitCodeCh)
// #nosec G204
cmd := exec.Command(runcPath, os.Args[1:]...)
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.SysProcAttr = &syscall.SysProcAttr{
Pdeathsig: syscall.SIGKILL,
}
}()
if err := cmd.Wait(); err != nil {
if exiterr, ok := err.(*exec.ExitError); ok {
if exitcode, ok := exiterr.Sys().(syscall.WaitStatus); ok {
return exitcode.ExitStatus()

sigCh := make(chan os.Signal, 32)
signal.Notify(sigCh)
if err := cmd.Start(); err != nil {
fmt.Printf("Error starting runc: %v", err)
exitCodeCh <- 1
return
}
go func() {
for sig := range sigCh {
cmd.Process.Signal(sig)
}
}()
if err := cmd.Wait(); err != nil {
if exiterr, ok := err.(*exec.ExitError); ok {
if waitStatus, ok := exiterr.Sys().(syscall.WaitStatus); ok {
exitcode := waitStatus.ExitStatus()
if exitcode < 0 {
exitcode = 255 // 255 is "unknown exit code"
}
exitCodeCh <- exitcode
return
}
}
fmt.Printf("Error waiting for runc: %v", err)
exitCodeCh <- 1
return
}
fmt.Printf("Error waiting for runc: %v", err)
return 1
}
return 0
}()
return <-exitCodeCh
}

const aliasPrefix = "_DAGGER_HOSTNAME_ALIAS_"
Expand Down

0 comments on commit d6cc49d

Please sign in to comment.