Skip to content

Commit

Permalink
slice bounds and nil VM access fix (microsoft#1754)
Browse files Browse the repository at this point in the history
Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
  • Loading branch information
helsaawy authored May 4, 2023
1 parent a8ec8c8 commit 8fa2489
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
13 changes: 8 additions & 5 deletions cmd/containerd-shim-runhcs-v1/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
package main

import (
gcontext "context"
"context"
"fmt"
"os"
"path/filepath"
Expand Down Expand Up @@ -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")
}
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 8 additions & 3 deletions internal/layers/layers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit 8fa2489

Please sign in to comment.