Merge release-20210628.0-23-g3d32a05a3 (automated)

3 files changed, 59 insertions, 19 deletions

diff --git a/runsc/cmd/chroot.go b/runsc/cmd/chroot.go
index e988247da..791a50135 100644
--- a/runsc/cmd/chroot.go
+++ b/runsc/cmd/chroot.go
@@ -30,7 +30,7 @@ func mountInChroot(chroot, src, dst, typ string, flags uint32) error {
 	chrootDst := filepath.Join(chroot, dst)
 	log.Infof("Mounting %q at %q", src, chrootDst)
 
-	if err := specutils.Mount(src, chrootDst, typ, flags); err != nil {
+	if err := specutils.Mount(src, chrootDst, typ, flags, "/proc"); err != nil {
 		return fmt.Errorf("error mounting %q at %q: %v", src, chrootDst, err)
 	}
 	return nil
@@ -70,11 +70,11 @@ func setUpChroot(pidns bool) error {
 
 	// Convert all shared mounts into slave to be sure that nothing will be
 	// propagated outside of our namespace.
-	if err := unix.Mount("", "/", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil {
+	if err := specutils.SafeMount("", "/", "", unix.MS_SLAVE|unix.MS_REC, "", "/proc"); err != nil {
 		return fmt.Errorf("error converting mounts: %v", err)
 	}
 
-	if err := unix.Mount("runsc-root", chroot, "tmpfs", unix.MS_NOSUID|unix.MS_NODEV|unix.MS_NOEXEC, ""); err != nil {
+	if err := specutils.SafeMount("runsc-root", chroot, "tmpfs", unix.MS_NOSUID|unix.MS_NODEV|unix.MS_NOEXEC, "", "/proc"); err != nil {
 		return fmt.Errorf("error mounting tmpfs in choot: %v", err)
 	}
 
@@ -89,7 +89,7 @@ func setUpChroot(pidns bool) error {
 		}
 	}
 
-	if err := unix.Mount("", chroot, "", unix.MS_REMOUNT|unix.MS_RDONLY|unix.MS_BIND, ""); err != nil {
+	if err := specutils.SafeMount("", chroot, "", unix.MS_REMOUNT|unix.MS_RDONLY|unix.MS_BIND, "", "/proc"); err != nil {
 		return fmt.Errorf("error remounting chroot in read-only: %v", err)
 	}
 
diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go
index 80da9c9a2..570df407c 100644
--- a/runsc/cmd/gofer.go
+++ b/runsc/cmd/gofer.go
@@ -265,7 +265,8 @@ func isReadonlyMount(opts []string) bool {
 func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 	// Convert all shared mounts into slaves to be sure that nothing will be
 	// propagated outside of our namespace.
-	if err := unix.Mount("", "/", "", unix.MS_SLAVE|unix.MS_REC, ""); err != nil {
+	procPath := "/proc"
+	if err := specutils.SafeMount("", "/", "", unix.MS_SLAVE|unix.MS_REC, "", procPath); err != nil {
 		Fatalf("error converting mounts: %v", err)
 	}
 
@@ -278,21 +279,24 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 		// We need a directory to construct a new root and we know that
 		// runsc can't start without /proc, so we can use it for this.
 		flags := uintptr(unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC)
-		if err := unix.Mount("runsc-root", "/proc", "tmpfs", flags, ""); err != nil {
+		if err := specutils.SafeMount("runsc-root", "/proc", "tmpfs", flags, "", procPath); err != nil {
 			Fatalf("error mounting tmpfs: %v", err)
 		}
 
 		// Prepare tree structure for pivot_root(2).
 		os.Mkdir("/proc/proc", 0755)
 		os.Mkdir("/proc/root", 0755)
+		// This cannot use SafeMount because there's no available procfs. But we
+		// know that /proc is an empty tmpfs mount, so this is safe.
 		if err := unix.Mount("runsc-proc", "/proc/proc", "proc", flags|unix.MS_RDONLY, ""); err != nil {
 			Fatalf("error mounting proc: %v", err)
 		}
 		root = "/proc/root"
+		procPath = "/proc/proc"
 	}
 
 	// Mount root path followed by submounts.
-	if err := unix.Mount(spec.Root.Path, root, "bind", unix.MS_BIND|unix.MS_REC, ""); err != nil {
+	if err := specutils.SafeMount(spec.Root.Path, root, "bind", unix.MS_BIND|unix.MS_REC, "", procPath); err != nil {
 		return fmt.Errorf("mounting root on root (%q) err: %v", root, err)
 	}
 
@@ -300,12 +304,12 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 	if spec.Linux != nil && spec.Linux.RootfsPropagation != "" {
 		flags = specutils.PropOptionsToFlags([]string{spec.Linux.RootfsPropagation})
 	}
-	if err := unix.Mount("", root, "", uintptr(flags), ""); err != nil {
+	if err := specutils.SafeMount("", root, "", uintptr(flags), "", procPath); err != nil {
 		return fmt.Errorf("mounting root (%q) with flags: %#x, err: %v", root, flags, err)
 	}
 
 	// Replace the current spec, with the clean spec with symlinks resolved.
-	if err := setupMounts(conf, spec.Mounts, root); err != nil {
+	if err := setupMounts(conf, spec.Mounts, root, procPath); err != nil {
 		Fatalf("error setting up FS: %v", err)
 	}
 
@@ -327,7 +331,7 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 		// to make it read-only for extra safety.
 		log.Infof("Remounting root as readonly: %q", root)
 		flags := uintptr(unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY | unix.MS_REC)
-		if err := unix.Mount(root, root, "bind", flags, ""); err != nil {
+		if err := specutils.SafeMount(root, root, "bind", flags, "", procPath); err != nil {
 			return fmt.Errorf("remounting root as read-only with source: %q, target: %q, flags: %#x, err: %v", root, root, flags, err)
 		}
 	}
@@ -343,10 +347,10 @@ func setupRootFS(spec *specs.Spec, conf *config.Config) error {
 	return nil
 }
 
-// setupMounts binds mount all mounts specified in the spec in their correct
+// setupMounts bind mounts all mounts specified in the spec in their correct
 // location inside root. It will resolve relative paths and symlinks. It also
 // creates directories as needed.
-func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
+func setupMounts(conf *config.Config, mounts []specs.Mount, root, procPath string) error {
 	for _, m := range mounts {
 		if !specutils.Is9PMount(m, conf.VFS2) {
 			continue
@@ -364,14 +368,14 @@ func setupMounts(conf *config.Config, mounts []specs.Mount, root string) error {
 		}
 
 		log.Infof("Mounting src: %q, dst: %q, flags: %#x", m.Source, dst, flags)
-		if err := specutils.Mount(m.Source, dst, m.Type, flags); err != nil {
-			return fmt.Errorf("mounting %v: %v", m, err)
+		if err := specutils.Mount(m.Source, dst, m.Type, flags, procPath); err != nil {
+			return fmt.Errorf("mounting %+v: %v", m, err)
 		}
 
 		// Set propagation options that cannot be set together with other options.
 		flags = specutils.PropOptionsToFlags(m.Options)
 		if flags != 0 {
-			if err := unix.Mount("", dst, "", uintptr(flags), ""); err != nil {
+			if err := specutils.SafeMount("", dst, "", uintptr(flags), "", procPath); err != nil {
 				return fmt.Errorf("mount dst: %q, flags: %#x, err: %v", dst, flags, err)
 			}
 		}
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index c228d6299..48a574373 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -217,7 +217,7 @@ func ReadMounts(f *os.File) ([]specs.Mount, error) {
 	}
 	var mounts []specs.Mount
 	if err := json.Unmarshal(bytes, &mounts); err != nil {
-		return nil, fmt.Errorf("error unmarshaling mounts: %v\n %s", err, string(bytes))
+		return nil, fmt.Errorf("error unmarshaling mounts: %v\nJSON bytes:\n%s", err, string(bytes))
 	}
 	return mounts, nil
 }
@@ -434,8 +434,10 @@ func DebugLogFile(logPattern, command, test string) (*os.File, error) {
 	return os.OpenFile(logPattern, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
 }
 
-// Mount creates the mount point and calls Mount with the given flags.
-func Mount(src, dst, typ string, flags uint32) error {
+// Mount creates the mount point and calls Mount with the given flags. procPath
+// is the path to procfs. If it is "", procfs is assumed to be mounted at
+// /proc.
+func Mount(src, dst, typ string, flags uint32, procPath string) error {
 	// Create the mount point inside. The type must be the same as the
 	// source (file or directory).
 	var isDir bool
@@ -468,12 +470,46 @@ func Mount(src, dst, typ string, flags uint32) error {
 	}
 
 	// Do the mount.
-	if err := unix.Mount(src, dst, typ, uintptr(flags), ""); err != nil {
+	if err := SafeMount(src, dst, typ, uintptr(flags), "", procPath); err != nil {
 		return fmt.Errorf("mount(%q, %q, %d) failed: %v", src, dst, flags, err)
 	}
 	return nil
 }
 
+// ErrSymlinkMount is returned by SafeMount when the mount destination is found
+// to be a symlink.
+type ErrSymlinkMount struct {
+	error
+}
+
+// SafeMount is like unix.Mount, but will fail if dst is a symlink. procPath is
+// the path to procfs. If it is "", procfs is assumed to be mounted at /proc.
+func SafeMount(src, dst, fstype string, flags uintptr, data, procPath string) error {
+	// Open the destination.
+	fd, err := unix.Open(dst, unix.O_PATH|unix.O_CLOEXEC, 0)
+	if err != nil {
+		return fmt.Errorf("failed to safely mount: Open(%s, _, _): %w", dst, err)
+	}
+	defer unix.Close(fd)
+
+	// Use /proc/self/fd/ to verify that we opened the intended destination. This
+	// guards against dst being a symlink, in which case we could accidentally
+	// mount over the symlink's target.
+	if procPath == "" {
+		procPath = "/proc"
+	}
+	safePath := filepath.Join(procPath, "self/fd", strconv.Itoa(fd))
+	target, err := os.Readlink(safePath)
+	if err != nil {
+		return fmt.Errorf("failed to safely mount: Readlink(%s): %w", safePath, err)
+	}
+	if dst != target {
+		return &ErrSymlinkMount{fmt.Errorf("failed to safely mount: expected to open %s, but found %s", dst, target)}
+	}
+
+	return unix.Mount(src, safePath, fstype, flags, data)
+}
+
 // ContainsStr returns true if 'str' is inside 'strs'.
 func ContainsStr(strs []string, str string) bool {
 	for _, s := range strs {