clarify safemount behavior

PiperOrigin-RevId: 383750666

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, "/proc"); err != nil {
+	if err := specutils.SafeSetupAndMount(src, chrootDst, typ, flags, "/proc"); err != nil {
 		return fmt.Errorf("error mounting %q at %q: %v", src, chrootDst, err)
 	}
 	return nil

runsc/cmd/gofer.go

@@ -368,7 +368,7 @@ func setupMounts(conf *config.Config, mounts []specs.Mount, root, procPath strin
 		}
 
 		log.Infof("Mounting src: %q, dst: %q, flags: %#x", m.Source, dst, flags)
-		if err := specutils.Mount(m.Source, dst, m.Type, flags, procPath); err != nil {
+		if err := specutils.SafeSetupAndMount(m.Source, dst, m.Type, flags, procPath); err != nil {
 			return fmt.Errorf("mounting %+v: %v", m, err)
 		}
 

runsc/config/config.go

@@ -142,7 +142,8 @@ type Config struct {
 	// Rootless allows the sandbox to be started with a user that is not root.
 	// Defense in depth measures are weaker in rootless mode. Specifically, the
 	// sandbox and Gofer process run as root inside a user namespace with root
-	// mapped to the caller's user.
+	// mapped to the caller's user. When using rootless, the container root path
+	// should not have a symlink.
 	Rootless bool `flag:"rootless"`
 
 	// AlsoLogToStderr allows to send log messages to stderr.
@@ -175,7 +176,8 @@ type Config struct {
 	// TestOnlyAllowRunAsCurrentUserWithoutChroot should only be used in
 	// tests. It allows runsc to start the sandbox process as the current
 	// user, and without chrooting the sandbox process. This can be
-	// necessary in test environments that have limited capabilities.
+	// necessary in test environments that have limited capabilities. When
+	// disabling chroot, the container root path should not have a symlink.
 	TestOnlyAllowRunAsCurrentUserWithoutChroot bool `flag:"TESTONLY-unsafe-nonroot"`
 
 	// TestOnlyTestNameEnv should only be used in tests. It looks up for the

runsc/specutils/specutils.go

@@ -434,12 +434,12 @@ 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. 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).
+// SafeSetupAndMount 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 SafeSetupAndMount(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
 	if typ == "proc" {
 		// Special case, as there is no source directory for proc mounts.
@@ -484,6 +484,10 @@ type ErrSymlinkMount struct {
 
 // 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.
+//
+// SafeMount can fail when dst contains a symlink. However, it is called in the
+// normal case with a destination consisting of a known root (/proc/root) and
+// symlink-free path (from resolveSymlink).
 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)