Skip to content

Commit

Permalink
Use an int for srcFD.
Browse files Browse the repository at this point in the history
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 -1.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
  • Loading branch information
eiffel-fl committed Jul 18, 2023
1 parent 07e0ccf commit 9a7cce4
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 29 deletions.
5 changes: 1 addition & 4 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, -1, m.Destination, dstFD, "", unix.MS_BIND|unix.MS_REC, "")
}); err != nil {
return err
}
Expand Down
22 changes: 12 additions & 10 deletions libcontainer/mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
type mountError struct {
op string
source string
srcFD string
srcFD int
target string
dstFD string
flags uintptr
Expand All @@ -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 != -1 {
out += "srcFD=" + strconv.Itoa(e.srcFD) + ", "
}
}
out += "dst=" + e.target
Expand Down Expand Up @@ -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, -1, 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 -1, 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 != -1 {
src = "/proc/self/fd/" + strconv.Itoa(srcFD)
}
dst := target
if dstFD != "" {
Expand Down
30 changes: 15 additions & 15 deletions libcontainer/rootfs_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != -1 {
return "/proc/self/fd/" + strconv.Itoa(m.srcFD)
}
return m.Source
}
Expand Down Expand Up @@ -86,19 +86,19 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds mountFds) (
cgroupns: config.Namespaces.Contains(configs.NEWCGROUP),
}
for i, m := range config.Mounts {
entry := mountEntry{Mount: m, idmapFD: -1}
entry := mountEntry{Mount: m, srcFD: -1, idmapFD: -1}
// 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].
if mountFds.idmapFds != nil && mountFds.idmapFds[i] != -1 {
entry.idmapFD = mountFds.idmapFds[i]
}

if entry.idmapFD != -1 && entry.srcFD != "" {
if entry.idmapFD != -1 && entry.srcFD != -1 {
return fmt.Errorf("malformed mountFds and idmapFds slice, entry: %v has fds in both slices", i)
}

Expand Down Expand Up @@ -274,7 +274,7 @@ func mountCgroupV1(m *configs.Mount, c *mountConfig) error {
PropagationFlags: m.PropagationFlags,
}

if err := mountToRootfs(c, mountEntry{Mount: tmpfs}); err != nil {
if err := mountToRootfs(c, mountEntry{Mount: tmpfs, srcFD: -1}); err != nil {
return err
}

Expand All @@ -297,12 +297,12 @@ 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, -1, b.Destination, dstFD, "cgroup", uintptr(flags), data)
}); err != nil {
return err
}
} else {
if err := mountToRootfs(c, mountEntry{Mount: b}); err != nil {
if err := mountToRootfs(c, mountEntry{Mount: b, srcFD: -1}); err != nil {
return err
}
}
Expand All @@ -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, -1, m.Destination, dstFD, "cgroup2", uintptr(m.Flags), m.Data)
})
if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) {
return err
Expand All @@ -351,7 +351,7 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error {
}
// mountToRootfs() handles remounting for MS_RDONLY.
// No need to set mountEntry.srcFD here, because mountToRootfs() calls utils.WithProcfd() by itself in mountPropagate().
err = mountToRootfs(c, mountEntry{Mount: bindM})
err = mountToRootfs(c, mountEntry{Mount: bindM, srcFD: -1})
if c.rootlessCgroups && errors.Is(err, unix.ENOENT) {
// ENOENT (for `src = c.cgroup2Path`) happens when rootless runc is being executed
// outside the userns+mountns.
Expand Down Expand Up @@ -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, -1, m.Destination, dstFD, "", unix.MS_MOVE, ""); err != nil {
return fmt.Errorf("tmpcopyup: failed to move mount: %w", err)
}
return nil
Expand Down Expand Up @@ -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("", -1, dest, dstFD, "", uintptr(pflag), ""); err != nil {
return err
}
}
Expand Down Expand Up @@ -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, -1, dest, dstFD, "bind", unix.MS_BIND, "")
})
}

Expand Down Expand Up @@ -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("", -1, m.Destination, dstFD, "", uintptr(pflag), ""); err != nil {
return err
}
}
Expand Down

0 comments on commit 9a7cce4

Please sign in to comment.