From e09058089817560ac5b55b7977f81143a24694e2 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Mon, 16 Jun 2025 14:47:20 -0400 Subject: [PATCH 1/2] [backport]Trim LCOW `GetProperties` response (#2458) * Trim LCOW `GetProperties` response Zero out the Linux `GetProperties` `Blkio` field, since it scales with the number of container layers attacked to the uVM. Additionally empty the `Rdma` and `Network` fields, in case they can also grow without bound. None of the fields are used in any code paths in the AzCRI, or exposed elsewhere. Clarify comment about the maximum message size, to reflect that it mirrors and HCS value and is not arbitrary. Additionally, don't quit the receive loop if the message size is too large, since that brings the bridge down with it. Signed-off-by: Hamza El-Saawy * PR: undo receive loop changes Signed-off-by: Hamza El-Saawy --------- Signed-off-by: Hamza El-Saawy (cherry picked from commit a53730ee83cf82dd158e5cdc3be2ec6c4846d6a6) Signed-off-by: Hamza El-Saawy --- internal/gcs/bridge.go | 4 +++- internal/guest/runtime/hcsv2/uvm.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/internal/gcs/bridge.go b/internal/gcs/bridge.go index 0aa9d54536..17e54f8242 100644 --- a/internal/gcs/bridge.go +++ b/internal/gcs/bridge.go @@ -32,6 +32,8 @@ const ( // maxMsgSize is the maximum size of an incoming message. This is not // enforced by the guest today but some maximum must be set to avoid // unbounded allocations. + // + // Matches HCS limitions on maximum (sent and received) message size. maxMsgSize = 0x10000 ) @@ -266,7 +268,7 @@ func readMessage(r io.Reader) (int64, msgType, []byte, error) { var h [hdrSize]byte _, err := io.ReadFull(r, h[:]) if err != nil { - return 0, 0, nil, err + return 0, 0, nil, fmt.Errorf("header read: %w", err) } typ := msgType(binary.LittleEndian.Uint32(h[hdrOffType:])) n := binary.LittleEndian.Uint32(h[hdrOffSize:]) diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 9b02a21aa0..9e98cefc91 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -24,6 +24,7 @@ import ( didx509resolver "github.com/Microsoft/didx509go/pkg/did-x509-resolver" "github.com/Microsoft/hcsshim/pkg/annotations" "github.com/Microsoft/hcsshim/pkg/securitypolicy" + cgroup1stats "github.com/containerd/cgroups/v3/cgroup1/stats" "github.com/mattn/go-shellwords" "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" @@ -837,7 +838,16 @@ func (h *Host) GetProperties(ctx context.Context, containerID string, query prot if err != nil { return nil, err } + // zero out [Blkio] sections, since: + // 1. (Az)CRI (currently) only looks at the CPU and memory sections; and + // 2. it can get very large for containers with many layers + cgroupMetrics.Blkio.Reset() + // also preemptively zero out [Rdma] and [Network], since they could also grow untenable large + cgroupMetrics.Rdma.Reset() + cgroupMetrics.Network = []*cgroup1stats.NetworkStat{} properties.Metrics = cgroupMetrics + default: + log.G(ctx).WithField("propertyType", requestedProperty).Warn("unknown or empty property type") } } From 9495cf4460b4bd16db9a940c1af23ed7bfc54280 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Mon, 16 Jun 2025 14:48:06 -0400 Subject: [PATCH 2/2] [backport]Swap `EvalSymlinks` with `ResolvePath` (#2455) * Swap `EvalSymlinks` with `ResolvePath` Redo PR 1644, which swapped builtin `"path/filepath".EvalSymlinks` with `"github.com/Microsoft/go-winio/pkg/fs".ResolvePath`, since the later is able to handle deeply nested symlinks and (as of [go1.23](golang.org/doc/go1.23#pathfilepathpkgpathfilepath)), mountpoints. Signed-off-by: Hamza El-Saawy * PR: update CIM test code Signed-off-by: Hamza El-Saawy --------- Signed-off-by: Hamza El-Saawy (cherry picked from commit e3722b0ed147343704636adbc6fc0998fb4fd182) Signed-off-by: Hamza El-Saawy --- internal/layers/lcow.go | 17 +++++++++++++++-- test/functional/make_uvm_cim_test.go | 10 ++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/internal/layers/lcow.go b/internal/layers/lcow.go index 2a6da2a622..dccd994e87 100644 --- a/internal/layers/lcow.go +++ b/internal/layers/lcow.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" + "github.com/Microsoft/go-winio/pkg/fs" "github.com/containerd/containerd/api/types" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -77,7 +78,13 @@ func (lc *lcowLayersCloser) Release(ctx context.Context) (retErr error) { // Returns the path at which the `rootfs` of the container can be accessed. Also, returns the path inside the // UVM at which container scratch directory is located. Usually, this path is the path at which the container // scratch VHD is mounted. However, in case of scratch sharing this is a directory under the UVM scratch. -func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers, guestRoot string, vm *uvm.UtilityVM) (_, _ string, _ resources.ResourceCloser, err error) { +func MountLCOWLayers( + ctx context.Context, + containerID string, + layers *LCOWLayers, + guestRoot string, + vm *uvm.UtilityVM, +) (_, _ string, _ resources.ResourceCloser, err error) { if vm == nil { return "", "", nil, errors.New("MountLCOWLayers cannot be called for process-isolated containers") } @@ -114,7 +121,13 @@ func MountLCOWLayers(ctx context.Context, containerID string, layers *LCOWLayers } hostPath := layers.ScratchVHDPath - hostPath, err = filepath.EvalSymlinks(hostPath) + // For LCOW, we can reuse another container's scratch space (usually the sandbox container's). + // + // When sharing a scratch space, the `hostPath` will be a symlink to the sandbox.vhdx location to use. + // When not sharing a scratch space, `hostPath` will be the path to the sandbox.vhdx to use. + // + // Evaluate the symlink here (if there is one). + hostPath, err = fs.ResolvePath(hostPath) if err != nil { return "", "", nil, fmt.Errorf("failed to eval symlinks on scratch path: %w", err) } diff --git a/test/functional/make_uvm_cim_test.go b/test/functional/make_uvm_cim_test.go index 968f1a0657..d99b1d1673 100644 --- a/test/functional/make_uvm_cim_test.go +++ b/test/functional/make_uvm_cim_test.go @@ -12,22 +12,24 @@ import ( "strings" "testing" + "github.com/Microsoft/go-winio/pkg/fs" "github.com/Microsoft/go-winio/pkg/guid" - "github.com/Microsoft/hcsshim/pkg/cimfs" - "github.com/Microsoft/hcsshim/pkg/extractuvm" "github.com/google/go-containerregistry/pkg/crane" v1 "github.com/google/go-containerregistry/pkg/v1" + + "github.com/Microsoft/hcsshim/pkg/cimfs" + "github.com/Microsoft/hcsshim/pkg/extractuvm" ) func compareFiles(t *testing.T, file1, file2 string) (bool, error) { t.Helper() - file1, err := filepath.EvalSymlinks(file1) + file1, err := fs.ResolvePath(file1) if err != nil { return false, err } - file2, err = filepath.EvalSymlinks(file2) + file2, err = fs.ResolvePath(file2) if err != nil { return false, err }