Skip to content

Commit c87c010

Browse files
committed
gcs: do not trigger container shutdown when signaling init process (#2538)
When implementing signal container process enforcement policy we introduced a bug, where instead of signalling just the container init process we ended up sending signals (SIGTERM or SIGKILL) to all processes running inside a container (by invoking `runc kill --all`). `container.Kill` no longer sends signals to all container processes. This results in an unpleasant behavior, where the init process could be handling (e.g. ignoring) SIGTERM, where as other processes inside container don't. This PR makes a change to the order in which the signal container policy is enforced: - always call `EnforceSignalContainerProcessPolicy` before sending any signals. Otherwise, this looks like a bug, since we would never call `EnforceSignalContainerProcessPolicy` with `signalingInitProcess == true` for `SIGTERM` and `SIGKILL` and potentially bypassing policies, which do not allow `SIGTERM` or `SIGKILL` to be sent to the init process. - no longer call `ShutdownContainer` and instead revert back to calling `process.Kill`. Signed-off-by: Maksim An <maksiman@microsoft.com> (cherry picked from commit 04735e0)
1 parent 41c1c93 commit c87c010

2 files changed

Lines changed: 1 addition & 14 deletions

File tree

internal/guest/runtime/hcsv2/uvm.go

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -681,17 +681,7 @@ func (h *Host) SignalContainerProcess(ctx context.Context, containerID string, p
681681
return err
682682
}
683683

684-
signalingInitProcess := (processID == c.initProcess.pid)
685-
686-
// Don't allow signalProcessV2 to route around container shutdown policy
687-
// Sending SIGTERM or SIGKILL to a containers init process will shut down
688-
// the container.
689-
if signalingInitProcess {
690-
if (signal == unix.SIGTERM) || (signal == unix.SIGKILL) {
691-
graceful := (signal == unix.SIGTERM)
692-
return h.ShutdownContainer(ctx, containerID, graceful)
693-
}
694-
}
684+
signalingInitProcess := processID == c.initProcess.pid
695685

696686
startupArgList := p.(*containerProcess).spec.Args
697687
err = h.securityPolicyEnforcer.EnforceSignalContainerProcessPolicy(ctx, containerID, signal, signalingInitProcess, startupArgList)

internal/guest/runtime/runc/container.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,6 @@ func (c *container) ExecProcess(process *oci.Process, stdioSet *stdio.Connection
7878
func (c *container) Kill(signal syscall.Signal) error {
7979
logrus.WithField(logfields.ContainerID, c.id).Debug("runc::container::Kill")
8080
args := []string{"kill"}
81-
if signal == syscall.SIGTERM || signal == syscall.SIGKILL {
82-
args = append(args, "--all")
83-
}
8481
args = append(args, c.id, strconv.Itoa(int(signal)))
8582
cmd := runcCommand(args...)
8683
out, err := cmd.CombinedOutput()

0 commit comments

Comments
 (0)