Skip to content

Commit e024175

Browse files
committed
Merged PR 12878779: Enhance uvm_state::hostMounts to track in-use mounts, and prevent unmounting or deleting in-use things
[cherry-picked from d0334883cd43eecbb401a6ded3e0317179a3e54b] This set of changes adds some checks (when running with a confidential policy) to prevent the host from trying to clean up mounts, overlays, or the container states dir when the container is running (or when the overlay has not been unmounted yet). This is through enhancing the existing `hostMounts` utility, as well as adding a `terminated` flag to the Container struct. The correct order of operations should always be: - mount read-only layers and scratch (in any order, and individual containers (not the sandbox) might not have their own scratch) - mount the overlay - start the container - container terminates - unmount overlay - unmount read-only layers and scratch The starting up order is implied, and we now explicitly deny e.g. unmounting layer/scratch before unmounting overlay, or unmounting the overlay while container has not terminated. We also deny deleteContainerState requests when the container is running or when the overlay is mounted. Doing so when a container is running can result in unexpectedly deleting its files, which breaks it in unpredictable ways and is bad. Signed-off-by: Tingmao Wang <tingmaowang@microsoft.com>
1 parent d2a5931 commit e024175

File tree

6 files changed

+737
-98
lines changed

6 files changed

+737
-98
lines changed

internal/guest/bridge/bridge_v2.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -467,16 +467,10 @@ func (b *Bridge) deleteContainerStateV2(r *Request) (_ RequestResponse, err erro
467467
return nil, errors.Wrapf(err, "failed to unmarshal JSON in message \"%s\"", r.Message)
468468
}
469469

470-
c, err := b.hostState.GetCreatedContainer(request.ContainerID)
470+
err = b.hostState.DeleteContainerState(ctx, request.ContainerID)
471471
if err != nil {
472472
return nil, err
473473
}
474-
// remove container state regardless of delete's success
475-
defer b.hostState.RemoveContainer(request.ContainerID)
476-
477-
if err := c.Delete(ctx); err != nil {
478-
return nil, err
479-
}
480474

481475
return &prot.MessageResponseBase{}, nil
482476
}

internal/guest/runtime/hcsv2/container.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,9 @@ type Container struct {
7373
// and deal with the extra pointer dereferencing overhead.
7474
status atomic.Uint32
7575

76+
// Set to true when the init process for the container has exited
77+
terminated atomic.Bool
78+
7679
// scratchDirPath represents the path inside the UVM where the scratch directory
7780
// of this container is located. Usually, this is either `/run/gcs/c/<containerID>` or
7881
// `/run/gcs/c/<UVMID>/container_<containerID>` if scratch is shared with UVM scratch.

internal/guest/runtime/hcsv2/process.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ func newProcess(c *Container, spec *oci.Process, process runtime.Process, pid ui
9999
log.G(ctx).WithError(err).Error("failed to wait for runc process")
100100
}
101101
p.exitCode = exitCode
102+
c.terminated.Store(true)
102103
log.G(ctx).WithField("exitCode", p.exitCode).Debug("process exited")
103104

104105
// Free any process waiters

0 commit comments

Comments
 (0)