From aa62272382d70c09e1f125a740496e2a0c1b6d93 Mon Sep 17 00:00:00 2001 From: Liu Hua Date: Mon, 19 Apr 2021 19:57:45 +0800 Subject: [PATCH 1/2] tiny fix iterative checkpoint test case Criu creates symlink named "parent" under image-path while doing interactive checkpoint, without checking wether symlink points to right place. This patch corrects related test case. Signed-off-by: Liu Hua --- tests/integration/checkpoint.bats | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/integration/checkpoint.bats b/tests/integration/checkpoint.bats index 535b1c1057f..f168b1be112 100644 --- a/tests/integration/checkpoint.bats +++ b/tests/integration/checkpoint.bats @@ -159,10 +159,13 @@ function simple_cr() { # checkpoint the running container mkdir image-dir mkdir work-dir - runc --criu "$CRIU" checkpoint --parent-path ./parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + runc --criu "$CRIU" checkpoint --parent-path ../parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox grep -B 5 Error ./work-dir/dump.log || true [ "$status" -eq 0 ] + # check parent path is valid + [ -e ./image-dir/parent ] + # after checkpoint busybox is no longer running testcontainer test_busybox checkpointed From 23e3794d9c337e903d90229b206b041a185c1a2f Mon Sep 17 00:00:00 2001 From: Liu Hua Date: Wed, 21 Apr 2021 20:08:08 +0800 Subject: [PATCH 2/2] checkpoint: validate parent path `--parent-path` needs to be relative path of `--image-path`, and points to previous checkpoint directory. Signed-off-by: Liu Hua --- checkpoint.go | 30 ++++++++++++++++++++++++++++-- restore.go | 7 ++++--- tests/integration/checkpoint.bats | 17 +++++++++++++++++ 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/checkpoint.go b/checkpoint.go index 3fe4541d777..c8c8e7458f9 100644 --- a/checkpoint.go +++ b/checkpoint.go @@ -7,6 +7,7 @@ import ( "fmt" "net" "os" + "path/filepath" "strconv" criu "github.com/checkpoint-restore/go-criu/v5/rpc" @@ -78,12 +79,37 @@ checkpointed.`, }, } -func getCheckpointImagePath(context *cli.Context) string { +func prepareImagePaths(context *cli.Context) (string, string, error) { imagePath := context.String("image-path") if imagePath == "" { imagePath = getDefaultImagePath(context) } - return imagePath + + if err := os.MkdirAll(imagePath, 0600); err != nil { + return "", "", err + } + + parentPath := context.String("parent-path") + if parentPath == "" { + return imagePath, parentPath, nil + } + + if filepath.IsAbs(parentPath) { + return "", "", errors.New("--parent-path must be relative") + } + + realParent := filepath.Join(imagePath, parentPath) + fi, err := os.Stat(realParent) + if err == nil && !fi.IsDir() { + err = &os.PathError{Path: realParent, Err: unix.ENOTDIR} + } + + if err != nil { + return "", "", fmt.Errorf("invalid --parent-path: %w", err) + } + + return imagePath, parentPath, nil + } func setPageServer(context *cli.Context, options *libcontainer.CriuOpts) { diff --git a/restore.go b/restore.go index ed69d15e784..586772b323b 100644 --- a/restore.go +++ b/restore.go @@ -121,14 +121,15 @@ using the runc checkpoint command.`, } func criuOptions(context *cli.Context) *libcontainer.CriuOpts { - imagePath := getCheckpointImagePath(context) - if err := os.MkdirAll(imagePath, 0755); err != nil { + imagePath, parentPath, err := prepareImagePaths(context) + if err != nil { fatal(err) } + return &libcontainer.CriuOpts{ ImagesDirectory: imagePath, WorkDirectory: context.String("work-path"), - ParentImage: context.String("parent-path"), + ParentImage: parentPath, LeaveRunning: context.Bool("leave-running"), TcpEstablished: context.Bool("tcp-established"), ExternalUnixConnections: context.Bool("ext-unix-sk"), diff --git a/tests/integration/checkpoint.bats b/tests/integration/checkpoint.bats index f168b1be112..3cf2fc7f928 100644 --- a/tests/integration/checkpoint.bats +++ b/tests/integration/checkpoint.bats @@ -144,6 +144,23 @@ function simple_cr() { simple_cr } +@test "checkpoint --pre-dump (bad --parent-path)" { + runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox + [ "$status" -eq 0 ] + + testcontainer test_busybox running + + # runc should fail with absolute parent image path. + runc --criu "$CRIU" checkpoint --parent-path "$(pwd)"/parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + [[ "${output}" == *"--parent-path"* ]] + [ "$status" -ne 0 ] + + # runc should fail with invalid parent image path. + runc --criu "$CRIU" checkpoint --parent-path ./parent-dir --work-path ./work-dir --image-path ./image-dir test_busybox + [[ "${output}" == *"--parent-path"* ]] + [ "$status" -ne 0 ] +} + @test "checkpoint --pre-dump and restore" { setup_pipes runc_run_with_pipes test_busybox