rootlesskit --pidns: fix propagating exit status

.github/workflows/main.yaml

@@ -19,6 +19,8 @@ jobs:
       uses: actions/checkout@v2
     - name: "Build integration test image"
       run: DOCKER_BUILDKIT=1 docker build -t rootlesskit:test-integration --target test-integration .
+    - name: "Integration test: exit-code"
+      run: docker run --rm --privileged rootlesskit:test-integration ./integration-exit-code.sh
     - name: "Integration test: propagation"
       run: docker run --rm --privileged rootlesskit:test-integration ./integration-propagation.sh
     - name: "Integration test: propagation (with `mount --make-rshared /`)"

cmd/rootlesskit/main.go

@@ -163,6 +163,11 @@ Note: RootlessKit requires /etc/subuid and /etc/subgid to be configured by the r
 			Usage: "mount propagation [rprivate, rslave]",
 			Value: "rprivate",
 		},
+		&cli.StringFlag{
+			Name:  "reaper",
+			Usage: "enable process reaper. Requires --pidns. [auto,true,false]",
+			Value: "auto",
+		},
 	}
 	app.Before = func(context *cli.Context) error {
 		if debug {
@@ -431,12 +436,25 @@ func (w *logrusDebugWriter) Write(p []byte) (int, error) {
 }
 
 func createChildOpt(clicontext *cli.Context, pipeFDEnvKey string, targetCmd []string) (child.Opt, error) {
+	pidns := clicontext.Bool("pidns")
 	opt := child.Opt{
 		PipeFDEnvKey: pipeFDEnvKey,
 		TargetCmd:    targetCmd,
-		MountProcfs:  clicontext.Bool("pidns"),
+		MountProcfs:  pidns,
 		Propagation:  clicontext.String("propagation"),
-		Reaper:       clicontext.Bool("pidns"),
+	}
+	switch reaperStr := clicontext.String("reaper"); reaperStr {
+	case "auto":
+		opt.Reaper = pidns
+		logrus.Debugf("reaper: auto chosen value: %v", opt.Reaper)
+	case "true":
+		if !pidns {
+			return opt, errors.New("reaper requires --pidns")
+		}
+		opt.Reaper = true
+	case "false":
+	default:
+		return opt, errors.Errorf("unknown reaper mode: %s", reaperStr)
 	}
 	switch s := clicontext.String("net"); s {
 	case "host":

hack/integration-exit-code.sh

@@ -0,0 +1,49 @@
+#!/bin/bash
+source $(realpath $(dirname $0))/common.inc.sh
+
+function test_exit_code() {
+	args="$@"
+	INFO "Testig exit status for args=${args}"
+	set +e
+	for f in 0 42; do
+		$ROOTLESSKIT $args sh -exc "exit $f" >/dev/null 2>&1
+		code=$?
+		if [ $code != $f ]; then
+			ERROR "expected code $f, got $code"
+			exit 1
+		fi
+	done
+}
+
+test_exit_code --pidns=false
+test_exit_code --pidns=true --reaper=auto
+test_exit_code --pidns=true --reaper=true
+test_exit_code --pidns=true --reaper=false
+
+function test_signal() {
+	args="$@"
+	INFO "Testig signal for args=${args}"
+	set +e
+	tmp=$(mktemp -d)
+	$ROOTLESSKIT --state-dir=${tmp}/state $args sleep infinity >${tmp}/out 2>&1 &
+	pid=$!
+	sleep 1
+	kill -SIGUSR1 $(cat ${tmp}/state/child_pid)
+	wait $pid
+	code=$?
+	if [ $code != 255 ]; then
+		ERROR "expected code 255, got $code"
+		exit 1
+	fi
+	if ! grep -q "user defined signal 1" ${tmp}/out; then
+		ERROR "didn't get SIGUSR1?"
+		cat ${tmp}/out
+		exit 1
+	fi
+	rm -rf $tmp
+}
+
+test_signal --pidns=false
+test_signal --pidns=true --reaper=auto
+test_signal --pidns=true --reaper=true
+test_signal --pidns=true --reaper=false

pkg/child/child.go

@@ -2,6 +2,7 @@ package child
 
 import (
 	"context"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"os/exec"
@@ -288,6 +289,7 @@ func setMountPropagation(propagation string) error {
 func runAndReap(cmd *exec.Cmd) error {
 	c := make(chan os.Signal, 32)
 	signal.Notify(c, syscall.SIGCHLD)
+	cmd.SysProcAttr.Setsid = true
 	if err := cmd.Start(); err != nil {
 		return err
 	}
@@ -297,17 +299,54 @@ func runAndReap(cmd *exec.Cmd) error {
 	result := make(chan error)
 	go func() {
 		defer close(result)
-		for range c {
-			for {
-				if pid, err := syscall.Wait4(-1, nil, syscall.WNOHANG, nil); err != nil || pid <= 0 {
-					break
-				} else {
-					if pid == cmd.Process.Pid {
-						result <- cmd.Wait()
-					}
+		for cEntry := range c {
+			logrus.Debugf("reaper: got signal %q", cEntry)
+			if wsPtr := reap(cmd.Process.Pid); wsPtr != nil {
+				ws := *wsPtr
+				if ws.Exited() && ws.ExitStatus() == 0 {
+					result <- nil
+					continue
 				}
+				var resultErr common.ErrorWithSys = &reaperErr{
+					ws: ws,
+				}
+				result <- resultErr
 			}
 		}
 	}()
 	return <-result
 }
+
+func reap(myPid int) *syscall.WaitStatus {
+	var res *syscall.WaitStatus
+	for {
+		var ws syscall.WaitStatus
+		pid, err := syscall.Wait4(-1, &ws, syscall.WNOHANG, nil)
+		logrus.Debugf("reaper: got ws=%+v, pid=%d, err=%+v", ws, pid, err)
+		if err != nil || pid <= 0 {
+			break
+		}
+		if pid == myPid {
+			res = &ws
+		}
+	}
+	return res
+}
+
+type reaperErr struct {
+	ws syscall.WaitStatus
+}
+
+func (e *reaperErr) Sys() interface{} {
+	return e.ws
+}
+
+func (e *reaperErr) Error() string {
+	if e.ws.Exited() {
+		return fmt.Sprintf("exit status %d", e.ws.ExitStatus())
+	}
+	if e.ws.Signaled() {
+		return fmt.Sprintf("signal: %s", e.ws.Signal())
+	}
+	return fmt.Sprintf("exited with WAITSTATUS=0x%08x", e.ws)
+}

pkg/common/exec.go

@@ -9,12 +9,18 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
+// ErrorWithSys is implemented by *exec.ExitError and *child.reaperErr
+type ErrorWithSys interface {
+	error
+	Sys() interface{}
+}
+
 func GetExecExitStatus(err error) (int, bool) {
 	err = errors.Cause(err)
 	if err == nil {
 		return 0, false
 	}
-	exitErr, ok := err.(*exec.ExitError)
+	exitErr, ok := err.(ErrorWithSys)
 	if !ok {
 		return 0, false
 	}