Merge pull request #4734 from cyphar/mount-errors

rootfs: improve mount-related errors

Author: lifubang

libcontainer/mount_linux.go

@@ -6,6 +6,7 @@ import (
 	"io/fs"
 	"os"
 	"strconv"
+	"strings"
 
 	"github.com/sirupsen/logrus"
 	"golang.org/x/sys/unix"
@@ -45,6 +46,64 @@ type mountError struct {
 	err     error
 }
 
+// int32plus is a collection of int types with >=32 bits.
+type int32plus interface {
+	int | uint | int32 | uint32 | int64 | uint64 | uintptr
+}
+
+// stringifyMountFlags converts mount(2) flags to a string that you can use in
+// error messages.
+func stringifyMountFlags[Int int32plus](flags Int) string {
+	flagNames := []struct {
+		name string
+		bits Int
+	}{
+		{"MS_RDONLY", unix.MS_RDONLY},
+		{"MS_NOSUID", unix.MS_NOSUID},
+		{"MS_NODEV", unix.MS_NODEV},
+		{"MS_NOEXEC", unix.MS_NOEXEC},
+		{"MS_SYNCHRONOUS", unix.MS_SYNCHRONOUS},
+		{"MS_REMOUNT", unix.MS_REMOUNT},
+		{"MS_MANDLOCK", unix.MS_MANDLOCK},
+		{"MS_DIRSYNC", unix.MS_DIRSYNC},
+		{"MS_NOSYMFOLLOW", unix.MS_NOSYMFOLLOW},
+		// No (1 << 9) flag.
+		{"MS_NOATIME", unix.MS_NOATIME},
+		{"MS_NODIRATIME", unix.MS_NODIRATIME},
+		{"MS_BIND", unix.MS_BIND},
+		{"MS_MOVE", unix.MS_MOVE},
+		{"MS_REC", unix.MS_REC},
+		// MS_VERBOSE was deprecated and swapped to MS_SILENT.
+		{"MS_SILENT", unix.MS_SILENT},
+		{"MS_POSIXACL", unix.MS_POSIXACL},
+		{"MS_UNBINDABLE", unix.MS_UNBINDABLE},
+		{"MS_PRIVATE", unix.MS_PRIVATE},
+		{"MS_SLAVE", unix.MS_SLAVE},
+		{"MS_SHARED", unix.MS_SHARED},
+		{"MS_RELATIME", unix.MS_RELATIME},
+		// MS_KERNMOUNT (1 << 22) is internal to the kernel.
+		{"MS_I_VERSION", unix.MS_I_VERSION},
+		{"MS_STRICTATIME", unix.MS_STRICTATIME},
+		{"MS_LAZYTIME", unix.MS_LAZYTIME},
+	}
+	var (
+		flagSet  []string
+		seenBits Int
+	)
+	for _, flag := range flagNames {
+		if flags&flag.bits == flag.bits {
+			seenBits |= flag.bits
+			flagSet = append(flagSet, flag.name)
+		}
+	}
+	// If there were any remaining flags specified we don't know the name of,
+	// just add them in an 0x... format.
+	if remaining := flags &^ seenBits; remaining != 0 {
+		flagSet = append(flagSet, "0x"+strconv.FormatUint(uint64(remaining), 16))
+	}
+	return strings.Join(flagSet, "|")
+}
+
 // Error provides a string error representation.
 func (e *mountError) Error() string {
 	out := e.op + " "
@@ -62,7 +121,7 @@ func (e *mountError) Error() string {
 	}
 
 	if e.flags != uintptr(0) {
-		out += ", flags=0x" + strconv.FormatUint(uint64(e.flags), 16)
+		out += ", flags=" + stringifyMountFlags(e.flags)
 	}
 	if e.data != "" {
 		out += ", data=" + e.data

libcontainer/mount_linux_test.go

@@ -0,0 +1,54 @@
+package libcontainer
+
+import (
+	"testing"
+
+	"golang.org/x/sys/unix"
+)
+
+func TestStringifyMountFlags(t *testing.T) {
+	for _, test := range []struct {
+		name     string
+		flags    uintptr
+		expected string
+	}{
+		{"Empty", 0, ""},
+		// Single valid flags.
+		{"Single-MS_RDONLY", unix.MS_RDONLY, "MS_RDONLY"},
+		{"Single-MS_NOSUID", unix.MS_NOSUID, "MS_NOSUID"},
+		{"Single-MS_NODEV", unix.MS_NODEV, "MS_NODEV"},
+		{"Single-MS_NOEXEC", unix.MS_NOEXEC, "MS_NOEXEC"},
+		{"Single-MS_SYNCHRONOUS", unix.MS_SYNCHRONOUS, "MS_SYNCHRONOUS"},
+		{"Single-MS_REMOUNT", unix.MS_REMOUNT, "MS_REMOUNT"},
+		{"Single-MS_MANDLOCK", unix.MS_MANDLOCK, "MS_MANDLOCK"},
+		{"Single-MS_DIRSYNC", unix.MS_DIRSYNC, "MS_DIRSYNC"},
+		{"Single-MS_NOSYMFOLLOW", unix.MS_NOSYMFOLLOW, "MS_NOSYMFOLLOW"},
+		{"Single-MS_NOATIME", unix.MS_NOATIME, "MS_NOATIME"},
+		{"Single-MS_NODIRATIME", unix.MS_NODIRATIME, "MS_NODIRATIME"},
+		{"Single-MS_BIND", unix.MS_BIND, "MS_BIND"},
+		{"Single-MS_MOVE", unix.MS_MOVE, "MS_MOVE"},
+		{"Single-MS_REC", unix.MS_REC, "MS_REC"},
+		{"Single-MS_SILENT", unix.MS_SILENT, "MS_SILENT"},
+		{"Single-MS_POSIXACL", unix.MS_POSIXACL, "MS_POSIXACL"},
+		{"Single-MS_UNBINDABLE", unix.MS_UNBINDABLE, "MS_UNBINDABLE"},
+		{"Single-MS_PRIVATE", unix.MS_PRIVATE, "MS_PRIVATE"},
+		{"Single-MS_SLAVE", unix.MS_SLAVE, "MS_SLAVE"},
+		{"Single-MS_SHARED", unix.MS_SHARED, "MS_SHARED"},
+		{"Single-MS_RELATIME", unix.MS_RELATIME, "MS_RELATIME"},
+		{"Single-MS_KERNMOUNT", unix.MS_KERNMOUNT, "0x400000"},
+		{"Single-MS_I_VERSION", unix.MS_I_VERSION, "MS_I_VERSION"},
+		{"Single-MS_STRICTATIME", unix.MS_STRICTATIME, "MS_STRICTATIME"},
+		{"Single-MS_LAZYTIME", unix.MS_LAZYTIME, "MS_LAZYTIME"},
+		// Invalid flag value.
+		{"Unknown-512", 1 << 9, "0x200"},
+		// Multiple flag values at the same time.
+		{"Multiple-Valid1", unix.MS_RDONLY | unix.MS_REC | unix.MS_BIND, "MS_RDONLY|MS_BIND|MS_REC"},
+		{"Multiple-Valid2", unix.MS_NOSUID | unix.MS_NODEV | unix.MS_NOEXEC | unix.MS_REC | unix.MS_NODIRATIME | unix.MS_I_VERSION, "MS_NOSUID|MS_NODEV|MS_NOEXEC|MS_NODIRATIME|MS_REC|MS_I_VERSION"},
+		{"Multiple-Mixed", unix.MS_REC | unix.MS_BIND | (1 << 9) | (1 << 31), "MS_BIND|MS_REC|0x80000200"},
+	} {
+		got := stringifyMountFlags(test.flags)
+		if got != test.expected {
+			t.Errorf("%s: stringifyMountFlags(0x%x) = %q, expected %q", test.name, test.flags, got, test.expected)
+		}
+	}
+}

libcontainer/rootfs_linux.go

@@ -666,11 +666,17 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 					return err
 				}
 				srcFlags := statfsToMountFlags(*st)
+
+				logrus.Debugf(
+					"working around failure to set vfs flags on bind-mount %s: srcFlags=%s flagsSet=%s flagsClr=%s: %v",
+					m.Destination, stringifyMountFlags(srcFlags),
+					stringifyMountFlags(m.Flags), stringifyMountFlags(m.ClearedFlags), mountErr)
+
 				// If the user explicitly request one of the locked flags *not*
 				// be set, we need to return an error to avoid producing mounts
 				// that don't match the user's request.
-				if srcFlags&m.ClearedFlags&mntLockFlags != 0 {
-					return mountErr
+				if cannotClearFlags := srcFlags & m.ClearedFlags & mntLockFlags; cannotClearFlags != 0 {
+					return fmt.Errorf("cannot clear locked flags %s: %w", stringifyMountFlags(cannotClearFlags), mountErr)
 				}
 
 				// If an MS_*ATIME flag was requested, it must match the
@@ -691,17 +697,19 @@ func mountToRootfs(c *mountConfig, m mountEntry) error {
 				// MS_STRICTATIME mounts even if the user requested MS_RELATIME
 				// or MS_NOATIME.
 				if m.Flags&mntAtimeFlags != 0 && m.Flags&mntAtimeFlags != srcFlags&mntAtimeFlags {
-					return mountErr
+					return fmt.Errorf("cannot change locked atime flags %s: %w", stringifyMountFlags(srcFlags&mntAtimeFlags), mountErr)
 				}
 
 				// Retry the mount with the existing lockable mount flags
 				// applied.
 				flags |= srcFlags & mntLockFlags
 				mountErr = mountViaFds("", nil, m.Destination, dstFd, "", uintptr(flags), "")
-				logrus.Debugf("remount retry: srcFlags=0x%x flagsSet=0x%x flagsClr=0x%x: %v", srcFlags, m.Flags, m.ClearedFlags, mountErr)
+				if mountErr != nil {
+					mountErr = fmt.Errorf("remount with locked flags %s re-applied: %w", stringifyMountFlags(srcFlags&mntLockFlags), mountErr)
+				}
 				return mountErr
 			}); err != nil {
-				return err
+				return fmt.Errorf("failed to set user-requested vfs flags on bind-mount: %w", err)
 			}
 		}