From b9d21371c6c72a69c6e19821d66b4f4b1472de10 Mon Sep 17 00:00:00 2001 From: Jie Chen Date: Fri, 31 Jul 2026 15:32:11 -0700 Subject: [PATCH] guest: pre-create nested mount points under read-only mounts runc applies the OCI mounts in spec order and remounts a bind mount read-only as soon as it processes it. When it then processes a child mount whose destination is nested inside that read-only mount and the mount point does not already exist, runc must create it under the now read-only parent and fails with EROFS. This breaks valid configurations where a read-only volume has another volume mounted into a subdirectory of it, for example 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 before handing the spec to the runtime. Inside an LCOW UVM the guest owns the mount setup, so do the equivalent: for each nested mount, create the mount point inside the writable source of its read-only parent before the spec is handed to runc, so it already exists once the parent is made read-only. This is best effort; if a mount point cannot be pre-created it is logged and skipped so runc's own error still surfaces. Signed-off-by: Jie Chen --- internal/guest/runtime/hcsv2/mount.go | 151 +++++++++ internal/guest/runtime/hcsv2/mount_test.go | 342 +++++++++++++++++++++ internal/guest/runtime/hcsv2/uvm.go | 6 + 3 files changed, 499 insertions(+) create mode 100644 internal/guest/runtime/hcsv2/mount.go create mode 100644 internal/guest/runtime/hcsv2/mount_test.go 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)