rootfs: handle nested procfs mounts for MS_MOVE

In a case where the host /proc mount has already been overmounted, the
MS_MOVE handling would get ENOENT when trying to hide (for instance)
"/proc/bus" because it had already hidden away "/proc". This revealed
two issues in the previous implementation of this hardening feaure:

1. No checks were done to make sure the mount was a "full" mount (it is
   a mount of the root of the filesystem), but the kernel doesn't permit
   a non-full mount to be converted to a full mount (for reference, see
   mnt_already_visible). This just removes extra busy-work during setup.

2. ENOENT was treated as a critical error, even though it actually
   indicates the mount doesn't exist and thus isn't a problem. A more
   theoretically pure solution would be to store the set of mountpoints
   to be hidden and only ignore the error if an ancestor directory of
   the current mountpoint was already hidden, but that would just add
   complexity with little justification.

In addition, better document the reasoning behind this logic so that
folks aren't confused when looking at it.

Fixes: 28a697c ("rootfs: umount all procfs and sysfs with --no-pivot")
Signed-off-by: Aleksa Sarai <[email protected]>

libcontainer/rootfs_linux.go

@@ -809,24 +809,46 @@ func pivotRoot(rootfs string) error {
 }
 
 func msMoveRoot(rootfs string) error {
+	// Before we move the root and chroot we have to mask all "full" sysfs and
+	// procfs mounts which exist on the host. This is because while the kernel
+	// has protections against mounting procfs if it has masks, when using
+	// chroot(2) the *host* procfs mount is still reachable in the mount
+	// namespace and the kernel permits procfs mounts inside --no-pivot
+	// containers.
+	//
+	// Users shouldn't be using --no-pivot except in exceptional circumstances,
+	// but to avoid such a trivial security flaw we apply a best-effort
+	// protection here. The kernel only allows a mount of a pseudo-filesystem
+	// like procfs or sysfs if there is a *full* mount (the root of the
+	// filesystem is mounted) without any other locked mount points covering a
+	// subtree of the mount.
+	//
+	// So we try to unmount (or mount tmpfs on top of) any mountpoint which is
+	// a full mount of either sysfs or procfs (since those are the most
+	// concerning filesystems to us).
 	mountinfos, err := mountinfo.GetMounts(func(info *mountinfo.Info) (skip, stop bool) {
-		skip = false
-		stop = false
-		// Collect every sysfs and proc file systems, except those under the container rootfs
-		if (info.FSType != "proc" && info.FSType != "sysfs") || strings.HasPrefix(info.Mountpoint, rootfs) {
+		// Collect every sysfs and procfs filesystem, except for those which
+		// are non-full mounts or are inside the rootfs of the container.
+		if info.Root != "/" ||
+			(info.FSType != "proc" && info.FSType != "sysfs") ||
+			strings.HasPrefix(info.Mountpoint, rootfs) {
 			skip = true
-			return
 		}
 		return
 	})
 	if err != nil {
 		return err
 	}
-
 	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 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.
+				continue
+			}
 			return err
 		}
 		if err := unix.Unmount(p, unix.MNT_DETACH); err != nil {
@@ -841,6 +863,8 @@ 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 {
 		return err
 	}