From 9a6104203b672cb2b3849314810109d6ab5a8f23 Mon Sep 17 00:00:00 2001 From: Charalampos Mainas Date: Mon, 20 Jul 2026 18:42:06 +0000 Subject: [PATCH] refactor(mounts): Use containerd's mount package for the mounts Replace the manual handling of mount options and mount calls with containerd's mount package, and use securejoin to resolve mount targets and copy destinations inside the monitor rootfs, so that symlinks in image-controlled paths cannot redirect them outside of it. containerd's option parser cannot translate propagation flags and the kernel ignores per-mount VFS flags when a bind mount is created. Therefore, split the mount options into three groups: the options forwarded to containerd, the propagation flags and the per-mount VFS flags. For bind mounts urunc reapplies the VFS flags, preserving the CL_UNPRIVILEGED locked flags of the source mount when running inside a user namespace. also, since we switch from moby/sys/mount to containerd's mount package we also switch from a lzy unmount to a strict one, which is necessary since we want to attach the block devices to the sandbox. Also fix getMonitorDevices to not append an empty device entry when /dev/net/tun does not exist on the host. Leftover from previous PR PR: https://github.com/urunc-dev/urunc/pull/840 Signed-off-by: Charalampos Mainas Reviewed-by: Anastassios Nanos Approved-by: Anastassios Nanos --- .github/linters/urunc-dict.txt | 2 + go.mod | 2 +- go.sum | 2 - pkg/unikontainers/block.go | 51 +++- pkg/unikontainers/mount.go | 484 +++++++++++++++++---------------- pkg/unikontainers/rootfs.go | 34 ++- pkg/unikontainers/shared_fs.go | 4 +- pkg/unikontainers/utils.go | 4 +- pkg/unikontainers/vaccel.go | 3 +- 9 files changed, 324 insertions(+), 262 deletions(-) diff --git a/.github/linters/urunc-dict.txt b/.github/linters/urunc-dict.txt index d7d2189a2..ac02f86bf 100644 --- a/.github/linters/urunc-dict.txt +++ b/.github/linters/urunc-dict.txt @@ -422,3 +422,5 @@ ESRCH Prafful praffq libcontainers +securejoin +cyphar diff --git a/go.mod b/go.mod index e2a5d292d..b544cec39 100644 --- a/go.mod +++ b/go.mod @@ -11,10 +11,10 @@ require ( github.com/containerd/log v0.1.0 github.com/containerd/ttrpc v1.2.9 github.com/creack/pty v1.1.24 + github.com/cyphar/filepath-securejoin v0.5.2 github.com/elastic/go-seccomp-bpf v1.6.0 github.com/hashicorp/go-version v1.9.0 github.com/jackpal/gateway v1.2.0 - github.com/moby/sys/mount v0.3.5 github.com/moby/sys/mountinfo v0.7.2 github.com/moby/sys/userns v0.1.0 github.com/nubificus/hedge_cli v0.0.3 diff --git a/go.sum b/go.sum index 4a3e44f8c..255032283 100644 --- a/go.sum +++ b/go.sum @@ -145,8 +145,6 @@ github.com/mfridman/tparse v0.18.0 h1:wh6dzOKaIwkUGyKgOntDW4liXSo37qg5AXbIhkMV3v github.com/mfridman/tparse v0.18.0/go.mod h1:gEvqZTuCgEhPbYk/2lS3Kcxg1GmTxxU7kTC8DvP0i/A= github.com/moby/sys/capability v0.4.0 h1:4D4mI6KlNtWMCM1Z/K0i7RV1FkX+DBDHKVJpCndZoHk= github.com/moby/sys/capability v0.4.0/go.mod h1:4g9IK291rVkms3LKCDOoYlnV8xKwoDTpIrNEE35Wq0I= -github.com/moby/sys/mount v0.3.5 h1:eS3fsZTjHaBihwjp4/+5Z3jxqLXYsbwxqpVSfFv3M00= -github.com/moby/sys/mount v0.3.5/go.mod h1:WUQDO+/uCiCIkIztx8SrwIDVn2dtMFRBebRhpDFT71M= github.com/moby/sys/mountinfo v0.7.2 h1:1shs6aH5s4o5H2zQLn796ADW1wMrIwHsyJ2v9KouLrg= github.com/moby/sys/mountinfo v0.7.2/go.mod h1:1YOa8w8Ih7uW0wALDUgT1dTTSBrZ+HiBLGws92L2RU4= github.com/moby/sys/sequential v0.7.0 h1:ASQNGNROJSuOO6LL6bPHbKvuZu6NU8P4ldPWk31zj/8= diff --git a/pkg/unikontainers/block.go b/pkg/unikontainers/block.go index 6914586c6..69db4629a 100644 --- a/pkg/unikontainers/block.go +++ b/pkg/unikontainers/block.go @@ -21,8 +21,8 @@ import ( "os" "path/filepath" "strings" + "time" - "github.com/moby/sys/mount" "github.com/moby/sys/mountinfo" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" @@ -95,7 +95,7 @@ func getMountInfo(path string) (types.BlockDevParams, error) { blockDev.Source = postDash[1] blockDev.FsType = postDash[0] blockDev.MountPoint = path - // Keep the-mount VFS options (field 6 of mountinfo) + // Keep the mount VFS options (field 6 of mountinfo) // to restore them later in the delete path. blockDev.MountOptions = preDash[5] blockDev.ID = "" @@ -161,7 +161,7 @@ func copyMountfiles(targetPath string, mounts []specs.Mount) error { if m.Type != "bind" { continue } - err := fileFromHost(targetPath, m.Source, m.Destination, 0, true) + err := fileFromHost(targetPath, m.Source, m.Destination) if (err != nil) && !errors.Is(err, ErrCopyDir) { return err } @@ -224,14 +224,14 @@ func getBlockVolumes(mounts []specs.Mount, ukernel types.Unikernel) ([]types.Blo // NOTE: Although we restore the autoclear flag in the delete path, // if delete is never called then the autoclear flag will never // get restored.and remounted - // TODO; Add the above note in a documentation for storage + // TODO: Add the above note in a documentation for storage // handling cleared, err := setLoopAutoclear(mInfo.Source, false) if err != nil { return nil, err } mInfo.LoopAutoclear = cleared - err = mount.Unmount(mInfo.MountPoint) + err = unmount(mInfo.MountPoint) if err != nil { return nil, err } @@ -262,7 +262,24 @@ func restoreBlockVolumes(blockArgs []types.BlockDevParams) error { continue } - flags, _ := splitMountOptions(strings.Split(b.MountOptions, ",")) + // restoring block volumes is a simple mount operation, but using + // containerd can lead to errors because the mount options parser of + // containerd might not handle some VFS flags correctly and misplace + // them in the options argument of mount system call. + // Therefore, do not use containerd for such mounts and handle them + // directly. + var flags uintptr + for _, o := range strings.Split(b.MountOptions, ",") { + flag, clearFlag, err := mapVFSFlag(o) + if err != nil { + continue + } + if clearFlag { + flags &^= flag + } else { + flags |= flag + } + } err = unix.Mount(b.Source, b.HostMountPoint, b.FsType, flags, "") if err != nil { return fmt.Errorf("failed to remount %s at %s: %w", b.Source, b.HostMountPoint, err) @@ -362,7 +379,7 @@ func (b blockRootfs) preSetup() error { return fmt.Errorf("failed to extract boot files from rootfs: %w", err) } - err = mount.Unmount(b.mountedPath) + err = unmount(b.mountedPath) if err != nil { return fmt.Errorf("failed to unmount rootfs: %w", err) } @@ -412,3 +429,23 @@ func (b blockRootfs) getSharedDirs() (types.SharedfsParams, error) { func (b blockRootfs) preStart() error { return nil } + +// Taken from https://github.com/containerd/containerd/blob/v1.7.34/mount/mount_linux.go#L203 +// and we simply change the timeout period to max 200 ms, then EBUSY is returned. +func unmount(target string) error { + for i := 0; i < 10; i++ { + // Always aim for strict unmount + err := unix.Unmount(target, 0) + if err != nil { + switch err { + case unix.EBUSY: + time.Sleep(20 * time.Millisecond) + continue + default: + return err + } + } + return nil + } + return fmt.Errorf("failed to unmount target %s: %w", target, unix.EBUSY) +} diff --git a/pkg/unikontainers/mount.go b/pkg/unikontainers/mount.go index 290e4e3f2..6170db0da 100644 --- a/pkg/unikontainers/mount.go +++ b/pkg/unikontainers/mount.go @@ -25,8 +25,10 @@ import ( "math" "os" "path/filepath" - "strings" + "slices" + "github.com/containerd/containerd/mount" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/moby/sys/userns" "golang.org/x/sys/unix" @@ -35,45 +37,6 @@ import ( var ErrCopyDir = errors.New("can not copy a directory") -type mountFlagStruct struct { - clear bool - flag int -} - -// createTmpfs creates a new tmpfs at path inside monRootfs -// In particular, it is used for the creation of /tmp and /dev. -// This is necessary to create the required devices for the monitor execution, -// such as KVM, null, urandom etc. -func createTmpfs(monRootfs string, path string, flags uintptr, data string) error { - dstPath := filepath.Join(monRootfs, path) - mountType := "tmpfs" - - err := os.MkdirAll(dstPath, 0755) - if err != nil { - return fmt.Errorf("failed to create %s dir: %w", path, err) - } - - err = unix.Mount(mountType, dstPath, mountType, flags, data) - if err != nil { - return fmt.Errorf("failed to mount %s tmpfs: %w", path, err) - } - - // Remove propagation - err = unix.Mount("", dstPath, "", unix.MS_PRIVATE, "") - if err != nil { - return fmt.Errorf("failed to create %s tmpfs: %w", path, err) - } - - if strings.Contains(","+data+",", ",mode=1777,") { - // sonarcloud:go:S2612 -- This is a tmpfs mount point, sticky bit 1777 is required (like /tmp), controlled path, safe by design - err := os.Chmod(dstPath, 01777) // NOSONAR - if err != nil { - return fmt.Errorf("failed to chmod %s: %w", path, err) - } - } - return nil -} - // setupDevices creates every device from the list inside monitor's rootfs func setupDevices(monRootfs string, devices []specs.LinuxDevice, needsTAP bool) error { for _, dev := range devices { @@ -99,7 +62,7 @@ func setupDev(monRootfs string, dev specs.LinuxDevice) error { // In a user namespace, always bind-mount the existing host device node. // Only MS_BIND is used here (no extra flags) to mirror runc's device handling. if userns.RunningInUserNS() { - return fileFromHost(monRootfs, dev.Path, "", unix.MS_BIND, false) + return applyMount(monRootfs, bindMount(dev.Path, dev.Path, false)) } var devType uint32 @@ -170,23 +133,24 @@ func setupDev(monRootfs string, dev specs.LinuxDevice) error { return nil } -// fileFromHost set ups a mirror of file from the host's rootfs inside the -// container's rootfs. Also, it preserves the permissions and ownership of the -// file in the host's rootfs. -// if withCopy is set then copy the file, otherwise -// bind mount it. -// In the context of monitor binaries a copy is considered safer, since -// none of the monitor processes will share memory with other processes -// of the same monitor. On the other hand, a copy is slower and consumes -// more space. -func fileFromHost(monRootfs string, hostPath string, target string, mFlags int, withCopy bool) error { +// fileFromHost copies a file from the host's rootfs into monRootfs, preserving +// the original file's permissions and ownership. If target is empty, the file +// 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. +func fileFromHost(monRootfs string, hostPath string, target string) error { // Get the info of the original file var fileInfo unix.Stat_t err := unix.Stat(hostPath, &fileInfo) if err != nil { return err } - mode := fileInfo.Mode + + if (fileInfo.Mode & unix.S_IFMT) == unix.S_IFDIR { + return ErrCopyDir + } if target == "" { // Set the correct path @@ -195,83 +159,25 @@ func fileFromHost(monRootfs string, hostPath string, target string, mFlags int, return fmt.Errorf("failed to get relative path of %s to /: %w", hostPath, err) } } - dstPath := filepath.Join(monRootfs, target) - - if (mode & unix.S_IFMT) != unix.S_IFDIR { - dstDir := filepath.Dir(dstPath) - if withCopy { - err = copyFile(hostPath, dstPath) - if err != nil { - return fmt.Errorf("failed to copy file %s: %w", hostPath, err) - } - } else { - err = bindMountFile(hostPath, dstDir, dstPath, fileInfo.Mode, mFlags, false) - if err != nil { - return fmt.Errorf("failed to bind mount file %s: %w", hostPath, err) - } - } - } else { - if withCopy { - return ErrCopyDir - } - err = bindMountFile(hostPath, dstPath, "", 0, mFlags, true) - if err != nil { - return fmt.Errorf("failed to bind mount file %s: %w", hostPath, err) - } - } - - // If a copy is created, set up the permissions and ownership to match the original file. - // For bind mounts the host inode attributes remain unchanged. - if withCopy { - err = unix.Chmod(dstPath, fileInfo.Mode) - if err != nil { - return fmt.Errorf("failed to chmod %s: %w", dstPath, err) - } - - err = os.Chown(dstPath, int(fileInfo.Uid), int(fileInfo.Gid)) - if err != nil { - return fmt.Errorf("failed to chown %s: %w", dstPath, err) - } - } - - // The initial MS_BIND won't change the mount options, we need to do a - // separate MS_BIND|MS_REMOUNT to apply the mount options. We skip - // doing this if the user has not specified any mount flags at all - // (including cleared flags) -- in which case we just keep the original - // mount flags. - if mFlags & ^(unix.MS_BIND|unix.MS_REC|unix.MS_REMOUNT) != 0 { - flags := mFlags | unix.MS_BIND | unix.MS_REMOUNT - err = unix.Mount(dstPath, dstPath, "", uintptr(flags), "") - if err != nil { - return fmt.Errorf("failed to set mount flags for %s: %w", dstPath, err) - } + dstPath, err := securejoin.SecureJoin(monRootfs, target) + if err != nil { + return fmt.Errorf("failed to resolve target %s: %w", target, err) } - return nil -} - -// bindMountFile bind mounts a file/directory to a new path -func bindMountFile(hostPath string, dstDir string, dstPath string, perm uint32, mFlags int, isDir bool) error { - var mountTarget string - err := os.MkdirAll(dstDir, 0755) + err = copyFile(hostPath, dstPath) if err != nil { - return fmt.Errorf("failed to create directory %s: %w", dstDir, err) + return fmt.Errorf("failed to copy file %s: %w", hostPath, err) } - if !isDir { - dstFile, err1 := unix.Open(dstPath, unix.O_CREAT, perm) - if err1 != nil { - return fmt.Errorf("failed to create file %s: %w", dstPath, err1) - } - unix.Close(dstFile) - mountTarget = dstPath - } else { - mountTarget = dstDir + // Set up the permissions and ownership to match the original file. + err = unix.Chmod(dstPath, fileInfo.Mode) + if err != nil { + return fmt.Errorf("failed to chmod %s: %w", dstPath, err) } - err = unix.Mount(hostPath, mountTarget, "", uintptr(mFlags), "") + err = os.Chown(dstPath, int(fileInfo.Uid), int(fileInfo.Gid)) if err != nil { - return fmt.Errorf("failed to bind mount %s: %w", mountTarget, err) + return fmt.Errorf("failed to chown %s: %w", dstPath, err) } return nil @@ -299,6 +205,102 @@ func mapRootfsPropagationFlag(value string) (int, error) { return propagation, nil } +// mapVFSFlag maps a per-mount VFS mount option to its mount(2) flag bit and +// whether the option clears the flag rather than setting it (e.g. "rw" clears +// the read-only flag set by "ro"). It returns an error for options that are not +// VFS flags. +func mapVFSFlag(value string) (flag uintptr, clear bool, err error) { + vfsFlagMapping := map[string]struct { + clear bool + flag uintptr + }{ + "ro": {false, unix.MS_RDONLY}, + "rw": {true, unix.MS_RDONLY}, + "nosuid": {false, unix.MS_NOSUID}, + "suid": {true, unix.MS_NOSUID}, + "nodev": {false, unix.MS_NODEV}, + "dev": {true, unix.MS_NODEV}, + "noexec": {false, unix.MS_NOEXEC}, + "exec": {true, unix.MS_NOEXEC}, + "noatime": {false, unix.MS_NOATIME}, + "atime": {true, unix.MS_NOATIME}, + "nodiratime": {false, unix.MS_NODIRATIME}, + "diratime": {true, unix.MS_NODIRATIME}, + "relatime": {false, unix.MS_RELATIME}, + "norelatime": {true, unix.MS_RELATIME}, + "strictatime": {false, unix.MS_STRICTATIME}, + "nostrictatime": {true, unix.MS_STRICTATIME}, + "nosymfollow": {false, unix.MS_NOSYMFOLLOW}, + "symfollow": {true, unix.MS_NOSYMFOLLOW}, + "sync": {false, unix.MS_SYNCHRONOUS}, + "async": {true, unix.MS_SYNCHRONOUS}, + "dirsync": {false, unix.MS_DIRSYNC}, + "mand": {false, unix.MS_MANDLOCK}, + "nomand": {true, unix.MS_MANDLOCK}, + "silent": {false, unix.MS_SILENT}, + "loud": {true, unix.MS_SILENT}, + "lazytime": {false, unix.MS_LAZYTIME}, + "nolazytime": {true, unix.MS_LAZYTIME}, + "iversion": {false, unix.MS_I_VERSION}, + "noiversion": {true, unix.MS_I_VERSION}, + } + + f, exists := vfsFlagMapping[value] + if !exists { + return 0, false, fmt.Errorf("%s is not a supported vfs mount flag", value) + } + + return f.flag, f.clear, nil +} + +// Get the set of mount flags that are set on the mount that contains the given +// path and are locked by CL_UNPRIVILEGED. This is necessary to ensure that +// bind-mounting "with options" will not fail with user namespaces, due to +// kernel restrictions that require user namespace mounts to preserve +// CL_UNPRIVILEGED locked flags. +// Similar to +// https://github.com/opencontainers/umoci/blob/f5d1219acaf67127ebacf6306776d3ff465735ea/oci/config/convert/utils_linux.go#L33 +// but returns the uintptr to use in mount later. +func getUnprivilegedMountFlags(path string) (uintptr, error) { + var st unix.Statfs_t + err := unix.Statfs(path, &st) + if err != nil { + return 0, err + } + + // statfs reports ST_* flags. They match the corresponding MS_* values for + // most flags, but ST_RELATIME (0x1000) differs from MS_RELATIME (0x200000), + // so map each ST_* bit that statfs reports to the MS_* flag used for the + // remount explicitly. + lockedFlags := []struct { + st, ms uintptr + }{ + {unix.ST_RDONLY, unix.MS_RDONLY}, + {unix.ST_NODEV, unix.MS_NODEV}, + {unix.ST_NOEXEC, unix.MS_NOEXEC}, + {unix.ST_NOSUID, unix.MS_NOSUID}, + {unix.ST_NOATIME, unix.MS_NOATIME}, + {unix.ST_RELATIME, unix.MS_RELATIME}, + {unix.ST_NODIRATIME, unix.MS_NODIRATIME}, + } + + var flags uintptr + for _, f := range lockedFlags { + if uintptr(st.Flags)&f.st == f.st { + flags |= f.ms + } + } + + // With neither noatime nor relatime set, the source uses strictatime; + // preserve it explicitly so a rootless remount of a strictatime-locked source + // is not rejected with EPERM. + if flags&(unix.MS_NOATIME|unix.MS_RELATIME) == 0 { + flags |= unix.MS_STRICTATIME + } + + return flags, nil +} + // rootfsParentMountPrivate ensures rootfs parent mount is private. // This is needed for two reasons: // - pivot_root() will fail if parent mount is shared; @@ -350,154 +352,166 @@ func prepareRoot(path string, rootfsPropagation string) error { return unix.Mount(path, path, "bind", unix.MS_BIND|unix.MS_REC, "") } +// applyMounts sets up every mount specified in the mounts argument inside the +// directory of rootfsPath. func applyMounts(rootfsPath string, mounts []specs.Mount) error { for _, m := range mounts { - switch m.Type { - case "tmpfs": - // TODO: replace createTmpfs with a mount package (e.g. - // containerd's). For the time being keep the compatibility - // with createTmpfs. - flags, data := splitMountOptions(m.Options) - err := createTmpfs(rootfsPath, m.Destination, flags, data) - if err != nil { - return fmt.Errorf("failed to create tmpfs at %s: %w", m.Destination, err) - } - case "proc", "devpts": - err := applySpecialFsMount(rootfsPath, m) - if err != nil { - return fmt.Errorf("failed to create %s at %s: %w", m.Type, m.Destination, err) - } - case "bind": - err := applyBindMount(rootfsPath, m) - if err != nil { - return fmt.Errorf("failed to bind mount %s at %s: %w", m.Source, m.Destination, err) - } - default: - // Skip unknown mount types - // TODO handle other types of mounts too - continue + err := applyMount(rootfsPath, m) + if err != nil { + // NOTE: We do not cleanup here because we assume that + // a mount namespace was declared and we are inside one. + // Therefore, after a fail, we will exit reexec (the only + // process inside the mount namespace and therefore the + // namespace will get removed and the kernel will cleanup + // the mounts. Revisit this in the future and check if it + // is urunc's responsibility to cleanup. If it is we need + // to account for cases where a mount namespace was not + // specified and therefore we need to manually unmount everything. + return fmt.Errorf("failed to apply mount %s: %w", m.Source, err) } } return nil } -// applySpecialFsMount handles pseudo filesystem (e.g. proc, devpts) mounts -func applySpecialFsMount(rootfsPath string, m specs.Mount) error { - dstPath := filepath.Join(rootfsPath, m.Destination) - err := os.MkdirAll(dstPath, 0755) +// applyMount mounts a single entry under rootfsPath. +func applyMount(rootfsPath string, m specs.Mount) error { + target, err := securejoin.SecureJoin(rootfsPath, m.Destination) if err != nil { - return fmt.Errorf("failed to create %s dir: %w", m.Destination, err) + return fmt.Errorf("failed to resolve mount target %s: %w", m.Destination, err) } - flags, data := splitMountOptions(m.Options) - err = unix.Mount(m.Source, dstPath, m.Type, flags, data) + err = createMountPoint(target, m) if err != nil { - return fmt.Errorf("failed to mount %s: %w", m.Destination, err) + return fmt.Errorf("failed to create mount target %s: %w", target, err) } - return nil -} + // 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") -// applyBindMount handles the bind mounts for the monitor rootfs -func applyBindMount(rootfsPath string, m specs.Mount) error { - var mountFlags int - var propFlag []int - for _, o := range m.Options { - f, exists := mapMountFlag(o) - if exists { - if f.clear { - mountFlags &= ^f.flag - } else { - mountFlags |= f.flag + cm := mount.Mount{ + Type: m.Type, + Source: m.Source, + Options: containerdOpts, + } + err = cm.Mount(target) + if err != nil { + return fmt.Errorf("failed to mount %s at %s: %w", cm.Source, target, err) + } + + // The kernel ignores per-mount VFS flags when a bind is created, so re-apply + // 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 { + remount := vfsFlags | unix.MS_BIND | unix.MS_REMOUNT + if userns.RunningInUserNS() { + locked, err := getUnprivilegedMountFlags(m.Source) + if err != nil { + return fmt.Errorf("failed to get locked mount flags of %s: %w", m.Source, err) } - continue + // Merging the locked flags means a source whose mount is locked + // read-only forces the bind read-only even if the spec asked for rw. + // We accept that as a trade-off to keep rootless remounts working; + // runc instead errors out in this case. + remount |= locked } - fprop, err := mapRootfsPropagationFlag(o) - if err == nil { - propFlag = append(propFlag, fprop) + err = unix.Mount("", target, "", remount, "") + if err != nil { + return fmt.Errorf("failed to apply mount flags for %s: %w", target, err) } - // Ignore unknown flags - // TODO: Handle unknown flags. These can be mount attribute flags - // or specific flags for a particular fs type. } - err := fileFromHost(rootfsPath, m.Source, m.Destination, mountFlags, false) - if err != nil { - return err + + for _, pFlag := range propagation { + err = unix.Mount("", target, "", uintptr(pFlag), "") + if err != nil { + return fmt.Errorf("failed to set propagation flag for %s: %w", m.Destination, err) + } } - dstPath := filepath.Join(rootfsPath, m.Destination) - for _, pFlag := range propFlag { - err = unix.Mount(dstPath, dstPath, "", uintptr(pFlag), "") + if m.Type == "tmpfs" && slices.Contains(m.Options, "mode=1777") { + err = os.Chmod(target, 0o777|os.ModeSticky) if err != nil { - return fmt.Errorf("failed to set propagation flag for %s: %w", m.Source, err) + return fmt.Errorf("failed to chmod %s: %w", target, err) } } return nil } -// splitMountOptions separates mount options into the corresponding mount flags -// and the remaining options, which are returned as a comma-separated data -// string to be passed to mount(2). -func splitMountOptions(options []string) (uintptr, string) { - var flags int - var data []string - for _, o := range options { - f, exists := mapMountFlag(o) - if exists { - if f.clear { - flags &= ^f.flag - } else { - flags |= f.flag +// createMountPoint creates the mountpoint of a mount. +func createMountPoint(target string, m specs.Mount) error { + if m.Type == "bind" { + var st unix.Stat_t + err := unix.Stat(m.Source, &st) + if err != nil { + return fmt.Errorf("failed to stat mount source %s: %w", m.Source, err) + } + if st.Mode&unix.S_IFMT != unix.S_IFDIR { + dstDir := filepath.Dir(target) + err := os.MkdirAll(dstDir, 0755) + if err != nil { + return fmt.Errorf("failed to create directory %s: %w", dstDir, err) } - continue + fd, err := unix.Open(target, unix.O_CREAT, 0644) + if err != nil { + return fmt.Errorf("failed to create file %s: %w", target, err) + } + err = unix.Close(fd) + if err != nil { + return fmt.Errorf("failed to close file %s: %w", target, err) + } + + return nil } - data = append(data, o) } - return uintptr(flags), strings.Join(data, ",") + err := os.MkdirAll(target, 0755) + if err != nil { + return fmt.Errorf("failed to create directory %s: %w", target, err) + } + + return nil } -// mapMountFlag retrieves the mount flags of a mount entry -// from the container's configuration -func mapMountFlag(value string) (mountFlagStruct, bool) { - mountFlagsMapping := map[string]mountFlagStruct{ - "async": {true, unix.MS_SYNCHRONOUS}, - "atime": {true, unix.MS_NOATIME}, - "bind": {false, unix.MS_BIND}, - "defaults": {false, 0}, - "dev": {true, unix.MS_NODEV}, - "diratime": {true, unix.MS_NODIRATIME}, - "dirsync": {false, unix.MS_DIRSYNC}, - "exec": {true, unix.MS_NOEXEC}, - "iversion": {false, unix.MS_I_VERSION}, - "lazytime": {false, unix.MS_LAZYTIME}, - "loud": {true, unix.MS_SILENT}, - "mand": {false, unix.MS_MANDLOCK}, - "noatime": {false, unix.MS_NOATIME}, - "nodev": {false, unix.MS_NODEV}, - "nodiratime": {false, unix.MS_NODIRATIME}, - "noexec": {false, unix.MS_NOEXEC}, - "noiversion": {true, unix.MS_I_VERSION}, - "nolazytime": {true, unix.MS_LAZYTIME}, - "nomand": {true, unix.MS_MANDLOCK}, - "norelatime": {true, unix.MS_RELATIME}, - "nostrictatime": {true, unix.MS_STRICTATIME}, - "nosuid": {false, unix.MS_NOSUID}, - "nosymfollow": {false, unix.MS_NOSYMFOLLOW}, // since kernel 5.10 - "rbind": {false, unix.MS_BIND | unix.MS_REC}, - "relatime": {false, unix.MS_RELATIME}, - "remount": {false, unix.MS_REMOUNT}, - "ro": {false, unix.MS_RDONLY}, - "rw": {true, unix.MS_RDONLY}, - "silent": {false, unix.MS_SILENT}, - "strictatime": {false, unix.MS_STRICTATIME}, - "suid": {true, unix.MS_NOSUID}, - "sync": {false, unix.MS_SYNCHRONOUS}, - "symfollow": {true, unix.MS_NOSYMFOLLOW}, // since kernel 5.10 +// splitMountOptions splits a mount's options into three groups: the options +// for containerd's mount package, the propagation flags, and the per-mount VFS +// flags. Propagation flags never go to containerd because it cannot parse them +// and would end up in fs-specific data. The VFS flags are where bind mounts +// differ. The kernel ignores per-mount VFS flags when a bind mount is created, +// so with isBind set these options are withheld from containerd and returned +// as flag bits for the caller to apply. For non-bind mounts the VFS options +// stay in the containerd options, since the initial mount(2) applies them; the +// returned flag bits are meaningless in that case and must be ignored. +func splitMountOptions(options []string, isBind bool) ([]string, []int, uintptr) { + var containerdOpts []string + var propagation []int + var vfsFlags uintptr + for _, o := range options { + pFlag, err := mapRootfsPropagationFlag(o) + if err == nil { + propagation = append(propagation, pFlag) + continue + } + + flag, clear, err := mapVFSFlag(o) + if err == nil { + if clear { + vfsFlags &^= flag + } else { + vfsFlags |= flag + } + // For bind mounts keep the VFS flags separate from containerd + // and apply them later. + if isBind { + continue + } + } + + containerdOpts = append(containerdOpts, o) } - f, e := mountFlagsMapping[value] - return f, e + return containerdOpts, propagation, vfsFlags } diff --git a/pkg/unikontainers/rootfs.go b/pkg/unikontainers/rootfs.go index 546684ba8..bd1c800f0 100644 --- a/pkg/unikontainers/rootfs.go +++ b/pkg/unikontainers/rootfs.go @@ -55,18 +55,26 @@ func tmpfsMount(target string, size string) specs.Mount { "strictatime", "mode=1777", "size=" + size, + "private", }, } } -// bindMount builds a private, non-recursive bind mount of source at target -func bindMount(source string, target string) specs.Mount { - return specs.Mount{ +// bindMount builds a non-recursive, and private if argument is set, bind mount +// of source at target +func bindMount(source string, target string, private bool) specs.Mount { + m := specs.Mount{ Type: "bind", Source: source, Destination: target, - Options: []string{"bind", "private"}, + Options: []string{"bind"}, + } + + if private { + m.Options = append(m.Options, "private") } + + return m } // deviceFromHost finds a device in the host from the path and returns its info @@ -397,6 +405,7 @@ func getMonitorDevices(needsKVM bool) ([]specs.LinuxDevice, error) { if err != nil { return nil, fmt.Errorf("could not get host device /dev/urandom: %w", err) } + devices := []specs.LinuxDevice{nullDev, randomDev} // The tun device is always included because in urunc create we can not know // if there will be a virtual ethernet device or not, since the CNI hooks // have not executed yet. Therefore, the decision about whether it is actually @@ -410,8 +419,9 @@ func getMonitorDevices(needsKVM bool) ([]specs.LinuxDevice, error) { } else { return nil, fmt.Errorf("could not get host device /dev/net/tun: %w", err) } + } else { + devices = append(devices, tunDev) } - devices := []specs.LinuxDevice{nullDev, randomDev, tunDev} if needsKVM { kvmDev, err := deviceFromHost("/dev/kvm") @@ -465,7 +475,7 @@ func mountsForMonitor(monitorPath string, monitorDataPath string) ([]specs.Mount Type: "tmpfs", Source: "tmpfs", Destination: "/dev", - Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k"}, + Options: []string{"nosuid", "strictatime", "mode=755", "size=65536k", "private"}, } devPtsMount := specs.Mount{ Type: "devpts", @@ -474,21 +484,21 @@ func mountsForMonitor(monitorPath string, monitorDataPath string) ([]specs.Mount Options: []string{"nosuid", "noexec", "newinstance", "ptmxmode=0666", "mode=0620"}, } - mounts := []specs.Mount{procMount, devMount, devPtsMount, bindMount(monitorPath, monitorPath)} + mounts := []specs.Mount{procMount, devMount, devPtsMount, bindMount(monitorPath, monitorPath, true)} monitorName := filepath.Base(monitorPath) // TODO: Remove most of these when we switch to static binaries. if monitorName != "firecracker" { - mounts = append(mounts, bindMount("/lib", "/lib")) + mounts = append(mounts, bindMount("/lib", "/lib", true)) // If /lib64 does not exist, just ignore it if _, err := os.Stat("/lib64"); err == nil { - mounts = append(mounts, bindMount("/lib64", "/lib64")) + mounts = append(mounts, bindMount("/lib64", "/lib64", true)) } else if !os.IsNotExist(err) { return nil, err } - mounts = append(mounts, bindMount("/usr/lib", "/usr/lib")) + mounts = append(mounts, bindMount("/usr/lib", "/usr/lib", true)) } if len(monitorName) >= 4 && monitorName[:4] == "qemu" { @@ -506,12 +516,12 @@ func mountsForMonitor(monitorPath string, monitorDataPath string) ([]specs.Mount } } - mounts = append(mounts, bindMount(qDataPath, "/usr/share/qemu")) + mounts = append(mounts, bindMount(qDataPath, "/usr/share/qemu", true)) // In urunc-deploy and in some distros seabios does not exist and // we do not need it. So if we could not find it, just ignore it. if _, err := os.Stat(sBiosPath); err == nil { - mounts = append(mounts, bindMount(sBiosPath, "/usr/share/seabios")) + mounts = append(mounts, bindMount(sBiosPath, "/usr/share/seabios", true)) } else if !os.IsNotExist(err) { return nil, err } diff --git a/pkg/unikontainers/shared_fs.go b/pkg/unikontainers/shared_fs.go index 33f91fbd7..9441f39a7 100644 --- a/pkg/unikontainers/shared_fs.go +++ b/pkg/unikontainers/shared_fs.go @@ -48,11 +48,11 @@ func (s sharedfsRootfs) postSetup() error { func (s sharedfsRootfs) getMounts() ([]specs.Mount, error) { // Mount the container's rootfs inside the monitor rootfs and then the // container's volumes on top of it. - mounts := []specs.Mount{bindMount(s.mountedPath, containerRootfsMountPath)} + mounts := []specs.Mount{bindMount(s.mountedPath, containerRootfsMountPath, true)} if s.sfsType == "virtiofs" { // Get the virtiofsd binary from host in monRootfs - mounts = append(mounts, bindMount(s.vfsdConfig.Path, s.vfsdConfig.Path)) + mounts = append(mounts, bindMount(s.vfsdConfig.Path, s.vfsdConfig.Path, true)) } tmpfsSize := chooseTmpfsSize(s.sfsType, s.memory) diff --git a/pkg/unikontainers/utils.go b/pkg/unikontainers/utils.go index 83c9d72af..90b9500cf 100644 --- a/pkg/unikontainers/utils.go +++ b/pkg/unikontainers/utils.go @@ -63,7 +63,9 @@ func saveMonitorResources(baseDir string, res monitorResources) error { path := filepath.Join(baseDir, monitorFilename) - return os.WriteFile(path, data, 0o644) //nolint: gosec + // monitor.json is internal runtime state, written and read back only by the + // urunc runtime. Keep it owner-only. + return os.WriteFile(path, data, 0o600) } // loadMonitorResources reads the monitor resources file stored by InitialSetup. diff --git a/pkg/unikontainers/vaccel.go b/pkg/unikontainers/vaccel.go index b640b1ea3..cc22cb17f 100644 --- a/pkg/unikontainers/vaccel.go +++ b/pkg/unikontainers/vaccel.go @@ -20,7 +20,6 @@ import ( "regexp" "github.com/opencontainers/runtime-spec/specs-go" - "golang.org/x/sys/unix" ) // ErrVAccelDisabled is returned by resolveVAccelConfig when the vAccel @@ -123,7 +122,7 @@ func prepareVSockEnvironment(monRootfs string, hypervisor string, vsockSocketPat // bind mount the unix socket directory if hypervisor == "firecracker" { - err = fileFromHost(monRootfs, vsockSocketPath, "", unix.MS_BIND|unix.MS_PRIVATE, false) + err = applyMount(monRootfs, bindMount(vsockSocketPath, vsockSocketPath, true)) if err != nil { return nil, err }