From 46ada59ba2339d4a7bef5a7682dba70d351551bc Mon Sep 17 00:00:00 2001 From: Francis Laniel Date: Tue, 18 Jul 2023 10:56:15 +0200 Subject: [PATCH] Use an *int for srcFD Previously to this commit, we used a string for srcFD as /proc/self/fd/NN. This commit modified to this behavior, so srcFD is only an *int and the full path is constructed in mountViaFDs() if srcFD is different than nil. Signed-off-by: Francis Laniel --- libcontainer/container_linux.go | 5 +---- libcontainer/mount_linux.go | 22 ++++++++++++---------- libcontainer/rootfs_linux.go | 22 +++++++++++----------- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index e4a1e6d7d..2f0a6c641 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -1419,10 +1419,7 @@ func (c *Container) prepareCriuRestoreMounts(mounts []*configs.Mount) error { // set up in the order they are configured. if m.Device == "bind" { if err := utils.WithProcfd(c.config.Rootfs, m.Destination, func(dstFD string) error { - if err := mountViaFDs(m.Source, "", m.Destination, dstFD, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { - return err - } - return nil + return mountViaFDs(m.Source, nil, m.Destination, dstFD, "", unix.MS_BIND|unix.MS_REC, "") }); err != nil { return err } diff --git a/libcontainer/mount_linux.go b/libcontainer/mount_linux.go index 751262f31..b391dd849 100644 --- a/libcontainer/mount_linux.go +++ b/libcontainer/mount_linux.go @@ -10,7 +10,7 @@ import ( type mountError struct { op string source string - srcFD string + srcFD *int target string dstFD string flags uintptr @@ -24,8 +24,8 @@ func (e *mountError) Error() string { if e.source != "" { out += "src=" + e.source + ", " - if e.srcFD != "" { - out += "srcFD=" + e.srcFD + ", " + if e.srcFD != nil { + out += "srcFD=" + strconv.Itoa(*e.srcFD) + ", " } } out += "dst=" + e.target @@ -53,21 +53,23 @@ func (e *mountError) Unwrap() error { // mount is a simple unix.Mount wrapper, returning an error with more context // in case it failed. func mount(source, target, fstype string, flags uintptr, data string) error { - return mountViaFDs(source, "", target, "", fstype, flags, data) + return mountViaFDs(source, nil, target, "", fstype, flags, data) } // mountViaFDs is a unix.Mount wrapper which uses srcFD instead of source, -// and dstFD instead of target, unless those are empty. The *FD arguments, -// if non-empty, are expected to be in the form of a path to an opened file -// descriptor on procfs (i.e. "/proc/self/fd/NN"). +// and dstFD instead of target, unless those are empty. +// If srcFD is different than nil, its path (i.e. "/proc/self/fd/NN") will be +// constructed by this function. +// dstFD argument, if non-empty, is expected to be in the form of a path to an +// opened file descriptor on procfs (i.e. "/proc/self/fd/NN"). // // If case an FD is used instead of a source or a target path, the // corresponding path is only used to add context to an error in case // the mount operation has failed. -func mountViaFDs(source, srcFD, target, dstFD, fstype string, flags uintptr, data string) error { +func mountViaFDs(source string, srcFD *int, target, dstFD, fstype string, flags uintptr, data string) error { src := source - if srcFD != "" { - src = srcFD + if srcFD != nil { + src = "/proc/self/fd/" + strconv.Itoa(*srcFD) } dst := target if dstFD != "" { diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index edd3abd3c..0da300f99 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -40,13 +40,13 @@ type mountConfig struct { // mountEntry contains mount data specific to a mount point. type mountEntry struct { *configs.Mount - srcFD string + srcFD *int idmapFD int } func (m *mountEntry) src() string { - if m.srcFD != "" { - return m.srcFD + if m.srcFD != nil { + return "/proc/self/fd/" + strconv.Itoa(*m.srcFD) } return m.Source } @@ -90,7 +90,7 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds mountFds) ( // Just before the loop we checked that if not empty, len(mountFds) == len(config.Mounts). // Therefore, we can access mountFds[i] without any concerns. if mountFds.sourceFds != nil && mountFds.sourceFds[i] != -1 { - entry.srcFD = "/proc/self/fd/" + strconv.Itoa(mountFds.sourceFds[i]) + entry.srcFD = &mountFds.sourceFds[i] } // We validated before we can access idmapFds[i]. @@ -98,7 +98,7 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds mountFds) ( entry.idmapFD = mountFds.idmapFds[i] } - if entry.idmapFD != -1 && entry.srcFD != "" { + if entry.idmapFD != -1 && entry.srcFD != nil { return fmt.Errorf("malformed mountFds and idmapFds slice, entry: %v has fds in both slices", i) } @@ -297,7 +297,7 @@ func mountCgroupV1(m *configs.Mount, c *mountConfig) error { data = cgroups.CgroupNamePrefix + data source = "systemd" } - return mountViaFDs(source, "", b.Destination, dstFD, "cgroup", uintptr(flags), data) + return mountViaFDs(source, nil, b.Destination, dstFD, "cgroup", uintptr(flags), data) }); err != nil { return err } @@ -329,7 +329,7 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { return err } err = utils.WithProcfd(c.root, m.Destination, func(dstFD string) error { - return mountViaFDs(m.Source, "", m.Destination, dstFD, "cgroup2", uintptr(m.Flags), m.Data) + return mountViaFDs(m.Source, nil, m.Destination, dstFD, "cgroup2", uintptr(m.Flags), m.Data) }) if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) { return err @@ -403,7 +403,7 @@ func doTmpfsCopyUp(m mountEntry, rootfs, mountLabel string) (Err error) { return fmt.Errorf("tmpcopyup: failed to copy %s to %s (%s): %w", m.Destination, dstFD, tmpDir, err) } // Now move the mount into the container. - if err := mountViaFDs(tmpDir, "", m.Destination, dstFD, "", unix.MS_MOVE, ""); err != nil { + if err := mountViaFDs(tmpDir, nil, m.Destination, dstFD, "", unix.MS_MOVE, ""); err != nil { return fmt.Errorf("tmpcopyup: failed to move mount: %w", err) } return nil @@ -497,7 +497,7 @@ func mountToRootfs(c *mountConfig, m mountEntry) error { // system type and data arguments are ignored: // https://man7.org/linux/man-pages/man2/mount.2.html // We also ignore procfd because we want to act on dest. - if err := mountViaFDs("", "", dest, dstFD, "", uintptr(pflag), ""); err != nil { + if err := mountViaFDs("", nil, dest, dstFD, "", uintptr(pflag), ""); err != nil { return err } } @@ -733,7 +733,7 @@ func bindMountDeviceNode(rootfs, dest string, node *devices.Device) error { _ = f.Close() } return utils.WithProcfd(rootfs, dest, func(dstFD string) error { - return mountViaFDs(node.Path, "", dest, dstFD, "bind", unix.MS_BIND, "") + return mountViaFDs(node.Path, nil, dest, dstFD, "bind", unix.MS_BIND, "") }) } @@ -1154,7 +1154,7 @@ func mountPropagate(m mountEntry, rootfs string, mountLabel string) error { // target needs to be re-opened. if err := utils.WithProcfd(rootfs, m.Destination, func(dstFD string) error { for _, pflag := range m.PropagationFlags { - if err := mountViaFDs("", "", m.Destination, dstFD, "", uintptr(pflag), ""); err != nil { + if err := mountViaFDs("", nil, m.Destination, dstFD, "", uintptr(pflag), ""); err != nil { return err } }