From 8fa2489ff65feba90a32e7bee6e9eea2d79957d3 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy <84944216+helsaawy@users.noreply.github.com> Date: Thu, 4 May 2023 17:57:01 -0400 Subject: [PATCH] slice bounds and nil VM access fix (#1754) Signed-off-by: Hamza El-Saawy --- cmd/containerd-shim-runhcs-v1/delete.go | 13 ++++++++----- internal/layers/layers.go | 11 ++++++++--- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/containerd-shim-runhcs-v1/delete.go b/cmd/containerd-shim-runhcs-v1/delete.go index cb8d5075dd..8b9c8db56f 100644 --- a/cmd/containerd-shim-runhcs-v1/delete.go +++ b/cmd/containerd-shim-runhcs-v1/delete.go @@ -3,7 +3,7 @@ package main import ( - gcontext "context" + "context" "fmt" "os" "path/filepath" @@ -50,16 +50,16 @@ This command allows containerd to delete any container resources created, mounte The delete command will be executed in the container's bundle as its cwd. `, SkipArgReorder: true, - Action: func(context *cli.Context) (err error) { + Action: func(cCtx *cli.Context) (err error) { // We cant write anything to stdout for this cmd other than the // task.DeleteResponse by protocol. We can write to stderr which will be // logged as a warning in containerd. - ctx, span := oc.StartSpan(gcontext.Background(), "delete") + ctx, span := oc.StartSpan(context.Background(), "delete") defer span.End() defer func() { oc.SetSpanStatus(span, err) }() - bundleFlag := context.GlobalString("bundle") + bundleFlag := cCtx.GlobalString("bundle") if bundleFlag == "" { return errors.New("bundle is required") } @@ -107,7 +107,10 @@ The delete command will be executed in the container's bundle as its cwd. // be deleted, but if the shim crashed unexpectedly (panic, terminated etc.) then the account may still be around. // The username will be the container ID so try and delete it here. The username character limit is 20, so we need to // slice down the container ID a bit. - username := idFlag[:winapi.UserNameCharLimit] + username := idFlag + if len(username) > winapi.UserNameCharLimit { + username = username[:winapi.UserNameCharLimit] + } // Always try and delete the user, if it doesn't exist we'll get a specific error code that we can use to // not log any warnings. diff --git a/internal/layers/layers.go b/internal/layers/layers.go index fd7cfa5389..df4df03261 100644 --- a/internal/layers/layers.go +++ b/internal/layers/layers.go @@ -65,6 +65,10 @@ func (lc *lcowLayersCloser) Release(ctx context.Context) (retErr error) { // 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, layerFolders []string, 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") + } + if vm.OS() != "linux" { return "", "", nil, errors.New("MountLCOWLayers should only be called for LCOW") } @@ -161,13 +165,14 @@ func MountLCOWLayers(ctx context.Context, containerID string, layerFolders []str // Job container: Returns the mount path on the host as a volume guid, with the volume mounted on // the host at `volumeMountPath`. func MountWCOWLayers(ctx context.Context, containerID string, layerFolders []string, guestRoot, volumeMountPath string, vm *uvm.UtilityVM) (_ string, _ resources.ResourceCloser, err error) { + if vm == nil { + return mountWCOWHostLayers(ctx, layerFolders, volumeMountPath) + } + if vm.OS() != "windows" { return "", nil, errors.New("MountWCOWLayers should only be called for WCOW") } - if vm == nil { - return mountWCOWHostLayers(ctx, layerFolders, volumeMountPath) - } return mountWCOWIsolatedLayers(ctx, containerID, layerFolders, guestRoot, volumeMountPath, vm) }