Merge pull request #29525 from sahilnyk/exec-sigproxy-pgid

exec: forward signals to the exec session's process group

libpod/container_exec.go

@@ -760,22 +760,80 @@ func (c *Container) ExecResize(sessionID string, newSize resize.TerminalSize) er
 	return c.ociRuntime.ExecAttachResize(c, sessionID, newSize)
 }
 
+// ExecKill sends a signal to the process group of a running exec session,
+// reaching any children it spawned, not just the tracked PID.
+func (c *Container) ExecKill(sessionID string, sig uint) error {
+	if !c.batched {
+		c.lock.Lock()
+		defer c.lock.Unlock()
+
+		if err := c.syncContainer(); err != nil {
+			return err
+		}
+	}
+
+	session, ok := c.state.ExecSessions[sessionID]
+	if !ok {
+		return fmt.Errorf("container %s has no exec session with ID %s: %w", c.ID(), sessionID, define.ErrNoSuchExecSession)
+	}
+
+	if session.State != define.ExecStateRunning {
+		return fmt.Errorf("cannot signal container %s exec session %s as it is not running: %w", c.ID(), session.ID(), define.ErrExecSessionStateInvalid)
+	}
+
+	// Also confirms via pidhandle that this PID is still our session, not
+	// one the kernel already reused.
+	running, err := c.ociRuntime.ExecUpdateStatus(c, session.ID())
+	if err != nil {
+		return err
+	}
+	if !running {
+		session.State = define.ExecStateStopped
+
+		if err := c.save(); err != nil {
+			logrus.Errorf("Saving state of container %s: %v", c.ID(), err)
+		}
+
+		return fmt.Errorf("cannot signal container %s exec session %s as it has stopped: %w", c.ID(), session.ID(), define.ErrExecSessionStateInvalid)
+	}
+
+	pidHandle, err := pidhandle.NewPIDHandleFromString(session.PID, session.PIDData)
+	if err != nil {
+		return fmt.Errorf("getting the PID handle for pid %d from '%s': %w", session.PID, session.PIDData, err)
+	}
+	defer pidHandle.Close()
+
+	if err := pidHandle.KillProcessGroup(unix.Signal(sig)); err != nil {
+		if errors.Is(err, unix.ESRCH) {
+			return nil
+		}
+		return fmt.Errorf("killing container %s exec session %s process group %d: %w", c.ID(), session.ID(), session.PID, err)
+	}
+
+	return nil
+}
+
 func (c *Container) healthCheckExec(config *ExecConfig, timeout time.Duration, streams *define.AttachStreams) (int, error) {
 	return c.execLightweight(config, streams, timeout)
 }
 
-func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resize <-chan resize.TerminalSize) (int, error) {
-	return c.exec(config, streams, resize, false)
+// sessionIDCallback, if not nil, fires with the session's ID right after
+// creation, before start/attach, for callers that need it early.
+func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resize <-chan resize.TerminalSize, sessionIDCallback func(string)) (int, error) {
+	return c.exec(config, streams, resize, false, sessionIDCallback)
 }
 
 // Exec emulates the old Libpod exec API, providing a single call to create,
 // run, and remove an exec session. Returns exit code and error. Exit code is
 // not guaranteed to be set sanely if error is not nil.
-func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool) (exitCode int, retErr error) {
+func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resizeChan <-chan resize.TerminalSize, isHealthcheck bool, sessionIDCallback func(string)) (exitCode int, retErr error) {
 	sessionID, err := c.ExecCreate(config)
 	if err != nil {
 		return -1, err
 	}
+	if sessionIDCallback != nil {
+		sessionIDCallback(sessionID)
+	}
 	cleanup := true
 	defer func() {
 		if cleanup {

libpod/container_top_linux.go

@@ -399,7 +399,7 @@ func (c *Container) execPSinContainer(args []string) ([]string, error) {
 	cmd := append([]string{"ps"}, args...)
 	config := new(ExecConfig)
 	config.Command = cmd
-	ec, err := c.Exec(config, streams, nil)
+	ec, err := c.Exec(config, streams, nil, nil)
 	wPipe.Close()
 	if err != nil {
 		return nil, err

pkg/api/handlers/compat/exec.go

@@ -18,6 +18,7 @@ import (
 	"go.podman.io/podman/v6/pkg/api/server/idle"
 	api "go.podman.io/podman/v6/pkg/api/types"
 	"go.podman.io/podman/v6/pkg/domain/entities"
+	"go.podman.io/podman/v6/pkg/signal"
 	"go.podman.io/podman/v6/pkg/specgenutil"
 	"go.podman.io/podman/v6/pkg/util"
 )
@@ -219,6 +220,49 @@ func ExecStartHandler(w http.ResponseWriter, r *http.Request) {
 	logrus.Debugf("Attach for container %s exec session %s completed successfully", sessionCtr.ID(), sessionID)
 }
 
+// ExecKillHandler sends a signal to a running exec session.
+func ExecKillHandler(w http.ResponseWriter, r *http.Request) {
+	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)
+	decoder := utils.GetDecoder(r)
+
+	sessionID := mux.Vars(r)["id"]
+
+	query := struct {
+		Signal string `schema:"signal"`
+	}{}
+	if err := decoder.Decode(&query, r.URL.Query()); err != nil {
+		utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse parameters for %s: %w", r.URL.String(), err))
+		return
+	}
+
+	sig, err := signal.ParseSignalNameOrNumber(query.Signal)
+	if err != nil {
+		utils.Error(w, http.StatusBadRequest, err)
+		return
+	}
+
+	sessionCtr, err := runtime.GetExecSessionContainer(sessionID)
+	if err != nil {
+		utils.Error(w, http.StatusNotFound, err)
+		return
+	}
+
+	if err := sessionCtr.ExecKill(sessionID, uint(sig)); err != nil {
+		if errors.Is(err, define.ErrNoSuchExecSession) {
+			utils.Error(w, http.StatusNotFound, err)
+			return
+		}
+		if errors.Is(err, define.ErrExecSessionStateInvalid) {
+			utils.Error(w, http.StatusConflict, err)
+			return
+		}
+		utils.InternalServerError(w, err)
+		return
+	}
+
+	utils.WriteResponse(w, http.StatusOK, "OK")
+}
+
 // ExecRemoveHandler removes a exec session.
 func ExecRemoveHandler(w http.ResponseWriter, r *http.Request) {
 	runtime := r.Context().Value(api.RuntimeKey).(*libpod.Runtime)

pkg/api/server/register_exec.go

@@ -386,5 +386,36 @@ func (s *APIServer) registerExecHandlers(r *mux.Router) error {
 	//   500:
 	//     $ref: "#/responses/internalError"
 	r.Handle(VersionedPath("/libpod/exec/{id}/remove"), s.APIHandler(compat.ExecRemoveHandler)).Methods(http.MethodPost)
+
+	// swagger:operation POST /libpod/exec/{id}/kill libpod ExecKillLibpod
+	// ---
+	// tags:
+	//   - exec
+	// summary: Signal an exec instance
+	// description: |
+	//   Send a signal to a running exec instance's process group.
+	// parameters:
+	//  - in: path
+	//    name: id
+	//    type: string
+	//    required: true
+	//    description: Exec instance ID
+	//  - in: query
+	//    name: signal
+	//    type: string
+	//    required: true
+	//    description: Signal name or number to send.
+	// produces:
+	// - application/json
+	// responses:
+	//   200:
+	//     description: no error
+	//   404:
+	//     $ref: "#/responses/execSessionNotFound"
+	//   409:
+	//	   description: exec session is not running.
+	//   500:
+	//     $ref: "#/responses/internalError"
+	r.Handle(VersionedPath("/libpod/exec/{id}/kill"), s.APIHandler(compat.ExecKillHandler)).Methods(http.MethodPost)
 	return nil
 }

pkg/bindings/containers/attach.go

@@ -275,6 +275,30 @@ func ResizeExecTTY(ctx context.Context, sessionID string, options *ResizeExecTTY
 	return resizeTTY(ctx, bindings.JoinURL("exec", sessionID, "resize"), options.Height, options.Width)
 }
 
+// ExecKill sends a signal to a running exec session's process group.
+func ExecKill(ctx context.Context, sessionID string, signal string, options *ExecKillOptions) error {
+	if options == nil {
+		options = new(ExecKillOptions)
+	}
+	conn, err := bindings.GetClient(ctx)
+	if err != nil {
+		return err
+	}
+
+	params, err := options.ToParams()
+	if err != nil {
+		return err
+	}
+	params.Set("signal", signal)
+	rsp, err := conn.DoRequest(ctx, nil, http.MethodPost, bindings.JoinURL("exec", sessionID, "kill"), params, nil)
+	if err != nil {
+		return err
+	}
+	defer rsp.Body.Close()
+
+	return rsp.Process(nil)
+}
+
 // resizeTTY set size of TTY of container
 func resizeTTY(ctx context.Context, endpoint string, height *int, width *int) error {
 	conn, err := bindings.GetClient(ctx)

pkg/bindings/containers/types.go

@@ -293,6 +293,11 @@ type ResizeExecTTYOptions struct {
 	Width  *int
 }
 
+// ExecKillOptions are optional options for signalling an exec session.
+//
+//go:generate go run ../generator/generator.go ExecKillOptions
+type ExecKillOptions struct{}
+
 // ExecStartAndAttachOptions are optional options for resizing
 // container ExecTTYs
 //

pkg/bindings/containers/types_execkill_options.go

@@ -0,0 +1,18 @@
+// Code generated by go generate; DO NOT EDIT.
+package containers
+
+import (
+	"net/url"
+
+	"go.podman.io/podman/v6/pkg/bindings/internal/util"
+)
+
+// Changed returns true if named field has been set
+func (o *ExecKillOptions) Changed(fieldName string) bool {
+	return util.Changed(o, fieldName)
+}
+
+// ToParams formats struct fields to be passed to API service
+func (o *ExecKillOptions) ToParams() (url.Values, error) {
+	return util.ToParams(o)
+}

pkg/domain/infra/abi/terminal/sigproxy_commn.go

@@ -14,6 +14,40 @@ import (
 	"go.podman.io/podman/v6/pkg/signal"
 )
 
+// ProxyExecSignals forwards signals Podman receives to an exec session, via
+// Container.ExecKill so delivery goes through libpod's own PID bookkeeping.
+func ProxyExecSignals(ctr *libpod.Container, sessionID string) {
+	// Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going
+	// to the exec session now.
+	shutdown.Stop() //nolint: errcheck
+
+	sigBuffer := make(chan os.Signal, signal.SignalBufferSize)
+	signal.CatchAll(sigBuffer)
+
+	logrus.Debugf("Enabling signal proxying to exec session %s", sessionID)
+
+	go func() {
+		for s := range sigBuffer {
+			syscallSignal := s.(syscall.Signal)
+
+			if err := ctr.ExecKill(sessionID, uint(syscallSignal)); err != nil {
+				if !errors.Is(err, define.ErrExecSessionStateInvalid) && !errors.Is(err, define.ErrNoSuchExecSession) {
+					logrus.Errorf("forwarding signal %d to exec session %s: %v", s, sessionID, err)
+					continue
+				}
+				// Session is gone: send this one to ourselves rather than
+				// lose it, and let the defaults play out.
+				logrus.Infof("Ceasing signal forwarding, exec session %s has stopped", sessionID)
+				signal.StopCatch(sigBuffer)
+				if err := syscall.Kill(syscall.Getpid(), syscallSignal); err != nil {
+					logrus.Errorf("Failed to kill pid %d", syscall.Getpid())
+				}
+				return
+			}
+		}
+	}()
+}
+
 // ProxySignals ...
 func ProxySignals(ctr *libpod.Container) {
 	// Stop catching the shutdown signals (SIGINT, SIGTERM) - they're going

pkg/domain/infra/abi/terminal/terminal_common.go

@@ -35,7 +35,10 @@ func ExecAttachCtr(ctx context.Context, ctr *libpod.Container, execConfig *libpo
 			}
 		}()
 	}
-	return ctr.Exec(execConfig, streams, resizechan)
+	// Forward our signals on, so killing `podman exec` stops what it started.
+	return ctr.Exec(execConfig, streams, resizechan, func(sessionID string) {
+		ProxyExecSignals(ctr, sessionID)
+	})
 }
 
 // StartAttachCtr starts and (if required) attaches to a container

pkg/domain/infra/tunnel/containers.go

@@ -635,6 +635,15 @@ func (ic *ContainerEngine) ContainerExec(_ context.Context, nameOrID string, opt
 	if err != nil {
 		return 125, err
 	}
+	// Forward our signals on, so killing `podman exec` stops what it started,
+	// same as the local path does via ProxyExecSignals.
+	remoteProxySignals(sessionID, func(sigName string) error {
+		err := containers.ExecKill(ic.ClientCtx, sessionID, sigName, nil)
+		if err != nil {
+			logrus.Debugf("forwarding signal %q to exec session %s: %v", sigName, sessionID, err)
+		}
+		return err
+	})
 	defer func() {
 		if err := containers.ExecRemove(ic.ClientCtx, sessionID, nil); err != nil {
 			apiErr := new(bindings.APIVersionError)

pkg/pidhandle/pidhandle.go

@@ -29,6 +29,8 @@ type PIDHandle interface {
 	Close() error
 	// Sends the signal to process.
 	Kill(signal unix.Signal) error
+	// Sends the signal to the process's entire process group.
+	KillProcessGroup(signal unix.Signal) error
 	// Returns true in case the process is still alive.
 	IsAlive() (bool, error)
 	// Returns a serialized representation of the PIDHandle.
@@ -61,8 +63,8 @@ func (h *pidHandle) Close() error {
 	return nil
 }
 
-// Sends the signal to process.
-func (h *pidHandle) Kill(signal unix.Signal) error {
+// Returns ESRCH if the process is gone or its PID was recycled.
+func (h *pidHandle) checkIdentity() error {
 	if h.pidData == noSuchProcessID {
 		// The process did not exist when we created the PIDHandle, so return
 		// ESRCH error.
@@ -92,9 +94,25 @@ func (h *pidHandle) Kill(signal unix.Signal) error {
 		}
 	}
 
+	return nil
+}
+
+// Sends the signal to process.
+func (h *pidHandle) Kill(signal unix.Signal) error {
+	if err := h.checkIdentity(); err != nil {
+		return err
+	}
 	return unix.Kill(h.pid, signal)
 }
 
+// Sends the signal to the process's entire process group.
+func (h *pidHandle) KillProcessGroup(signal unix.Signal) error {
+	if err := h.checkIdentity(); err != nil {
+		return err
+	}
+	return unix.Kill(-h.pid, signal)
+}
+
 // Returns true in case the process is still alive.
 func (h *pidHandle) IsAlive() (bool, error) {
 	err := h.Kill(0)

pkg/pidhandle/pidhandle_linux.go

@@ -10,11 +10,17 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"sync/atomic"
 
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
 )
 
+// PIDFD_SIGNAL_PROCESS_GROUP, added in Linux 6.9, not yet in golang.org/x/sys/unix.
+const pidfdSignalProcessGroup = 1 << 2
+
+var processGroupSignalUnsupported atomic.Bool
+
 type pidfdHandle struct {
 	pidfd        int
 	normalHandle pidHandle
@@ -160,6 +166,22 @@ func (h *pidfdHandle) Kill(signal unix.Signal) error {
 	return h.normalHandle.Kill(signal)
 }
 
+// Sends the signal to the process's entire process group.
+func (h *pidfdHandle) KillProcessGroup(signal unix.Signal) error {
+	if h.pidfd > -1 && !processGroupSignalUnsupported.Load() {
+		err := pidfdSendSignal(h.pidfd, signal, nil, pidfdSignalProcessGroup)
+		if err == nil {
+			return nil
+		}
+		if !errors.Is(err, unix.EINVAL) {
+			return err
+		}
+		processGroupSignalUnsupported.Store(true)
+	}
+
+	return h.normalHandle.KillProcessGroup(signal)
+}
+
 // Returns true in case the process is still alive.
 func (h *pidfdHandle) IsAlive() (bool, error) {
 	err := h.Kill(0)

pkg/pidhandle/pidhandle_linux_test.go

@@ -233,3 +233,85 @@ func TestPIDHandleKillPidfdNotSupportedStartTimeNotMatch(t *testing.T) {
 	assert.NoError(t, err)
 	assert.False(t, isAlive)
 }
+
+func TestPIDHandleKillProcessGroupUsesFlagWhenSupported(t *testing.T) {
+	processGroupSignalUnsupported.Store(false)
+	defer processGroupSignalUnsupported.Store(false)
+
+	originalSend := pidfdSendSignal
+	defer func() { pidfdSendSignal = originalSend }()
+	var gotFlags int
+	pidfdSendSignal = func(_ int, _ unix.Signal, _ *unix.Siginfo, flags int) error {
+		gotFlags = flags
+		return nil
+	}
+
+	h := &pidfdHandle{
+		pidfd:        123,
+		normalHandle: pidHandle{pid: os.Getpid(), pidData: "start-time:1234567890"},
+	}
+
+	err := h.KillProcessGroup(unix.SIGTERM)
+	assert.NoError(t, err)
+	assert.Equal(t, pidfdSignalProcessGroup, gotFlags)
+}
+
+func TestPIDHandleKillProcessGroupFallsBackOnEinval(t *testing.T) {
+	processGroupSignalUnsupported.Store(false)
+	defer processGroupSignalUnsupported.Store(false)
+
+	originalSend := pidfdSendSignal
+	defer func() { pidfdSendSignal = originalSend }()
+	pidfdSendSignal = func(_ int, _ unix.Signal, _ *unix.Siginfo, _ int) error {
+		return unix.EINVAL
+	}
+
+	h := &pidfdHandle{
+		pidfd:        123,
+		normalHandle: pidHandle{pid: os.Getpid(), pidData: "start-time:1234567890"},
+	}
+
+	err := h.KillProcessGroup(0)
+	assert.ErrorIs(t, err, unix.ESRCH)
+	assert.True(t, processGroupSignalUnsupported.Load())
+}
+
+func TestPIDHandleKillProcessGroupSkipsFlagOnceCached(t *testing.T) {
+	processGroupSignalUnsupported.Store(true)
+	defer processGroupSignalUnsupported.Store(false)
+
+	originalSend := pidfdSendSignal
+	defer func() { pidfdSendSignal = originalSend }()
+	pidfdSendSignal = func(_ int, _ unix.Signal, _ *unix.Siginfo, _ int) error {
+		t.Fatal("pidfdSendSignal should not be called once cached as unsupported")
+		return nil
+	}
+
+	h := &pidfdHandle{
+		pidfd:        123,
+		normalHandle: pidHandle{pid: os.Getpid(), pidData: "start-time:1234567890"},
+	}
+
+	err := h.KillProcessGroup(0)
+	assert.ErrorIs(t, err, unix.ESRCH)
+}
+
+func TestPIDHandleKillProcessGroupPropagatesRealError(t *testing.T) {
+	processGroupSignalUnsupported.Store(false)
+	defer processGroupSignalUnsupported.Store(false)
+
+	originalSend := pidfdSendSignal
+	defer func() { pidfdSendSignal = originalSend }()
+	pidfdSendSignal = func(_ int, _ unix.Signal, _ *unix.Siginfo, _ int) error {
+		return unix.EPERM
+	}
+
+	h := &pidfdHandle{
+		pidfd:        123,
+		normalHandle: pidHandle{pid: os.Getpid(), pidData: "start-time:1234567890"},
+	}
+
+	err := h.KillProcessGroup(unix.SIGTERM)
+	assert.ErrorIs(t, err, unix.EPERM)
+	assert.False(t, processGroupSignalUnsupported.Load())
+}

test/system/032-sig-proxy.bats

@@ -32,4 +32,54 @@ load helpers.sig-proxy
     _test_sigproxy c_attach $kidpid
 }
 
+@test "podman sigproxy test: exec" {
+    local cname=c-exec-$(safename)
+    run_podman run -d --name $cname $IMAGE top
+
+    # sleep 97 is spawned by the exec'd shell, so both share a process group
+    # and signalling it must take down both.
+    # See above comments regarding $PODMAN and backgrounding.
+    # SIGTERM, not SIGINT: shells ignore SIGINT in background jobs.
+    "${PODMAN_CMD[@]}" exec $cname sh -c 'sleep 97 & sleep 98' &
+    local kidpid=$!
+
+    # Wait for both to come up
+    local timeout=10
+    while :;do
+          sleep 0.5
+          run_podman top $cname args
+          if [[ "$output" =~ "sleep 97" ]] && [[ "$output" =~ "sleep 98" ]]; then
+              break
+          fi
+          timeout=$((timeout - 1))
+          if [[ $timeout -eq 0 ]]; then
+              die "Timed out waiting for exec'd processes to start"
+          fi
+    done
+
+    kill -TERM $kidpid
+    local exec_status=0
+    wait $kidpid || exec_status=$?
+    # 128 + SIGTERM(15): the exec'd shell died from the forwarded signal,
+    # not from some other error.
+    is "$exec_status" "143" "podman exec exit status reflects SIGTERM"
+
+    # Neither may outlive the exec. A dead child lingers as "[sleep]", so
+    # match full command lines.
+    timeout=20
+    while :;do
+          sleep 0.5
+          run_podman top $cname args
+          if [[ ! "$output" =~ "sleep 9" ]]; then
+              break
+          fi
+          timeout=$((timeout - 1))
+          if [[ $timeout -eq 0 ]]; then
+              die "Timed out waiting for exec'd processes to be signalled"
+          fi
+    done
+
+    run_podman rm -f -t0 $cname
+}
+
 # vim: filetype=sh