Skip to content

Commit 009d775

Browse files
committed
gcs: do not trigger container shutdown when signaling init process
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`). 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 6efa5fd commit 009d775

File tree

2 files changed

+3
-12
lines changed

2 files changed

+3
-12
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: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ func (c *container) ExecProcess(process *oci.Process, stdioSet *stdio.Connection
7474
return p, nil
7575
}
7676

77-
// Kill sends the specified signal to the container's init process.
77+
// Kill sends the specified signal to the container's init process. When syscall.SIGTERM or syscall.SIGKILL is sent,
78+
// send the specified signal to all processes inside the container
7879
func (c *container) Kill(signal syscall.Signal) error {
7980
logrus.WithField(logfields.ContainerID, c.id).Debug("runc::container::Kill")
8081
args := []string{"kill"}

0 commit comments

Comments
 (0)