diff --git a/internal/guest/runtime/hcsv2/mount.go b/internal/guest/runtime/hcsv2/mount.go new file mode 100644 index 0000000000..3ccacc5aa2 --- /dev/null +++ b/internal/guest/runtime/hcsv2/mount.go @@ -0,0 +1,151 @@ +//go:build linux +// +build linux + +package hcsv2 + +import ( + "context" + "os" + "path/filepath" + "strings" + + oci "github.com/opencontainers/runtime-spec/specs-go" + + "github.com/Microsoft/hcsshim/internal/log" +) + +// ensureNestedMountTargets pre-creates the mount point for any mount whose +// destination is nested inside a read-only bind mount. +// +// runc applies the mounts in spec order and remounts a bind mount read-only as +// soon as it processes it. If a later mount targets a path inside that mount and +// the mount point does not already exist, runc tries to create it under the now +// read-only parent and fails with EROFS ("read-only file system"). This breaks +// valid CRI configs where a read-only volume has another volume mounted into a +// subdirectory of it (e.g. a read-only /etc/coredns configMap with a custom +// config volume at /etc/coredns/custom). +// +// On a regular Kubernetes node the kubelet creates these subdirectories on the +// host first. Inside an LCOW UVM the guest owns the mount setup, so we do the +// equivalent: create the mount point inside the parent's (writable) source, so +// it already exists once runc makes the parent read-only. +// +// Best effort: a failure is logged and skipped so runc's own error still +// surfaces and unaffected containers are unchanged. +func ensureNestedMountTargets(ctx context.Context, spec *oci.Spec) { + // runc applies mounts in spec order, so when it creates a mount point only + // the mounts before this one are active. The parent the mount point is + // created under is therefore the deepest ancestor among the preceding mounts. + for i, child := range spec.Mounts { + parent, ok := deepestParentMount(child.Destination, spec.Mounts[:i]) + if !ok || !mountIsReadonly(parent) || !mountIsBind(parent) { + // runc only fails when it must create the mount point under a + // read-only bind mount; otherwise it creates the target itself. + continue + } + if info, err := os.Stat(parent.Source); err != nil || !info.IsDir() { + continue + } + rel, err := filepath.Rel(parent.Destination, child.Destination) + if err != nil { + continue + } + target := filepath.Join(parent.Source, rel) + if err := createMountTarget(target, child.Source); err != nil { + log.G(ctx).WithError(err).WithField("target", target). + Warn("failed to pre-create mount point under read-only mount") + } + } +} + +// deepestParentMount returns the mount in mounts whose destination is the +// closest strict path ancestor of dest. When several are ancestors (stacked +// mounts) the one with the longest destination wins, which is where dest +// resolves to once those mounts are applied. Callers pass the mounts preceding +// dest in spec order, since those are the ones runc has already applied. +func deepestParentMount(dest string, mounts []oci.Mount) (oci.Mount, bool) { + cleanDest := filepath.Clean(dest) + var best oci.Mount + found := false + for _, m := range mounts { + p := filepath.Clean(m.Destination) + if !isStrictSubPath(p, cleanDest) { + continue + } + if !found || len(p) > len(filepath.Clean(best.Destination)) { + best = m + found = true + } + } + return best, found +} + +// isStrictSubPath reports whether target is strictly nested underneath base. +func isStrictSubPath(base, target string) bool { + base = filepath.Clean(base) + target = filepath.Clean(target) + if base == target { + return false + } + rel, err := filepath.Rel(base, target) + if err != nil { + return false + } + return rel != "." && rel != ".." && !strings.HasPrefix(rel, ".."+string(os.PathSeparator)) +} + +// mountIsReadonly reports whether the mount will be mounted read-only, honoring +// the last of any ro/rw options (which is how they resolve when both appear). +func mountIsReadonly(m oci.Mount) bool { + ro := false + for _, o := range m.Options { + switch o { + case "ro": + ro = true + case "rw": + ro = false + } + } + return ro +} + +// mountIsBind reports whether the mount is a bind mount. +func mountIsBind(m oci.Mount) bool { + if m.Type == "bind" { + return true + } + for _, o := range m.Options { + if o == "bind" || o == "rbind" { + return true + } + } + return false +} + +// createMountTarget creates the mountpoint at target. It mirrors runc's own +// behavior: if source is a non-directory the target is created as an empty file, +// otherwise it is created as a directory. Intermediate directories are created +// as needed, and an already-existing target is left untouched. +func createMountTarget(target, source string) error { + if _, err := os.Lstat(target); err == nil { + // Something already exists at the mountpoint (e.g. shipped in the image + // or the volume). Leave it as-is and let runc validate compatibility. + return nil + } + + sourceIsDir := true + if info, err := os.Stat(source); err == nil { + sourceIsDir = info.IsDir() + } + if sourceIsDir { + return mkdirAllModePerm(target) + } + if err := mkdirAllModePerm(filepath.Dir(target)); err != nil { + return err + } + f, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return err + } + return f.Close() +} diff --git a/internal/guest/runtime/hcsv2/mount_test.go b/internal/guest/runtime/hcsv2/mount_test.go new file mode 100644 index 0000000000..d6eab762b5 --- /dev/null +++ b/internal/guest/runtime/hcsv2/mount_test.go @@ -0,0 +1,342 @@ +//go:build linux +// +build linux + +package hcsv2 + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + oci "github.com/opencontainers/runtime-spec/specs-go" + "golang.org/x/sys/unix" +) + +func Test_isStrictSubPath(t *testing.T) { + for _, tc := range []struct { + base string + target string + want bool + }{ + {"/mnt/data", "/mnt/data/subdir", true}, + {"/mnt/data", "/mnt/data/a/b/c", true}, + {"/mnt/data", "/mnt/data", false}, + {"/mnt/data", "/mnt/database", false}, + {"/mnt/data", "/mnt", false}, + {"/mnt/data/", "/mnt/data/subdir/", true}, + {"/", "/etc", true}, + {"/mnt/data", "/other", false}, + } { + if got := isStrictSubPath(tc.base, tc.target); got != tc.want { + t.Errorf("isStrictSubPath(%q, %q) = %v, want %v", tc.base, tc.target, got, tc.want) + } + } +} + +func Test_mountIsReadonly(t *testing.T) { + for _, tc := range []struct { + name string + options []string + want bool + }{ + {"none", []string{"bind"}, false}, + {"ro", []string{"bind", "ro"}, true}, + {"rw", []string{"bind", "rw"}, false}, + {"ro_then_rw", []string{"ro", "rw"}, false}, + {"rw_then_ro", []string{"rw", "ro"}, true}, + {"empty", nil, false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := mountIsReadonly(oci.Mount{Options: tc.options}); got != tc.want { + t.Errorf("mountIsReadonly(%v) = %v, want %v", tc.options, got, tc.want) + } + }) + } +} + +func Test_mountIsBind(t *testing.T) { + for _, tc := range []struct { + name string + m oci.Mount + want bool + }{ + {"type_bind", oci.Mount{Type: "bind"}, true}, + {"opt_bind", oci.Mount{Options: []string{"bind"}}, true}, + {"opt_rbind", oci.Mount{Options: []string{"rbind"}}, true}, + {"tmpfs", oci.Mount{Type: "tmpfs", Source: "none"}, false}, + {"empty", oci.Mount{}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := mountIsBind(tc.m); got != tc.want { + t.Errorf("mountIsBind(%+v) = %v, want %v", tc.m, got, tc.want) + } + }) + } +} + +func Test_deepestParentMount(t *testing.T) { + mounts := []oci.Mount{ + {Destination: "/mnt/data"}, + {Destination: "/mnt/data/a"}, + {Destination: "/other"}, + } + for _, tc := range []struct { + name string + dest string + wantDest string + wantOK bool + }{ + {"direct_child", "/mnt/data/x", "/mnt/data", true}, + {"deepest_wins", "/mnt/data/a/b", "/mnt/data/a", true}, + {"self_excluded", "/mnt/data", "", false}, + {"no_parent", "/nope", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + got, ok := deepestParentMount(tc.dest, mounts) + if ok != tc.wantOK { + t.Fatalf("deepestParentMount(%q) ok = %v, want %v", tc.dest, ok, tc.wantOK) + } + if ok && got.Destination != tc.wantDest { + t.Errorf("deepestParentMount(%q) = %q, want %q", tc.dest, got.Destination, tc.wantDest) + } + }) + } +} + +// Test_ensureNestedMountTargets_ReadonlyParent verifies the reported bug is +// fixed: a volume mounted into a subdirectory of a read-only volume gets its +// mountpoint created inside the read-only parent's (writable) source. +func Test_ensureNestedMountTargets_ReadonlyParent(t *testing.T) { + parentSrc := t.TempDir() + childSrc := t.TempDir() + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + { + Destination: "/mnt/data", + Source: parentSrc, + Type: "bind", + Options: []string{"bind", "ro"}, + }, + { + Destination: "/mnt/data/subdir", + Source: childSrc, + Type: "bind", + Options: []string{"bind"}, + }, + }, + } + + ensureNestedMountTargets(context.Background(), spec) + + created := filepath.Join(parentSrc, "subdir") + info, err := os.Stat(created) + if err != nil { + t.Fatalf("expected mountpoint %q to be created: %v", created, err) + } + if !info.IsDir() { + t.Errorf("expected %q to be a directory", created) + } +} + +func Test_ensureNestedMountTargets_MultiLevelRelPath(t *testing.T) { + parentSrc := t.TempDir() + childSrc := t.TempDir() + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + { + Destination: "/etc/coredns", + Source: parentSrc, + Type: "bind", + Options: []string{"bind", "ro"}, + }, + { + Destination: "/etc/coredns/a/b/custom", + Source: childSrc, + Type: "bind", + Options: []string{"bind"}, + }, + }, + } + + ensureNestedMountTargets(context.Background(), spec) + + created := filepath.Join(parentSrc, "a", "b", "custom") + if info, err := os.Stat(created); err != nil { + t.Fatalf("expected nested mountpoint %q to be created: %v", created, err) + } else if !info.IsDir() { + t.Errorf("expected %q to be a directory", created) + } +} + +func Test_ensureNestedMountTargets_FileSourceCreatesFile(t *testing.T) { + parentSrc := t.TempDir() + childFile := filepath.Join(t.TempDir(), "config") + if err := os.WriteFile(childFile, []byte("x"), 0644); err != nil { + t.Fatal(err) + } + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + { + Destination: "/etc/coredns", + Source: parentSrc, + Type: "bind", + Options: []string{"bind", "ro"}, + }, + { + Destination: "/etc/coredns/Corefile", + Source: childFile, + Type: "bind", + Options: []string{"bind"}, + }, + }, + } + + ensureNestedMountTargets(context.Background(), spec) + + created := filepath.Join(parentSrc, "Corefile") + info, err := os.Stat(created) + if err != nil { + t.Fatalf("expected file mountpoint %q to be created: %v", created, err) + } + if info.IsDir() { + t.Errorf("expected %q to be a regular file, got directory", created) + } +} + +func Test_ensureNestedMountTargets_WritableParentNoOp(t *testing.T) { + parentSrc := t.TempDir() + childSrc := t.TempDir() + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + { + Destination: "/mnt/data", + Source: parentSrc, + Type: "bind", + Options: []string{"bind"}, // read-write parent + }, + { + Destination: "/mnt/data/subdir", + Source: childSrc, + Type: "bind", + Options: []string{"bind"}, + }, + }, + } + + ensureNestedMountTargets(context.Background(), spec) + + created := filepath.Join(parentSrc, "subdir") + if _, err := os.Stat(created); !os.IsNotExist(err) { + t.Errorf("expected no mountpoint to be created under writable parent, stat err = %v", err) + } +} + +func Test_ensureNestedMountTargets_UsesPrecedingMountsOnly(t *testing.T) { + mntSrc := t.TempDir() // source of the read-only /mnt + abSrc := t.TempDir() // source of /mnt/a/b + aSrc := t.TempDir() // source of /mnt/a, listed after its own child + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + {Destination: "/mnt", Source: mntSrc, Type: "bind", Options: []string{"bind", "ro"}}, + {Destination: "/mnt/a/b", Source: abSrc, Type: "bind", Options: []string{"bind"}}, + {Destination: "/mnt/a", Source: aSrc, Type: "bind", Options: []string{"bind"}}, + }, + } + + ensureNestedMountTargets(context.Background(), spec) + + // runc processes /mnt/a/b while only the read-only /mnt is mounted, so its + // mount point must be created under /mnt's source, not /mnt/a's. + if _, err := os.Stat(filepath.Join(mntSrc, "a", "b")); err != nil { + t.Fatalf("expected mount point under /mnt source: %v", err) + } + if entries, _ := os.ReadDir(aSrc); len(entries) != 0 { + t.Errorf("nothing should be created under /mnt/a source, found %d entries", len(entries)) + } +} + +// skipIfCannotMount skips the test unless the process can create bind mounts +// (root with CAP_SYS_ADMIN). This keeps the functional tests from failing in +// non-privileged or restricted CI environments while still running wherever +// mounts are available (e.g. the guest UVM or a privileged dev box). +func skipIfCannotMount(t *testing.T) { + t.Helper() + if os.Getuid() != 0 { + t.Skip("requires root to create bind mounts") + } + src, dst := t.TempDir(), t.TempDir() + if err := unix.Mount(src, dst, "", unix.MS_BIND, ""); err != nil { + t.Skipf("bind mounts not permitted in this environment: %v", err) + } + _ = unix.Unmount(dst, 0) +} + +// Test_readonlyBindMount_blocksMkdir is the control that reproduces the failure +// the fix addresses: once a bind mount is read-only, creating a mount point +// under it fails with EROFS. This is exactly what runc hits when it tries to +// create a nested mount's target under an already read-only parent. +func Test_readonlyBindMount_blocksMkdir(t *testing.T) { + skipIfCannotMount(t) + src := t.TempDir() + dst := t.TempDir() + if err := unix.Mount(src, dst, "", unix.MS_BIND, ""); err != nil { + t.Fatalf("bind mount: %v", err) + } + defer func() { _ = unix.Unmount(dst, 0) }() + if err := unix.Mount("", dst, "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, ""); err != nil { + t.Fatalf("remount read-only: %v", err) + } + + err := os.Mkdir(filepath.Join(dst, "subdir"), 0o755) + if !errors.Is(err, unix.EROFS) { + t.Fatalf("expected EROFS creating a dir under a read-only mount, got %v", err) + } +} + +// Test_ensureNestedMountTargets_Functional proves the fix end to end: after +// pre-creating the nested mount point in the parent's source, the mount point is +// visible once the parent is bind-mounted read-only, and a child can be mounted +// there without EROFS (which is what runc does). Requires root. +func Test_ensureNestedMountTargets_Functional(t *testing.T) { + skipIfCannotMount(t) + parentSrc := t.TempDir() + childSrc := t.TempDir() + parentDst := t.TempDir() + + spec := &oci.Spec{ + Mounts: []oci.Mount{ + {Destination: parentDst, Source: parentSrc, Type: "bind", Options: []string{"bind", "ro"}}, + {Destination: filepath.Join(parentDst, "subdir"), Source: childSrc, Type: "bind", Options: []string{"bind"}}, + }, + } + ensureNestedMountTargets(context.Background(), spec) + + // Bind-mount the parent source read-only, exactly as runc would. + if err := unix.Mount(parentSrc, parentDst, "", unix.MS_BIND, ""); err != nil { + t.Fatalf("bind mount parent: %v", err) + } + childMountPoint := filepath.Join(parentDst, "subdir") + defer func() { + _ = unix.Unmount(childMountPoint, 0) + _ = unix.Unmount(parentDst, 0) + }() + if err := unix.Mount("", parentDst, "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, ""); err != nil { + t.Fatalf("remount parent read-only: %v", err) + } + + // The pre-created mount point must be visible through the read-only parent... + if _, err := os.Stat(childMountPoint); err != nil { + t.Fatalf("nested mount point not visible under read-only parent: %v", err) + } + // ...and runc must be able to bind-mount the child there without EROFS. + if err := unix.Mount(childSrc, childMountPoint, "", unix.MS_BIND, ""); err != nil { + t.Fatalf("failed to bind-mount child under read-only parent: %v", err) + } +} diff --git a/internal/guest/runtime/hcsv2/uvm.go b/internal/guest/runtime/hcsv2/uvm.go index 0609966a0a..87ce71d067 100644 --- a/internal/guest/runtime/hcsv2/uvm.go +++ b/internal/guest/runtime/hcsv2/uvm.go @@ -744,6 +744,12 @@ func (h *Host) CreateContainer(ctx context.Context, id string, settings *prot.VM } } + // Pre-create mountpoints for mounts nested under a read-only mount so that + // runc does not have to create them under an already read-only parent + // (which fails with EROFS). Runs on the final mount set for all container + // types, just before the spec is handed to the runtime. + ensureNestedMountTargets(ctx, settings.OCISpecification) + // Create the BundlePath if err := os.MkdirAll(settings.OCIBundlePath, 0700); err != nil { return nil, errors.Wrapf(err, "failed to create OCIBundlePath: '%s'", settings.OCIBundlePath)