From 8bc0804b12b54bc07b6c80cd2e03cbd8148c14b4 Mon Sep 17 00:00:00 2001 From: Daniel Pfeifer Date: Thu, 9 Apr 2026 16:44:40 -0700 Subject: [PATCH] Add runsc kill --pgid Follow up PR of https://github.com/google/gvisor/pull/12739 for adding `runsc kill --pgid xxx` support. ## Changes ## Adds a `--pgid` flag to `runsc kill` that sends a signal to all processes in a given process group (identified by PGID in the root PID namespace). This complements the existing `--pid` and `--all` flags. The signal is delivered through the existing `ContMgrSignal` `RPC` via a new `DeliverToProcessGroup` delivery mode, following the same `container -> sandbox -> loader` chain as the other signal modes. ## Testing ## `TestSignalProcessGroup` creates a 3-process container (`init -> child -> grandchild`) where the child calls `setpgid` to form a new process group shared with the grandchild. The test sends `SIGKILL` to that PGID and verifies: - Both child and grandchild are killed - Init (PGID 1) survives A new `task-tree-pgid` test app subcommand supports this by spawning a deterministic process tree with a distinct process group. FUTURE_COPYBARA_INTEGRATE_REVIEW=https://github.com/google/gvisor/pull/12905 from danielpfeifer02:dpfeifer/runsc-kill-pgid 7afe64181add61f7992deb5780b9697e9aa01a75 PiperOrigin-RevId: 897368159 --- runsc/boot/controller.go | 6 +++ runsc/boot/loader.go | 31 +++++++++++ runsc/cmd/kill.go | 25 +++++++-- runsc/container/container.go | 13 +++++ runsc/container/container_test.go | 87 +++++++++++++++++++++++++++++++ runsc/sandbox/sandbox.go | 17 ++++++ test/cmd/test_app/main.go | 59 +++++++++++++++++++++ 7 files changed, 234 insertions(+), 4 deletions(-) diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 3467a3a682..7fff05fb4e 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -918,6 +918,10 @@ const ( // process. If PID is 0, then the signal is delivered to the foreground // process group for the TTY for the init process. DeliverToForegroundProcessGroup + + // DeliverToProcessGroup delivers the signal to all processes in the + // process group identified by a PGID. + DeliverToProcessGroup ) func (s SignalDeliveryMode) String() string { @@ -928,6 +932,8 @@ func (s SignalDeliveryMode) String() string { return "All" case DeliverToForegroundProcessGroup: return "Foreground Process Group" + case DeliverToProcessGroup: + return "Process Group" } return fmt.Sprintf("unknown signal delivery mode: %d", s) } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 770c4e9136..b7de068bc1 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1826,6 +1826,15 @@ func (l *Loader) signal(cid string, pid, signo int32, mode SignalDeliveryMode) e } return nil + case DeliverToProcessGroup: + if pid == 0 { + return fmt.Errorf("PGID must be set when signaling a process group") + } + if err := l.signalProcessGroup(cid, kernel.ProcessGroupID(pid), signo); err != nil { + return fmt.Errorf("signaling process group %d: %w", pid, err) + } + return nil + default: panic(fmt.Sprintf("unknown signal delivery mode %v", mode)) } @@ -1905,6 +1914,28 @@ func (l *Loader) signalAllProcesses(cid string, signo int32) error { return l.k.SendContainerSignal(cid, &linux.SignalInfo{Signo: signo}) } +// signalProcessGroup sends the signal to all processes in the process group +// identified by pgid. pgid is relative to the root PID namespace. It verifies +// that the process group exists in the container with the given ID. +func (l *Loader) signalProcessGroup(cid string, pgid kernel.ProcessGroupID, signo int32) error { + pg := l.k.RootPIDNamespace().ProcessGroupWithID(pgid) + if pg == nil { + return fmt.Errorf("no such process group with PGID %d", pgid) + } + // Verify that the process group exists in correct container. + found := false + for _, tg := range l.k.TaskSet().Root.ThreadGroups() { + if tg.ProcessGroup() == pg && tg.Leader().ContainerID() == cid { + found = true + break + } + } + if !found { + return fmt.Errorf("process group %d does not belong to container %q", pgid, cid) + } + return l.k.SendExternalSignalProcessGroup(pg, &linux.SignalInfo{Signo: signo}) +} + // threadGroupFromID is similar to tryThreadGroupFromIDLocked except that it // acquires mutex before calling it and fails in case container hasn't started // yet. diff --git a/runsc/cmd/kill.go b/runsc/cmd/kill.go index c5fbb67e30..ff86c2e71b 100644 --- a/runsc/cmd/kill.go +++ b/runsc/cmd/kill.go @@ -32,8 +32,9 @@ import ( // Kill implements subcommands.Command for the "kill" command. type Kill struct { containerLoader - all bool - pid int + all bool + pid int + pgid int } // Name implements subcommands.Command.Name. @@ -55,6 +56,7 @@ func (*Kill) Usage() string { func (k *Kill) SetFlags(f *flag.FlagSet) { f.BoolVar(&k.all, "all", false, "send the specified signal to all processes inside the container") f.IntVar(&k.pid, "pid", 0, "send the specified signal to a specific process. pid is relative to the root PID namespace") + f.IntVar(&k.pgid, "pgid", 0, "send the specified signal to all processes in the given process group. pgid is relative to the root PID namespace") } // FetchSpec implements util.SubCommand.FetchSpec. @@ -75,8 +77,19 @@ func (k *Kill) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma conf := args[0].(*config.Config) - if k.pid != 0 && k.all { - util.Fatalf("it is invalid to specify both --all and --pid") + // Validate that at most one targeting mode is used. + modes := 0 + if k.all { + modes++ + } + if k.pid != 0 { + modes++ + } + if k.pgid != 0 { + modes++ + } + if modes > 1 { + util.Fatalf("it is invalid to specify more than one of --all, --pid, and --pgid") } c, err := k.loadContainer(conf, f, container.LoadOpts{}) @@ -101,6 +114,10 @@ func (k *Kill) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma if err := c.SignalProcess(sig, int32(k.pid)); err != nil { util.Fatalf("failed to signal pid %d: %v", k.pid, err) } + } else if k.pgid != 0 { + if err := c.SignalProcessGroup(sig, int32(k.pgid)); err != nil { + util.Fatalf("failed to signal process group %d: %v", k.pgid, err) + } } else { if err := c.SignalContainer(sig, k.all); err != nil { util.Fatalf("%v", err) diff --git a/runsc/container/container.go b/runsc/container/container.go index 9ac08e5b8d..1621fee7d0 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -780,6 +780,19 @@ func (c *Container) SignalProcess(sig unix.Signal, pid int32) error { return c.Sandbox.SignalProcess(c.ID, int32(pid), sig, false) } +// SignalProcessGroup sends sig to all processes in the given process group +// inside the container. +func (c *Container) SignalProcessGroup(sig unix.Signal, pgid int32) error { + log.Debugf("Signal process group %d in container, cid: %s, signal: %v (%d)", pgid, c.ID, sig, sig) + if err := c.requireStatus("signal a process group inside", Running); err != nil { + return err + } + if !c.IsSandboxRunning() { + return fmt.Errorf("sandbox is not running") + } + return c.Sandbox.SignalProcessGroup(c.ID, pgid, sig) +} + // ForwardSignals forwards all signals received by the current process to the // container process inside the sandbox. It returns a function that will stop // forwarding signals. diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index c7471b1033..c68b5101ff 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -1023,6 +1023,93 @@ func TestKillPid(t *testing.T) { } } +// TestSignalProcessGroup verifies that SignalProcessGroup kills all +// processes in the targeted process group while leaving other groups +// running. +func TestSignalProcessGroup(t *testing.T) { + for name, conf := range configs(t, false /* noOverlay */) { + t.Run(name, func(t *testing.T) { + app, err := testutil.FindFile("test/cmd/test_app/test_app") + if err != nil { + t.Fatal("error finding test_app:", err) + } + + spec := testutil.NewSpecWithArgs(app, "task-tree-pgid") + _, bundleDir, cleanup, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatalf("error setting up container: %v", err) + } + defer cleanup() + + args := Args{ + ID: testutil.RandomContainerID(), + Spec: spec, + BundleDir: bundleDir, + } + cont, err := New(conf, args) + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer cont.Destroy() + if err := cont.Start(conf); err != nil { + t.Fatalf("error starting container: %v", err) + } + + // Wait for all 3 processes: init, child, grandchild. + if err := waitForProcessCount(cont, 3); err != nil { + t.Fatalf("timed out waiting for processes: %v", err) + } + + // Collect PGIDs. + procs, err := cont.Processes() + if err != nil { + t.Fatalf("failed to get process list: %v", err) + } + t.Logf("before signal: %s", procListToString(procs)) + + // Init (PID 1) is in PGID 1. + // Child + grandchild should share a PGID != 1. + pgidA := int32(1) + pgidBCount := make(map[int32]int) + for _, p := range procs { + if int32(p.PGID) != pgidA { + pgidBCount[int32(p.PGID)]++ + } + } + + // Find the PGID shared by child+grandchild. + var pgidB int32 + for pgid, n := range pgidBCount { + if n == 2 { + pgidB = pgid + } + } + if pgidB == 0 { + t.Fatalf("expected child and grandchild to share a PGID distinct from init (%d); got: %v", pgidA, pgidBCount) + } + t.Logf("PGID_init=%d, PGID_target=%d (%d processes)", pgidA, pgidB, pgidBCount[pgidB]) + + // Signal the target PGID (both child and grandchild should die, init survives). + if err := cont.SignalProcessGroup(unix.SIGKILL, pgidB); err != nil { + t.Fatalf("SignalProcessGroup(%d): %v", pgidB, err) + } + + if err := waitForProcessCount(cont, 1); err != nil { + procs, procsErr := cont.Processes() + t.Fatalf("expected only init to survive: %v; processes: %s / %v", err, procListToString(procs), procsErr) + } + + procs, err = cont.Processes() + if err != nil { + t.Fatalf("failed to get process list: %v", err) + } + if len(procs) != 1 || procs[0].PID != 1 { + t.Fatalf("expected only PID 1 to survive, got: %s", procListToString(procs)) + } + }) + } +} + // testCheckpointRestore creates a container that continuously writes successive // integers to a file. To test checkpoint and restore functionality, the // container is checkpointed and the last number printed to the file is diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index b03ed9cc41..f6ff5058cc 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -1539,6 +1539,23 @@ func (s *Sandbox) SignalProcess(cid string, pid int32, sig unix.Signal, fgProces return nil } +// SignalProcessGroup sends the signal to all processes in the process group +// identified by pgid. pgid is relative to the root PID namespace. +func (s *Sandbox) SignalProcessGroup(cid string, pgid int32, sig unix.Signal) error { + log.Debugf("Signal sandbox %q process group %d", s.ID, pgid) + + args := boot.SignalArgs{ + CID: cid, + Signo: int32(sig), + PID: pgid, + Mode: boot.DeliverToProcessGroup, + } + if err := s.call(boot.ContMgrSignal, &args, nil); err != nil { + return fmt.Errorf("signaling container %q PGID %d: %v", cid, pgid, err) + } + return nil +} + // CheckpointOpts contains the options for checkpointing a sandbox. type CheckpointOpts struct { Compression statefile.CompressionLevel diff --git a/test/cmd/test_app/main.go b/test/cmd/test_app/main.go index 58bc1baf16..7ef3ef7bdb 100644 --- a/test/cmd/test_app/main.go +++ b/test/cmd/test_app/main.go @@ -55,6 +55,7 @@ func main() { subcommands.Register(new(reaper), "") subcommands.Register(new(syscall), "") subcommands.Register(new(taskTree), "") + subcommands.Register(new(taskTreePGID), "") subcommands.Register(new(uds), "") subcommands.Register(new(zombieTest), "") registerSubcommandsExtra() @@ -372,6 +373,64 @@ func (c *taskTree) Execute(ctx context.Context, f *flag.FlagSet, args ...any) su return subcommands.ExitSuccess } +type taskTreePGID struct { + level int +} + +// Name implements subcommands.Command.Name. +func (*taskTreePGID) Name() string { + return "task-tree-pgid" +} + +// Synopsis implements subcommands.Command.Synopsys. +func (*taskTreePGID) Synopsis() string { + return "creates a child+grandchild in a new process group" +} + +// Usage implements subcommands.Command.Usage. +func (*taskTreePGID) Usage() string { + return "task-tree-pgid --level=N\n" +} + +// SetFlags implements subcommands.Command.SetFlags. +func (c *taskTreePGID) SetFlags(f *flag.FlagSet) { + f.IntVar(&c.level, "level", 0, "0=init, 1=child (new pgid), 2=grandchild") +} + +// Execute implements subcommands.Command.Execute. +func (c *taskTreePGID) Execute(ctx context.Context, f *flag.FlagSet, args ...any) subcommands.ExitStatus { + switch c.level { + case 0: + stop := testutil.StartReaper() + defer stop() + cmd := exec.Command("/proc/self/exe", "task-tree-pgid", "--level=1") + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.SysProcAttr = &sys.SysProcAttr{Setpgid: true} + if err := cmd.Start(); err != nil { + log.Fatalf("level 0: failed to start child: %v", err) + } + for { + time.Sleep(time.Hour) + } + + case 1: + cmd := exec.Command("/proc/self/exe", "task-tree-pgid", "--level=2") + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if err := cmd.Start(); err != nil { + log.Fatalf("level 1: failed to start grandchild: %v", err) + } + cmd.Wait() + + case 2: + for { + time.Sleep(time.Hour) + } + } + return subcommands.ExitSuccess +} + type gvisorDetect struct { exitCodeOnGVisor int exitCodeOnNotGVisor int