From 3efb58e91cb98922e54a8561280008cee26aeb66 Mon Sep 17 00:00:00 2001 From: Charalampos Mainas Date: Thu, 17 Sep 2026 16:06:21 +0200 Subject: [PATCH 1/3] fix(mount): recognise bind mounts spelled as type none with rbind An OCI mount is a bind mount when its options include "bind" or "rbind"; its Type is then a dummy that runtimes such as runc ignore. Docker sets it to "bind", but containerd and nerdctl emit volumes (`nerdctl run -v host:/data`) with Type "none". urunc only checked for Type "bind", so such volumes were dropped everywhere bind mounts are filtered: they never reached the shared rootfs of a virtiofs/9p guest, were not copied into a block rootfs or an initrd, and were not remounted with their VFS flags. Add isBindMount, which follows the runtime spec and looks only at the options, and use it in every such check (mount application, shared-fs and block filtering). The initrd package no longer filters mounts itself; its caller passes it only the bind mounts. Also a mount spelled only by its Type, e.g. Type "bind" without a bind option, is not a bind mount and is dropped like other unsupported mounts, so make the e2e ctr helper pass the rbind option that `ctr --mount type=bind` omits. PR: https://github.com/urunc-dev/urunc/pull/1150 Signed-off-by: Charalampos Mainas Reviewed-by: Anastassios Nanos Approved-by: Anastassios Nanos --- pkg/unikontainers/bind_mount_test.go | 50 +++++++++++++++++++++++++ pkg/unikontainers/block.go | 4 +- pkg/unikontainers/initrd/initrd.go | 10 ++--- pkg/unikontainers/initrd/initrd_test.go | 1 - pkg/unikontainers/initrd_rootfs.go | 6 ++- pkg/unikontainers/initrd_rootfs_test.go | 3 +- pkg/unikontainers/mount.go | 27 +++++++++++-- pkg/unikontainers/shared_fs.go | 2 +- tests/e2e/ctr.go | 2 +- 9 files changed, 87 insertions(+), 18 deletions(-) create mode 100644 pkg/unikontainers/bind_mount_test.go diff --git a/pkg/unikontainers/bind_mount_test.go b/pkg/unikontainers/bind_mount_test.go new file mode 100644 index 000000000..780a12e95 --- /dev/null +++ b/pkg/unikontainers/bind_mount_test.go @@ -0,0 +1,50 @@ +// Copyright (c) 2023-2026, Nubificus LTD +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package unikontainers + +import ( + "testing" + + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestIsBindMount(t *testing.T) { + t.Parallel() + + // Docker spells a volume as type bind with an rbind option. + assert.True(t, isBindMount(specs.Mount{Type: "bind", Options: []string{"rbind", "ro"}})) + // nerdctl and containerd spell a volume as type none with an rbind option. + assert.True(t, isBindMount(specs.Mount{Type: "none", Options: []string{"rbind", "rprivate", "rw"}})) + assert.True(t, isBindMount(specs.Mount{Options: []string{"bind"}})) + // The options alone decide: a bind or rbind Type without such an option is + // not a bind mount. + assert.False(t, isBindMount(specs.Mount{Type: "bind"})) + assert.False(t, isBindMount(specs.Mount{Type: "rbind", Options: []string{"rw"}})) + assert.False(t, isBindMount(specs.Mount{Type: "tmpfs", Options: []string{"nosuid"}})) + assert.False(t, isBindMount(specs.Mount{Type: "proc"})) + + // filterBindMounts keeps such volumes and re-roots them under the shared rootfs. + dir := t.TempDir() + mounts, err := filterBindMounts(dir, []specs.Mount{ + {Type: "none", Source: "/host/vol", Destination: "/data", Options: []string{"rbind", "rw"}}, + {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, + }) + require.NoError(t, err) + require.Len(t, mounts, 1) + assert.Equal(t, "/host/vol", mounts[0].Source) + assert.Equal(t, containerRootfsMountPath+"/data", mounts[0].Destination) +} diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index 309b6b25a..81e7881d9 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -172,7 +172,7 @@ func extractBootFiles(rootfsPath string, newRootfsPath string, unikernel string, func copyMountfiles(targetPath string, mounts []specs.Mount) error { for _, m := range mounts { - if m.Type != "bind" { + if !isBindMount(m) { continue } err := fileFromHost(targetPath, m.Source, m.Destination) @@ -212,7 +212,7 @@ func getBlockVolumes(mounts []specs.Mount, ukernel types.Unikernel) ([]types.Blo blkImgs := []types.BlockDevParams{} for i, m := range mounts { // We check only bind mounts - if m.Type != "bind" { + if !isBindMount(m) { continue } // Get the information of the source path diff --git a/pkg/unikontainers/initrd/initrd.go b/pkg/unikontainers/initrd/initrd.go index 71c67ad29..c6ea5dd8b 100644 --- a/pkg/unikontainers/initrd/initrd.go +++ b/pkg/unikontainers/initrd/initrd.go @@ -116,6 +116,8 @@ func copyFileToInitrdWithInode(w *cpio.Writer, srcFile string, destFile string, return nil } +// CopyFileMountsToInitrd appends the files of the given mounts to the initrd +// as a new archive member. The caller should pass only bind mounts. func CopyFileMountsToInitrd(oldInitrd string, mounts []specs.Mount) error { f, err := os.OpenFile(oldInitrd, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) if err != nil { @@ -125,9 +127,6 @@ func CopyFileMountsToInitrd(oldInitrd string, mounts []specs.Mount) error { w := cpio.NewWriter(f) for _, m := range mounts { - if m.Type != "bind" { - continue - } err = CopyFileToInitrd(w, m.Source, m.Destination) if err != nil { return fmt.Errorf("could not add file %s to initrd: %w", m.Source, err) @@ -143,7 +142,7 @@ func CopyFileMountsToInitrd(oldInitrd string, mounts []specs.Mount) error { // MergeFileMountsIntoInitrd adds regular-file bind mounts before the trailer // of an uncompressed newc archive. The archive is fully parsed before the -// disposable initrd is updated in place. +// disposable initrd is updated in place. The caller should pass only bind mounts. func MergeFileMountsIntoInitrd(oldInitrd string, mounts []specs.Mount) (retErr error) { fileMounts, err := regularFileBindMounts(mounts) if err != nil { @@ -208,9 +207,6 @@ func MergeFileMountsIntoInitrd(oldInitrd string, mounts []specs.Mount) (retErr e func regularFileBindMounts(mounts []specs.Mount) ([]specs.Mount, error) { var fileMounts []specs.Mount for _, mount := range mounts { - if mount.Type != "bind" { - continue - } info, err := os.Stat(mount.Source) if err != nil { return nil, fmt.Errorf("could not stat file %s: %w", mount.Source, err) diff --git a/pkg/unikontainers/initrd/initrd_test.go b/pkg/unikontainers/initrd/initrd_test.go index 426102a86..bfac37748 100644 --- a/pkg/unikontainers/initrd/initrd_test.go +++ b/pkg/unikontainers/initrd/initrd_test.go @@ -181,7 +181,6 @@ func TestMergeFileMountsIntoInitrdWithoutRegularBindMountsDoesNotRewrite(t *test require.NoError(t, err) require.NoError(t, MergeFileMountsIntoInitrd(initrdPath, []specs.Mount{ - {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, {Type: "bind", Source: dir, Destination: "/directory"}, })) diff --git a/pkg/unikontainers/initrd_rootfs.go b/pkg/unikontainers/initrd_rootfs.go index 7eaff1324..657ca13ac 100644 --- a/pkg/unikontainers/initrd_rootfs.go +++ b/pkg/unikontainers/initrd_rootfs.go @@ -44,10 +44,12 @@ func (i initrdRootfs) postSetup() error { return fmt.Errorf("failed to resolve guest's initrd path: %w", err) } + // The initrd package expects only bind mounts. + mounts := onlyBindMounts(i.mounts) if i.guestType == unikernels.UnikraftUnikernel { - err = initrd.MergeFileMountsIntoInitrd(initrdHostFullPath, i.mounts) + err = initrd.MergeFileMountsIntoInitrd(initrdHostFullPath, mounts) } else { - err = initrd.CopyFileMountsToInitrd(initrdHostFullPath, i.mounts) + err = initrd.CopyFileMountsToInitrd(initrdHostFullPath, mounts) } if err != nil { return fmt.Errorf("failed to update guest's initrd: %w", err) diff --git a/pkg/unikontainers/initrd_rootfs_test.go b/pkg/unikontainers/initrd_rootfs_test.go index 638f2d341..f3c2346ea 100644 --- a/pkg/unikontainers/initrd_rootfs_test.go +++ b/pkg/unikontainers/initrd_rootfs_test.go @@ -67,7 +67,8 @@ func TestInitrdRootfsPostSetupSelectsUpdateByGuestType(t *testing.T) { initrdPath: "initrd.cpio", guestType: tt.guestType, mounts: []specs.Mount{ - {Type: "bind", Source: source, Destination: "/mounted"}, + {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, + {Type: "none", Source: source, Destination: "/mounted", Options: []string{"rbind", "rw"}}, }, } diff --git a/pkg/unikontainers/mount.go b/pkg/unikontainers/mount.go index 6b9eaeecc..d88f400e8 100644 --- a/pkg/unikontainers/mount.go +++ b/pkg/unikontainers/mount.go @@ -389,7 +389,7 @@ func applyMount(rootfsPath string, m specs.Mount) error { // containerd's parser cannot translate propagation tokens, and the kernel // ignores per-mount VFS flags (nosuid, nodev, ...) when a bind is created, so // parse the options once and apply them manually later. - containerdOpts, propagation, vfsFlags := splitMountOptions(m.Options, m.Type == "bind") + containerdOpts, propagation, vfsFlags := splitMountOptions(m.Options, isBindMount(m)) cm := mount.Mount{ Type: m.Type, @@ -405,7 +405,7 @@ func applyMount(rootfsPath string, m specs.Mount) error { // them with a remount. In a user namespace the remount must preserve the // flags locked on the source mount, otherwise the kernel rejects it with // EPERM (this is what containerd's own bind remount does for us elsewhere). - if m.Type == "bind" && vfsFlags != 0 { + if isBindMount(m) && vfsFlags != 0 { remount := vfsFlags | unix.MS_BIND | unix.MS_REMOUNT if userns.RunningInUserNS() { locked, err := getUnprivilegedMountFlags(m.Source) @@ -441,9 +441,30 @@ func applyMount(rootfsPath string, m specs.Mount) error { return nil } +// isBindMount reports whether m is a bind mount. As the OCI runtime spec +// defines it, a mount is a bind mount if its options include "bind" or +// "rbind". Its Type is then a dummy and is ignored, as runc does. +// See the Mounts section of runtime-spec v1.2.1: +// https://github.com/opencontainers/runtime-spec/blob/524fc0e1b8ab0180e2fc9abd31837a0f4ed1fd6b/config.md#configMounts +func isBindMount(m specs.Mount) bool { + return slices.Contains(m.Options, "bind") || slices.Contains(m.Options, "rbind") +} + +// onlyBindMounts returns the bind mounts among mounts, as recognized by +// isBindMount. +func onlyBindMounts(mounts []specs.Mount) []specs.Mount { + var result []specs.Mount + for _, m := range mounts { + if isBindMount(m) { + result = append(result, m) + } + } + return result +} + // createMountPoint creates the mountpoint of a mount. func createMountPoint(target string, m specs.Mount) error { - if m.Type == "bind" { + if isBindMount(m) { var st unix.Stat_t err := unix.Stat(m.Source, &st) if err != nil { diff --git a/pkg/unikontainers/shared_fs.go b/pkg/unikontainers/shared_fs.go index 09449605b..37d2a0d1b 100644 --- a/pkg/unikontainers/shared_fs.go +++ b/pkg/unikontainers/shared_fs.go @@ -123,7 +123,7 @@ func filterBindMounts(containerRootfs string, mounts []specs.Mount) ([]specs.Mou for _, m := range mounts { // Skip non-bind mounts // TODO handle other types of mounts too - if m.Type != "bind" { + if !isBindMount(m) { continue } diff --git a/tests/e2e/ctr.go b/tests/e2e/ctr.go index 627a5c498..7998a5270 100644 --- a/tests/e2e/ctr.go +++ b/tests/e2e/ctr.go @@ -51,7 +51,7 @@ func ctrNewContainerCmd(a containerTestArgs) string { cmdBase += fmt.Sprintf("-u %d:%d ", a.UID, a.GID) } for _, vol := range a.Volumes { - cmdBase += fmt.Sprintf("--mount type=bind,src=%s,dst=%s ", vol.Source, vol.Dest) + cmdBase += fmt.Sprintf("--mount type=bind,src=%s,dst=%s,options=rbind ", vol.Source, vol.Dest) } cmdBase += a.Image + " " cmdBase += a.Name + " " From 6e2acdd1d9ab704191061969428112a16a0e6e5b Mon Sep 17 00:00:00 2001 From: Charalampos Mainas Date: Thu, 17 Sep 2026 18:06:21 +0200 Subject: [PATCH 2/3] fix(mount): do not pass host kernel filesystems to the guest Rootless runtimes replace the container's sysfs mount with an rbind of the host's /sys, e.g. with --net=host, since they cannot mount a fresh sysfs. Now that such Type "none" binds are recognised, this mount would end up in the rootfs shared with a virtiofs/9p guest, exposing the host's sysfs to it and, through the file server, letting the guest write host kernel knobs. On the block and initrd paths, single-file binds from /proc or /sys would also copy host state into the guest. Add onlyBindMounts as the single place that selects the spec mounts passed to the guest: the bind mounts, minus those whose source resides on a host kernel filesystem (sysfs, procfs, cgroupfs, securityfs, debugfs, tracefs, bpffs, efivarfs, pstore, binfmt_misc, nsfs, configfs, fusectl, mqueue, selinuxfs and smackfs), detected through statfs. Only the source's own filesystem is checked, not what an rbind brings along below it. Skipped mounts are logged. Use it for the shared-fs, initrd and block (file copies and volumes) paths. The guest has its own kernel and its own instances of these filesystems. newRootfsBuilder calls it once and hands the result to the shared-fs, initrd and block (file copies and volumes) paths, which no longer filter on their own. filterBindMounts now only re-roots the mounts, so rename it to rebaseBindMounts. A bind mount whose source cannot be statfs'd, e.g. because it does not exist, now fails container creation on every path. So does a mount with Type "bind" or "rbind" but neither option: it is not a bind mount and runc rejects it too, while dropping it would start the container without the volume it asked for. PR: https://github.com/urunc-dev/urunc/pull/1150 Signed-off-by: Charalampos Mainas Reviewed-by: Anastassios Nanos Approved-by: Anastassios Nanos --- .github/linters/urunc-dict.txt | 12 ++++ pkg/unikontainers/bind_mount_test.go | 68 ++++++++++++++++++++-- pkg/unikontainers/block.go | 16 ++--- pkg/unikontainers/initrd_rootfs.go | 7 +-- pkg/unikontainers/initrd_rootfs_test.go | 50 +++++++++++++++- pkg/unikontainers/mount.go | 77 +++++++++++++++++++++++-- pkg/unikontainers/shared_fs.go | 16 ++--- pkg/unikontainers/unikontainers.go | 34 ++++++++--- 8 files changed, 234 insertions(+), 46 deletions(-) diff --git a/.github/linters/urunc-dict.txt b/.github/linters/urunc-dict.txt index b9b062e0c..2f2e1b0be 100644 --- a/.github/linters/urunc-dict.txt +++ b/.github/linters/urunc-dict.txt @@ -34,6 +34,18 @@ LOGPIPE MANDLOCK Mainas Mewz +cgroupfs +SECURITYFS +DEBUGFS +TRACEFS +EFIVARFS +PSTOREFS +BINFMTFS +NSFS +configfs +fusectl +mqueue +uapi Mirred Mkdev Mountpoint diff --git a/pkg/unikontainers/bind_mount_test.go b/pkg/unikontainers/bind_mount_test.go index 780a12e95..626126cda 100644 --- a/pkg/unikontainers/bind_mount_test.go +++ b/pkg/unikontainers/bind_mount_test.go @@ -15,6 +15,8 @@ package unikontainers import ( + "os" + "path/filepath" "testing" "github.com/opencontainers/runtime-spec/specs-go" @@ -37,14 +39,70 @@ func TestIsBindMount(t *testing.T) { assert.False(t, isBindMount(specs.Mount{Type: "tmpfs", Options: []string{"nosuid"}})) assert.False(t, isBindMount(specs.Mount{Type: "proc"})) - // filterBindMounts keeps such volumes and re-roots them under the shared rootfs. + // rebaseBindMounts re-roots such volumes under the shared rootfs. dir := t.TempDir() - mounts, err := filterBindMounts(dir, []specs.Mount{ - {Type: "none", Source: "/host/vol", Destination: "/data", Options: []string{"rbind", "rw"}}, - {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, + vol := t.TempDir() + mounts, err := rebaseBindMounts(dir, []specs.Mount{ + {Type: "none", Source: vol, Destination: "/data", Options: []string{"rbind", "rw"}}, }) require.NoError(t, err) require.Len(t, mounts, 1) - assert.Equal(t, "/host/vol", mounts[0].Source) + assert.Equal(t, vol, mounts[0].Source) assert.Equal(t, containerRootfsMountPath+"/data", mounts[0].Destination) } + +func TestOnlyBindMounts(t *testing.T) { + t.Parallel() + + vol := t.TempDir() + file := filepath.Join(vol, "file") + require.NoError(t, os.WriteFile(file, []byte("data"), 0o600)) + + mounts := []specs.Mount{ + {Type: "bind", Source: vol, Destination: "/data", Options: []string{"rbind"}}, + {Type: "none", Source: file, Destination: "/etc/file", Options: []string{"bind", "ro"}}, + {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, + {Type: "sysfs", Source: "sysfs", Destination: "/sys"}, + // Rootless runtimes emit an rbind of the host's /sys in place of sysfs. + {Type: "none", Source: "/sys", Destination: "/sys", Options: []string{"rbind", "nosuid", "noexec", "nodev", "ro"}}, + {Type: "bind", Source: "/proc", Destination: "/host/proc", Options: []string{"rbind"}}, + {Type: "bind", Source: "/proc/cpuinfo", Destination: "/cpuinfo", Options: []string{"bind"}}, + } + // Kernel filesystems mounted on their own, e.g. cgroupfs under /sys, when + // this host has them. + for _, kfs := range []struct { + path string + magic int64 + }{ + {"/sys/fs/cgroup", unix.CGROUP2_SUPER_MAGIC}, + {"/sys/kernel/config", configfsMagic}, + {"/sys/fs/fuse/connections", fusectlMagic}, + {"/dev/mqueue", mqueueMagic}, + {"/sys/fs/selinux", unix.SELINUX_MAGIC}, + } { + var st unix.Statfs_t + if unix.Statfs(kfs.path, &st) == nil && int64(st.Type) == kfs.magic { + mounts = append(mounts, specs.Mount{Type: "bind", Source: kfs.path, Destination: "/host" + kfs.path, Options: []string{"rbind"}}) + } + } + + result, err := onlyBindMounts(mounts) + require.NoError(t, err) + require.Len(t, result, 2) + assert.Equal(t, vol, result[0].Source) + assert.Equal(t, file, result[1].Source) + + _, err = onlyBindMounts([]specs.Mount{ + {Type: "bind", Source: filepath.Join(vol, "missing"), Destination: "/missing", Options: []string{"rbind"}}, + }) + assert.Error(t, err) + + // A bind or rbind Type without such an option, as `ctr run --mount + // type=bind,...` emits, is an error rather than a silently dropped volume. + for _, typ := range []string{"bind", "rbind"} { + _, err = onlyBindMounts([]specs.Mount{ + {Type: typ, Source: vol, Destination: "/data", Options: []string{"rw"}}, + }) + assert.ErrorContains(t, err, "no bind or rbind option") + } +} diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index 81e7881d9..3c3aa58ab 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -170,12 +170,12 @@ func extractBootFiles(rootfsPath string, newRootfsPath string, unikernel string, return nil } +// copyMountfiles copies the sources of the bind mounts passed to the guest, as +// selected by onlyBindMounts, into targetPath. func copyMountfiles(targetPath string, mounts []specs.Mount) error { + var err error for _, m := range mounts { - if !isBindMount(m) { - continue - } - err := fileFromHost(targetPath, m.Source, m.Destination) + err = fileFromHost(targetPath, m.Source, m.Destination) if (err != nil) && !errors.Is(err, ErrCopyDir) { return err } @@ -206,15 +206,11 @@ func handleExplicitBlockImage(blockImg string, mountPoint string) (types.BlockDe }, nil } -// Search all the mount entries in the container's config and -// find the ones that come from a block. +// Search the bind mounts passed to the guest, as selected by onlyBindMounts, +// and find the ones that come from a block. func getBlockVolumes(mounts []specs.Mount, ukernel types.Unikernel) ([]types.BlockDevParams, error) { blkImgs := []types.BlockDevParams{} for i, m := range mounts { - // We check only bind mounts - if !isBindMount(m) { - continue - } // Get the information of the source path // from /proc/thread-self/mountinfo mInfo, err := getMountInfo(m.Source) diff --git a/pkg/unikontainers/initrd_rootfs.go b/pkg/unikontainers/initrd_rootfs.go index 657ca13ac..06ce5080b 100644 --- a/pkg/unikontainers/initrd_rootfs.go +++ b/pkg/unikontainers/initrd_rootfs.go @@ -44,12 +44,11 @@ func (i initrdRootfs) postSetup() error { return fmt.Errorf("failed to resolve guest's initrd path: %w", err) } - // The initrd package expects only bind mounts. - mounts := onlyBindMounts(i.mounts) + // i.mounts holds only bind mounts, as the initrd package expects. if i.guestType == unikernels.UnikraftUnikernel { - err = initrd.MergeFileMountsIntoInitrd(initrdHostFullPath, mounts) + err = initrd.MergeFileMountsIntoInitrd(initrdHostFullPath, i.mounts) } else { - err = initrd.CopyFileMountsToInitrd(initrdHostFullPath, mounts) + err = initrd.CopyFileMountsToInitrd(initrdHostFullPath, i.mounts) } if err != nil { return fmt.Errorf("failed to update guest's initrd: %w", err) diff --git a/pkg/unikontainers/initrd_rootfs_test.go b/pkg/unikontainers/initrd_rootfs_test.go index f3c2346ea..120a59556 100644 --- a/pkg/unikontainers/initrd_rootfs_test.go +++ b/pkg/unikontainers/initrd_rootfs_test.go @@ -34,12 +34,59 @@ func TestNewRootfsBuilderPassesGuestTypeToInitrd(t *testing.T) { Spec: &specs.Spec{}, } - builder := u.newRootfsBuilder("", types.RootfsParams{Type: "initrd"}, nil, "", "", 0) + builder, err := u.newRootfsBuilder("", types.RootfsParams{Type: "initrd"}, nil, "", "", 0) + require.NoError(t, err) rfs, ok := builder.(initrdRootfs) require.True(t, ok) require.Equal(t, unikernels.UnikraftUnikernel, rfs.guestType) } +func TestNewRootfsBuilderSelectsBindMounts(t *testing.T) { + t.Parallel() + + vol := t.TempDir() + u := &Unikontainer{ + State: &specs.State{Annotations: map[string]string{}}, + UruncCfg: defaultUruncConfig(), + Spec: &specs.Spec{Mounts: []specs.Mount{ + {Type: "bind", Source: vol, Destination: "/data", Options: []string{"rbind"}}, + {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, + {Type: "none", Source: "/sys", Destination: "/sys", Options: []string{"rbind", "ro"}}, + }}, + } + guestRootfs := []string{"block", "initrd", "virtiofs", "9pfs"} + + // Each guest rootfs gets the bind mounts passed to the guest, selected once. + for _, typ := range guestRootfs { + builder, err := u.newRootfsBuilder("", types.RootfsParams{Type: typ}, nil, "", "", 0) + require.NoError(t, err) + var mounts []specs.Mount + switch rfs := builder.(type) { + case blockRootfs: + mounts = rfs.mounts + case initrdRootfs: + mounts = rfs.mounts + case sharedfsRootfs: + mounts = rfs.mounts + default: + t.Fatalf("unexpected builder %T for %q", builder, typ) + } + require.Len(t, mounts, 1, typ) + require.Equal(t, vol, mounts[0].Source, typ) + } + + // The mounts are selected only for a guest rootfs, so a bind or rbind Type + // without such an option fails only there. + u.Spec.Mounts = []specs.Mount{{Type: "bind", Source: vol, Destination: "/data"}} + for _, typ := range guestRootfs { + _, err := u.newRootfsBuilder("", types.RootfsParams{Type: typ}, nil, "", "", 0) + require.ErrorContains(t, err, "no bind or rbind option", typ) + } + builder, err := u.newRootfsBuilder("", types.RootfsParams{}, nil, "", "", 0) + require.NoError(t, err) + require.IsType(t, noRootfs{}, builder) +} + func TestInitrdRootfsPostSetupSelectsUpdateByGuestType(t *testing.T) { tests := []struct { name string @@ -67,7 +114,6 @@ func TestInitrdRootfsPostSetupSelectsUpdateByGuestType(t *testing.T) { initrdPath: "initrd.cpio", guestType: tt.guestType, mounts: []specs.Mount{ - {Type: "tmpfs", Source: "tmpfs", Destination: "/tmp"}, {Type: "none", Source: source, Destination: "/mounted", Options: []string{"rbind", "rw"}}, }, } diff --git a/pkg/unikontainers/mount.go b/pkg/unikontainers/mount.go index d88f400e8..cc817d9d2 100644 --- a/pkg/unikontainers/mount.go +++ b/pkg/unikontainers/mount.go @@ -450,16 +450,83 @@ func isBindMount(m specs.Mount) bool { return slices.Contains(m.Options, "bind") || slices.Contains(m.Options, "rbind") } +// Magic numbers of host kernel filesystems that golang.org/x/sys/unix does not +// define, from the kernel's include/uapi/linux/magic.h. +const ( + configfsMagic = 0x62656570 + fusectlMagic = 0x65735543 + mqueueMagic = 0x19800202 +) + +// isHostKernelFS reports whether path resides on one of the filesystems +// through which the host kernel exposes its state and configuration (sysfs, +// procfs, cgroupfs, ...). +func isHostKernelFS(path string) (bool, error) { + var st unix.Statfs_t + err := unix.Statfs(path, &st) + if err != nil { + return false, fmt.Errorf("failed to statfs %s: %w", path, err) + } + + // Statfs_t.Type is an int64 on amd64 and arm64; the conversion only + // matters on architectures where it is narrower. + switch int64(st.Type) { + case unix.SYSFS_MAGIC, + unix.PROC_SUPER_MAGIC, + unix.CGROUP_SUPER_MAGIC, + unix.CGROUP2_SUPER_MAGIC, + unix.SECURITYFS_MAGIC, + unix.DEBUGFS_MAGIC, + unix.TRACEFS_MAGIC, + unix.BPF_FS_MAGIC, + unix.EFIVARFS_MAGIC, + unix.PSTOREFS_MAGIC, + unix.BINFMTFS_MAGIC, + unix.NSFS_MAGIC, + unix.SELINUX_MAGIC, + unix.SMACK_MAGIC, + configfsMagic, + fusectlMagic, + mqueueMagic: + return true, nil + } + + return false, nil +} + // onlyBindMounts returns the bind mounts among mounts, as recognized by -// isBindMount. -func onlyBindMounts(mounts []specs.Mount) []specs.Mount { +// isBindMount, which urunc passes to the guest. It leaves out bind mounts +// whose source resides on a host kernel filesystem, which the guest should not +// have access to. The guest has its own kernel and its own instances of these +// filesystems. Only the filesystem of the source itself is checked: kernel +// filesystems mounted below it, which an rbind brings along (e.g. /proc and +// /sys under -v /:/host), are kept, so this is not a boundary against host +// paths a mount explicitly asks for. A mount with Type "bind" or "rbind" but +// neither option is an error, as it is for runc. +func onlyBindMounts(mounts []specs.Mount) ([]specs.Mount, error) { var result []specs.Mount for _, m := range mounts { - if isBindMount(m) { - result = append(result, m) + if !isBindMount(m) { + // TODO: Reject such mounts once, while validating the spec in + // New(), so that every rootfs type does, without another loop + // over the mounts. + if m.Type == "bind" || m.Type == "rbind" { + return nil, fmt.Errorf("mount of %s to %s has type %q but no bind or rbind option", m.Source, m.Destination, m.Type) + } + continue + } + kernelFS, err := isHostKernelFS(m.Source) + if err != nil { + return nil, err } + if kernelFS { + uniklog.Warnf("skipping bind mount of %s to %s: its source is a host kernel filesystem", m.Source, m.Destination) + continue + } + result = append(result, m) } - return result + + return result, nil } // createMountPoint creates the mountpoint of a mount. diff --git a/pkg/unikontainers/shared_fs.go b/pkg/unikontainers/shared_fs.go index 37d2a0d1b..33923cc34 100644 --- a/pkg/unikontainers/shared_fs.go +++ b/pkg/unikontainers/shared_fs.go @@ -57,7 +57,7 @@ func (s sharedfsRootfs) getMounts() ([]specs.Mount, error) { tmpfsSize := chooseTmpfsSize(s.sfsType, s.memory) mounts = append(mounts, tmpfsMount("/tmp", tmpfsSize)) - bindMounts, err := filterBindMounts(s.mountedPath, s.mounts) + bindMounts, err := rebaseBindMounts(s.mountedPath, s.mounts) if err != nil { return nil, err } @@ -115,18 +115,12 @@ func chooseTmpfsSize(sfsType string, mem uint64) string { return tmpMountMemStr } -// filterBindMounts filters the mounts form the container's spec keeping only the -// bind mounts and adjusts the Destination path to the mountpoint of the -// container's rootfs inside the monitor rootfs. -func filterBindMounts(containerRootfs string, mounts []specs.Mount) ([]specs.Mount, error) { +// rebaseBindMounts adjusts the Destination path of the bind mounts passed to the +// guest, as selected by onlyBindMounts, to the mountpoint of the container's +// rootfs inside the monitor rootfs. +func rebaseBindMounts(containerRootfs string, mounts []specs.Mount) ([]specs.Mount, error) { var result []specs.Mount for _, m := range mounts { - // Skip non-bind mounts - // TODO handle other types of mounts too - if !isBindMount(m) { - continue - } - // Resolve the destination against the real container rootfs, // and then re-root the resolved path under the container // rootfs mount inside the monitor rootfs, where the mount is diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index 27efaf6bd..fd998d23a 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -214,7 +214,10 @@ func (u *Unikontainer) InitialSetup() error { defaultMemSizeMB := u.UruncCfg.Monitors[vmmType].DefaultMemoryMB memory := monitorMemoryBytes(defaultMemSizeMB, u.Spec.Linux.Resources) - rfsBuilder := u.newRootfsBuilder(rootfsDir, rootfsParams, unikernel, unikernelPath, initrdPath, memory) + rfsBuilder, err := u.newRootfsBuilder(rootfsDir, rootfsParams, unikernel, unikernelPath, initrdPath, memory) + if err != nil { + return err + } err = rfsBuilder.preSetup() if err != nil { @@ -481,11 +484,24 @@ func getMonitorResources(rfs rootfsBuilder, rootfsParams types.RootfsParams, vmm // because MountedPath is empty and this is the only way to reach the container // rootfs that holds the image and the boot files. // TODO: Find a better way for the explicit block image case to reduce arguments. -func (u *Unikontainer) newRootfsBuilder(containerRootfs string, rootfsParams types.RootfsParams, unikernel types.Unikernel, unikernelPath string, initrdPath string, memory uint64) rootfsBuilder { +func (u *Unikontainer) newRootfsBuilder(containerRootfs string, rootfsParams types.RootfsParams, unikernel types.Unikernel, unikernelPath string, initrdPath string, memory uint64) (rootfsBuilder, error) { + // Select the bind mounts passed to the guest once, for every step of the + // rootfs setup. Without a guest rootfs, no mount is passed to the guest. + // TODO handle other types of mounts too + var mounts []specs.Mount + switch rootfsParams.Type { + case "block", "initrd", "virtiofs", "9pfs": + var err error + mounts, err = onlyBindMounts(u.Spec.Mounts) + if err != nil { + return nil, err + } + } + switch rootfsParams.Type { case "block": return blockRootfs{ - mounts: u.Spec.Mounts, + mounts: mounts, monRootfs: rootfsParams.MonRootfs, mountedPath: rootfsParams.MountedPath, containerRootfs: containerRootfs, @@ -495,29 +511,29 @@ func (u *Unikontainer) newRootfsBuilder(containerRootfs string, rootfsParams typ uruncJSONPath: uruncJSONFilename, guestType: u.State.Annotations[annotType], guest: unikernel, - } + }, nil case "initrd": return initrdRootfs{ - mounts: u.Spec.Mounts, + mounts: mounts, mountedPath: rootfsParams.MountedPath, initrdPath: rootfsParams.Path, guestType: u.State.Annotations[annotType], - } + }, nil case "virtiofs", "9pfs": return sharedfsRootfs{ - mounts: u.Spec.Mounts, + mounts: mounts, mountedPath: rootfsParams.MountedPath, sfsType: rootfsParams.Type, vfsdConfig: u.UruncCfg.ExtraBins["virtiofsd"], sharedPath: containerRootfsMountPath, memory: memory, - } + }, nil default: return noRootfs{ containerRootfsPath: rootfsParams.MountedPath, annotBlockPath: u.State.Annotations[annotBlock], annotBlockMountPoint: u.State.Annotations[annotBlockMntPoint], - } + }, nil } } From d544628d387bf0daadf10d65f63bd4efff5eb9d0 Mon Sep 17 00:00:00 2001 From: Charalampos Mainas Date: Thu, 17 Sep 2026 19:07:11 +0200 Subject: [PATCH 3/3] fix(mount): copy only regular files from bind mounts into the rootfs For a block rootfs, copyMountfiles copies the source of each bind mount into the rootfs with fileFromHost, which only refused directories. Any other special file broke container creation: opening a socket fails, opening a FIFO blocks forever and reading a device such as /dev/urandom never ends, filling the rootfs. Replace ErrCopyDir with ErrNotRegularFile, which fileFromHost returns for anything that is not a regular file, before opening it, and skip such mounts in copyMountfiles as directories already were. This matches the initrd path, which already copies only regular files. PR: https://github.com/urunc-dev/urunc/pull/1150 Signed-off-by: Charalampos Mainas Reviewed-by: Anastassios Nanos Approved-by: Anastassios Nanos --- pkg/unikontainers/bind_mount_test.go | 29 ++++++++++++++++++++++++++++ pkg/unikontainers/block.go | 2 +- pkg/unikontainers/mount.go | 13 ++++++++----- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/pkg/unikontainers/bind_mount_test.go b/pkg/unikontainers/bind_mount_test.go index 626126cda..424f41242 100644 --- a/pkg/unikontainers/bind_mount_test.go +++ b/pkg/unikontainers/bind_mount_test.go @@ -22,6 +22,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" ) func TestIsBindMount(t *testing.T) { @@ -106,3 +107,31 @@ func TestOnlyBindMounts(t *testing.T) { assert.ErrorContains(t, err, "no bind or rbind option") } } + +func TestCopyMountfilesSkipsSpecialFiles(t *testing.T) { + t.Parallel() + + src := t.TempDir() + file := filepath.Join(src, "file") + require.NoError(t, os.WriteFile(file, []byte("data"), 0o600)) + fifo := filepath.Join(src, "fifo") + require.NoError(t, unix.Mkfifo(fifo, 0o600)) + + assert.ErrorIs(t, fileFromHost(t.TempDir(), fifo, "/fifo"), ErrNotRegularFile) + assert.ErrorIs(t, fileFromHost(t.TempDir(), "/dev/null", "/dev/null"), ErrNotRegularFile) + assert.ErrorIs(t, fileFromHost(t.TempDir(), src, "/data"), ErrNotRegularFile) + + target := t.TempDir() + require.NoError(t, copyMountfiles(target, []specs.Mount{ + {Type: "none", Source: file, Destination: "/etc/file", Options: []string{"rbind"}}, + {Type: "none", Source: fifo, Destination: "/run/fifo", Options: []string{"rbind"}}, + {Type: "bind", Source: "/dev/null", Destination: "/dev/null", Options: []string{"rbind"}}, + {Type: "none", Source: src, Destination: "/data", Options: []string{"rbind"}}, + })) + + content, err := os.ReadFile(filepath.Join(target, "etc/file")) + require.NoError(t, err) + assert.Equal(t, "data", string(content)) + assert.NoFileExists(t, filepath.Join(target, "run/fifo")) + assert.NoFileExists(t, filepath.Join(target, "dev/null")) +} diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index 3c3aa58ab..bedda38f4 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -176,7 +176,7 @@ func copyMountfiles(targetPath string, mounts []specs.Mount) error { var err error for _, m := range mounts { err = fileFromHost(targetPath, m.Source, m.Destination) - if (err != nil) && !errors.Is(err, ErrCopyDir) { + if (err != nil) && !errors.Is(err, ErrNotRegularFile) { return err } } diff --git a/pkg/unikontainers/mount.go b/pkg/unikontainers/mount.go index cc817d9d2..b0a8f036a 100644 --- a/pkg/unikontainers/mount.go +++ b/pkg/unikontainers/mount.go @@ -35,7 +35,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" ) -var ErrCopyDir = errors.New("can not copy a directory") +var ErrNotRegularFile = errors.New("not a regular file") // setupDevices creates every device from the list inside monitor's rootfs func setupDevices(monRootfs string, devices []specs.LinuxDevice, needsTAP bool) error { @@ -138,8 +138,8 @@ func setupDev(monRootfs string, dev specs.LinuxDevice) error { // is placed at the same path it has on the host. // A copy is preferred for monitor binaries: none of the monitor processes share // memory with other processes of the same monitor, at the cost of being slower -// and consuming more space. Copying a directory is not supported and returns -// ErrCopyDir. +// and consuming more space. Only regular files can be copied; for anything +// else (a directory, socket, FIFO or device node) it returns ErrNotRegularFile. func fileFromHost(monRootfs string, hostPath string, target string) error { // Get the info of the original file var fileInfo unix.Stat_t @@ -148,8 +148,11 @@ func fileFromHost(monRootfs string, hostPath string, target string) error { return err } - if (fileInfo.Mode & unix.S_IFMT) == unix.S_IFDIR { - return ErrCopyDir + // Only a regular file has content to copy: a directory would need a + // recursive copy, opening a FIFO blocks, opening a socket fails with ENXIO + // and reading a device like /dev/urandom never ends. + if (fileInfo.Mode & unix.S_IFMT) != unix.S_IFREG { + return ErrNotRegularFile } if target == "" {