From 36aefad45dddf2f83b0af1a10ee3d9223a6ade87 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 22 Jun 2021 15:33:09 -0700 Subject: [PATCH] libct: wrap unix.Mount/Unmount errors Errors returned by unix are bare. In some cases it's impossible to find out what went wrong because there's is not enough context. Add a mountError type (mostly copy-pasted from github.com/moby/sys/mount), and mount/unmount helpers. Use these where appropriate, and convert error checks to use errors.Is. Signed-off-by: Kir Kolyshkin --- libcontainer/console_linux.go | 2 +- libcontainer/container_linux.go | 6 +-- libcontainer/factory_linux.go | 2 +- libcontainer/mount_linux.go | 89 +++++++++++++++++++++++++++++++++ libcontainer/rootfs_linux.go | 83 +++++++++++++++--------------- 5 files changed, 135 insertions(+), 47 deletions(-) create mode 100644 libcontainer/mount_linux.go diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index 7bfff026a..29b9c3b08 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -18,7 +18,7 @@ func mountConsole(slavePath string) error { if f != nil { f.Close() } - return unix.Mount(slavePath, "/dev/console", "bind", unix.MS_BIND, "") + return mount(slavePath, "/dev/console", "", "bind", unix.MS_BIND, "") } // dupStdio opens the slavePath for the console and dups the fds to the current diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 31f02df1e..5ebac4df6 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -1282,8 +1282,8 @@ func (c *linuxContainer) 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(procfd string) error { - if err := unix.Mount(m.Source, procfd, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { - return errorsf.Wrapf(err, "unable to bind mount %q to %q (through %q)", m.Source, m.Destination, procfd) + if err := mount(m.Source, m.Destination, procfd, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { + return err } return nil }); err != nil { @@ -1346,7 +1346,7 @@ func (c *linuxContainer) Restore(process *Process, criuOpts *CriuOpts) error { if err != nil { return err } - err = unix.Mount(c.config.Rootfs, root, "", unix.MS_BIND|unix.MS_REC, "") + err = mount(c.config.Rootfs, root, "", "", unix.MS_BIND|unix.MS_REC, "") if err != nil { return err } diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index 726924320..3d9a45dc3 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -168,7 +168,7 @@ func TmpfsRoot(l *LinuxFactory) error { return err } if !mounted { - if err := unix.Mount("tmpfs", l.Root, "tmpfs", 0, ""); err != nil { + if err := mount("tmpfs", l.Root, "", "tmpfs", 0, ""); err != nil { return err } } diff --git a/libcontainer/mount_linux.go b/libcontainer/mount_linux.go new file mode 100644 index 000000000..e6e90d453 --- /dev/null +++ b/libcontainer/mount_linux.go @@ -0,0 +1,89 @@ +package libcontainer + +import ( + "strconv" + + "golang.org/x/sys/unix" +) + +// mountError holds an error from a failed mount or unmount operation. +type mountError struct { + op string + source string + target string + procfd string + flags uintptr + data string + err error +} + +// Error provides a string error representation. +func (e *mountError) Error() string { + out := e.op + " " + + if e.source != "" { + out += e.source + ":" + e.target + } else { + out += e.target + } + if e.procfd != "" { + out += " (via " + e.procfd + ")" + } + + if e.flags != uintptr(0) { + out += ", flags: 0x" + strconv.FormatUint(uint64(e.flags), 16) + } + if e.data != "" { + out += ", data: " + e.data + } + + out += ": " + e.err.Error() + return out +} + +// Cause returns the underlying cause of the error. +// This is a convention used by github.com/pkg/errors. +func (e *mountError) Cause() error { + return e.err +} + +// Unwrap returns the underlying error. +// This is a convention used by Go 1.13+ standard library. +func (e *mountError) Unwrap() error { + return e.err +} + +// mount is a simple unix.Mount wrapper. If procfd is not empty, it is used +// instead of target (and the target is only used to add context to an error). +func mount(source, target, procfd, fstype string, flags uintptr, data string) error { + dst := target + if procfd != "" { + dst = procfd + } + if err := unix.Mount(source, dst, fstype, flags, data); err != nil { + return &mountError{ + op: "mount", + source: source, + target: target, + procfd: procfd, + flags: flags, + data: data, + err: err, + } + } + return nil +} + +// unmount is a simple unix.Unmount wrapper. +func unmount(target string, flags int) error { + err := unix.Unmount(target, flags) + if err != nil { + return &mountError{ + op: "unmount", + target: target, + flags: uintptr(flags), + err: err, + } + } + return nil +} diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 430f490de..ccee60fa3 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -3,6 +3,7 @@ package libcontainer import ( + "errors" "fmt" "io" "io/ioutil" @@ -187,10 +188,10 @@ func prepareTmp(topTmpDir string) (string, error) { if err != nil { return "", err } - if err := unix.Mount(tmpdir, tmpdir, "bind", unix.MS_BIND, ""); err != nil { + if err := mount(tmpdir, tmpdir, "", "bind", unix.MS_BIND, ""); err != nil { return "", err } - if err := unix.Mount("", tmpdir, "", uintptr(unix.MS_PRIVATE), ""); err != nil { + if err := mount("", tmpdir, "", "", uintptr(unix.MS_PRIVATE), ""); err != nil { return "", err } return tmpdir, nil @@ -278,7 +279,7 @@ func mountCgroupV1(m *configs.Mount, c *mountConfig) error { data = cgroups.CgroupNamePrefix + data source = "systemd" } - return unix.Mount(source, procfd, "cgroup", uintptr(flags), data) + return mount(source, b.Destination, procfd, "cgroup", uintptr(flags), data) }); err != nil { return err } @@ -310,9 +311,9 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { return err } return utils.WithProcfd(c.root, m.Destination, func(procfd string) error { - if err := unix.Mount(m.Source, procfd, "cgroup2", uintptr(m.Flags), m.Data); err != nil { + if err := mount(m.Source, m.Destination, procfd, "cgroup2", uintptr(m.Flags), m.Data); err != nil { // when we are in UserNS but CgroupNS is not unshared, we cannot mount cgroup2 (#2158) - if err == unix.EPERM || err == unix.EBUSY { + if errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY) { src := fs2.UnifiedMountpoint if c.cgroupns && c.cgroup2Path != "" { // Emulate cgroupns by bind-mounting @@ -320,8 +321,8 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { // the whole /sys/fs/cgroup. src = c.cgroup2Path } - err = unix.Mount(src, procfd, "", uintptr(m.Flags)|unix.MS_BIND, "") - if err == unix.ENOENT && c.rootlessCgroups { + err = mount(src, m.Destination, procfd, "", uintptr(m.Flags)|unix.MS_BIND, "") + if c.rootlessCgroups && errors.Is(err, unix.ENOENT) { err = nil } } @@ -355,8 +356,8 @@ func doTmpfsCopyUp(m *configs.Mount, rootfs, mountLabel string) (Err error) { } defer func() { if Err != nil { - if err := unix.Unmount(tmpDir, unix.MNT_DETACH); err != nil { - logrus.Warnf("tmpcopyup: failed to unmount tmpdir on error: %v", err) + if err := unmount(tmpDir, unix.MNT_DETACH); err != nil { + logrus.Warnf("tmpcopyup: %v", err) } } }() @@ -369,8 +370,8 @@ func doTmpfsCopyUp(m *configs.Mount, rootfs, mountLabel string) (Err error) { return fmt.Errorf("tmpcopyup: failed to copy %s to %s (%s): %w", m.Destination, procfd, tmpDir, err) } // Now move the mount into the container. - if err := unix.Mount(tmpDir, procfd, "", unix.MS_MOVE, ""); err != nil { - return fmt.Errorf("tmpcopyup: failed to move mount %s to %s (%s): %w", tmpDir, procfd, m.Destination, err) + if err := mount(tmpDir, m.Destination, procfd, "", unix.MS_MOVE, ""); err != nil { + return fmt.Errorf("tmpcopyup: failed to move mount: %w", err) } return nil }) @@ -664,7 +665,7 @@ func bindMountDeviceNode(rootfs, dest string, node *devices.Device) error { _ = f.Close() } return utils.WithProcfd(rootfs, dest, func(procfd string) error { - return unix.Mount(node.Path, procfd, "bind", unix.MS_BIND, "") + return mount(node.Path, dest, procfd, "bind", unix.MS_BIND, "") }) } @@ -761,7 +762,7 @@ func rootfsParentMountPrivate(rootfs string) error { // shared. Secondly when we bind mount rootfs it will propagate to // parent namespace and we don't want that to happen. if sharedMount { - return unix.Mount("", parentMount, "", unix.MS_PRIVATE, "") + return mount("", parentMount, "", "", unix.MS_PRIVATE, "") } return nil @@ -772,7 +773,7 @@ func prepareRoot(config *configs.Config) error { if config.RootPropagation != 0 { flag = config.RootPropagation } - if err := unix.Mount("", "/", "", uintptr(flag), ""); err != nil { + if err := mount("", "/", "", "", uintptr(flag), ""); err != nil { return err } @@ -783,13 +784,13 @@ func prepareRoot(config *configs.Config) error { return err } - return unix.Mount(config.Rootfs, config.Rootfs, "bind", unix.MS_BIND|unix.MS_REC, "") + return mount(config.Rootfs, config.Rootfs, "", "bind", unix.MS_BIND|unix.MS_REC, "") } func setReadonly() error { flags := uintptr(unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY) - err := unix.Mount("", "/", "", flags, "") + err := mount("", "/", "", "", flags, "") if err == nil { return nil } @@ -798,7 +799,7 @@ func setReadonly() error { return &os.PathError{Op: "statfs", Path: "/", Err: err} } flags |= uintptr(s.Flags) - return unix.Mount("", "/", "", flags, "") + return mount("", "/", "", "", flags, "") } func setupPtmx(config *configs.Config) error { @@ -856,11 +857,11 @@ func pivotRoot(rootfs string) error { // known to cause issues due to races where we still have a reference to a // mount while a process in the host namespace are trying to operate on // something they think has no mounts (devicemapper in particular). - if err := unix.Mount("", ".", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil { + if err := mount("", ".", "", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil { return err } // Preform the unmount. MNT_DETACH allows us to unmount /proc/self/cwd. - if err := unix.Unmount(".", unix.MNT_DETACH); err != nil { + if err := unmount(".", unix.MNT_DETACH); err != nil { return err } @@ -905,8 +906,8 @@ func msMoveRoot(rootfs string) error { for _, info := range mountinfos { p := info.Mountpoint // Be sure umount events are not propagated to the host. - if err := unix.Mount("", p, "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil { - if err == unix.ENOENT { + if err := mount("", p, "", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil { + if errors.Is(err, unix.ENOENT) { // If the mountpoint doesn't exist that means that we've // already blasted away some parent directory of the mountpoint // and so we don't care about this error. @@ -914,13 +915,13 @@ func msMoveRoot(rootfs string) error { } return err } - if err := unix.Unmount(p, unix.MNT_DETACH); err != nil { - if err != unix.EINVAL && err != unix.EPERM { + if err := unmount(p, unix.MNT_DETACH); err != nil { + if !errors.Is(err, unix.EINVAL) && !errors.Is(err, unix.EPERM) { return err } else { // If we have not privileges for umounting (e.g. rootless), then // cover the path. - if err := unix.Mount("tmpfs", p, "tmpfs", 0, ""); err != nil { + if err := mount("tmpfs", p, "", "tmpfs", 0, ""); err != nil { return err } } @@ -928,7 +929,7 @@ func msMoveRoot(rootfs string) error { } // Move the rootfs on top of "/" in our mount namespace. - if err := unix.Mount(rootfs, "/", "", unix.MS_MOVE, ""); err != nil { + if err := mount(rootfs, "/", "", "", unix.MS_MOVE, ""); err != nil { return err } return chroot() @@ -963,11 +964,11 @@ func createIfNotExists(path string, isDir bool) error { // readonlyPath will make a path read only. func readonlyPath(path string) error { - if err := unix.Mount(path, path, "", unix.MS_BIND|unix.MS_REC, ""); err != nil { - if os.IsNotExist(err) { + if err := mount(path, path, "", "", unix.MS_BIND|unix.MS_REC, ""); err != nil { + if errors.Is(err, os.ErrNotExist) { return nil } - return &os.PathError{Op: "bind-mount", Path: path, Err: err} + return err } var s unix.Statfs_t @@ -976,8 +977,8 @@ func readonlyPath(path string) error { } flags := uintptr(s.Flags) & (unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC) - if err := unix.Mount(path, path, "", flags|unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, ""); err != nil { - return &os.PathError{Op: "bind-mount-ro", Path: path, Err: err} + if err := mount(path, path, "", "", flags|unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, ""); err != nil { + return err } return nil @@ -997,14 +998,12 @@ func remountReadonly(m *configs.Mount) error { // nosuid, etc.). So, let's use that case so that we can do // this re-mount without failing in a userns. flags |= unix.MS_REMOUNT | unix.MS_BIND | unix.MS_RDONLY - if err := unix.Mount("", dest, "", uintptr(flags), ""); err != nil { - switch err { - case unix.EBUSY: + if err := mount("", dest, "", "", uintptr(flags), ""); err != nil { + if errors.Is(err, unix.EBUSY) { time.Sleep(100 * time.Millisecond) continue - default: - return err } + return err } return nil } @@ -1017,9 +1016,9 @@ func remountReadonly(m *configs.Mount) error { // For files, maskPath bind mounts /dev/null over the top of the specified path. // For directories, maskPath mounts read-only tmpfs over the top of the specified path. func maskPath(path string, mountLabel string) error { - if err := unix.Mount("/dev/null", path, "", unix.MS_BIND, ""); err != nil && !os.IsNotExist(err) { - if err == unix.ENOTDIR { - return unix.Mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("", mountLabel)) + if err := mount("/dev/null", path, "", "", unix.MS_BIND, ""); err != nil && !errors.Is(err, os.ErrNotExist) { + if errors.Is(err, unix.ENOTDIR) { + return mount("tmpfs", path, "", "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("", mountLabel)) } return err } @@ -1035,7 +1034,7 @@ func writeSystemProperty(key, value string) error { func remount(m *configs.Mount, rootfs string) error { return utils.WithProcfd(rootfs, m.Destination, func(procfd string) error { - return unix.Mount(m.Source, procfd, m.Device, uintptr(m.Flags|unix.MS_REMOUNT), "") + return mount(m.Source, m.Destination, procfd, m.Device, uintptr(m.Flags|unix.MS_REMOUNT), "") }) } @@ -1059,16 +1058,16 @@ func mountPropagate(m *configs.Mount, rootfs string, mountLabel string) error { // inside the container with WithProcfd() -- mounting through a procfd // mounts on the target. if err := utils.WithProcfd(rootfs, m.Destination, func(procfd string) error { - return unix.Mount(m.Source, procfd, m.Device, uintptr(flags), data) + return mount(m.Source, m.Destination, procfd, m.Device, uintptr(flags), data) }); err != nil { - return fmt.Errorf("mount through procfd: %w", err) + return err } // We have to apply mount propagation flags in a separate WithProcfd() call // because the previous call invalidates the passed procfd -- the mount // target needs to be re-opened. if err := utils.WithProcfd(rootfs, m.Destination, func(procfd string) error { for _, pflag := range m.PropagationFlags { - if err := unix.Mount("", procfd, "", uintptr(pflag), ""); err != nil { + if err := mount("", m.Destination, procfd, "", uintptr(pflag), ""); err != nil { return err } }