Skip to content

Commit 04735e0

Browse files
authored
gcs: do not trigger container shutdown when signaling init process (microsoft#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>
1 parent 296144f commit 04735e0

File tree

2 files changed

+1
-14
lines changed

2 files changed

+1
-14
lines changed

internal/guest/runtime/hcsv2/uvm.go

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

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

698688
startupArgList := p.(*containerProcess).spec.Args
699689
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)