From d6cc49dd0d4c034a98dcc8ca97a99948c1c0bf20 Mon Sep 17 00:00:00 2001
From: Erik Sipsma <erik@dagger.io>
Date: Mon, 3 Apr 2023 08:07:10 -0700
Subject: [PATCH] Safeguard pdeathsig usage in shim w/ os thread lock.

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: https://github.com/golang/go/issues/27505

Signed-off-by: Erik Sipsma <erik@dagger.io>
---
 cmd/shim/main.go | 68 ++++++++++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 26 deletions(-)

diff --git a/cmd/shim/main.go b/cmd/shim/main.go
index 1b994f8f2c..426ac1c608 100644
--- a/cmd/shim/main.go
+++ b/cmd/shim/main.go
@@ -11,6 +11,7 @@ import (
 	"os/exec"
 	"os/signal"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"syscall"
 	"time"
@@ -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_"