Umount procfs from gofer process before starting LISAFS server.

fsgofer saves an open FD to /proc/self/fd and uses it when required. The procfs
mount at /proc is not required by the gofer process. It is also not accessible
by the gofer process because the gofer chroot(2)s into the container filesystem
directory before running the LISAFS server.

However, if the gofer were to donate a directory FD to the sandbox process, the
sandbox could walk backwards and discover the procfs. This is because the
chroot only effects the gofer process. The sandbox is running a different
chroot. We don't want a compromised sandbox to have access to gofer's procfs
because it open up vulnerabilities around /proc/self/exe.

Note that this is not an issue today because:
1. The gofer never donates directory FDs to the sandbox.
2. The sandbox seccomp filters do not allow openat(2) syscall.

This is in preparation for directfs work which will donate directory FDs and
also allow openat(2) from the sandbox.

Reported-by: Andrei Vagin <avagin@gmail.com>
PiperOrigin-RevId: 512725221

runsc/cmd/boot.go

@@ -217,9 +217,9 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma
 			// /proc is umounted from a forked process, because the
 			// current one is going to re-execute itself without
 			// capabilities.
-			cmd, w := b.execProcUmounter()
-			defer w.Close()
+			cmd, w := execProcUmounter()
 			defer cmd.Wait()
+			defer w.Close()
 			if b.procMountSyncFD != -1 {
 				panic("procMountSyncFD is set")
 			}
@@ -367,23 +367,8 @@ func (b *Boot) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomma
 
 	if b.procMountSyncFD != -1 {
 		l.PreSeccompCallback = func() {
-			syncFile := os.NewFile(uintptr(b.procMountSyncFD), "sync file")
-			buf := make([]byte, 1)
-			if w, err := syncFile.Write(buf); err != nil || w != 1 {
-				util.Fatalf("unable to write into the proc umounter descriptor: %v", err)
-			}
-			syncFile.Close()
-
-			var waitStatus unix.WaitStatus
-			if _, err := unix.Wait4(0, &waitStatus, 0, nil); err != nil {
-				util.Fatalf("error waiting for the proc umounter process: %v", err)
-			}
-			if !waitStatus.Exited() || waitStatus.ExitStatus() != 0 {
-				util.Fatalf("the proc umounter process failed: %v", waitStatus)
-			}
-			if err := unix.Access("/proc/self", unix.F_OK); err != unix.ENOENT {
-				util.Fatalf("/proc is still accessible")
-			}
+			// Umount /proc right before installing seccomp filters.
+			umountProc(b.procMountSyncFD)
 		}
 	}
 
@@ -450,9 +435,9 @@ func (b *Boot) prepareArgs(exclude ...string) []string {
 	return args
 }
 
-// execProcUmounter execute a child process that umounts /proc when the sks[1]
-// socket is closed.
-func (b *Boot) execProcUmounter() (*exec.Cmd, *os.File) {
+// execProcUmounter execute a child process that umounts /proc when the
+// returned pipe is closed.
+func execProcUmounter() (*exec.Cmd, *os.File) {
 	r, w, err := os.Pipe()
 	if err != nil {
 		util.Fatalf("error creating a pipe: %v", err)
@@ -470,3 +455,25 @@ func (b *Boot) execProcUmounter() (*exec.Cmd, *os.File) {
 	}
 	return cmd, w
 }
+
+// umountProc writes to syncFD signalling the process started by
+// execProcUmounter() to umount /proc.
+func umountProc(syncFD int) {
+	syncFile := os.NewFile(uintptr(syncFD), "sync file")
+	buf := make([]byte, 1)
+	if w, err := syncFile.Write(buf); err != nil || w != 1 {
+		util.Fatalf("unable to write into the proc umounter descriptor: %v", err)
+	}
+	syncFile.Close()
+
+	var waitStatus unix.WaitStatus
+	if _, err := unix.Wait4(0, &waitStatus, 0, nil); err != nil {
+		util.Fatalf("error waiting for the proc umounter process: %v", err)
+	}
+	if !waitStatus.Exited() || waitStatus.ExitStatus() != 0 {
+		util.Fatalf("the proc umounter process failed: %v", waitStatus)
+	}
+	if err := unix.Access("/proc/self", unix.F_OK); err != unix.ENOENT {
+		util.Fatalf("/proc is still accessible")
+	}
+}

runsc/cmd/gofer.go

@@ -67,6 +67,9 @@ type Gofer struct {
 	specFD       int
 	mountsFD     int
 	syncUsernsFD int
+	// procMountSyncFD is a file descriptor that has to be closed when the
+	// procfs mount isn't needed anymore.
+	procMountSyncFD int
 
 	profileFDs    profile.FDArgs
 	stopProfiling func()
@@ -98,6 +101,7 @@ func (g *Gofer) SetFlags(f *flag.FlagSet) {
 	f.IntVar(&g.specFD, "spec-fd", -1, "required fd with the container spec")
 	f.IntVar(&g.mountsFD, "mounts-fd", -1, "mountsFD is the file descriptor to write list of mounts after they have been resolved (direct paths, no symlinks).")
 	f.IntVar(&g.syncUsernsFD, "sync-userns-fd", -1, "file descriptor used to synchronize rootless user namespace initialization.")
+	f.IntVar(&g.procMountSyncFD, "proc-mount-sync-fd", -1, "file descriptor that has to be written to when /proc isn't needed anymore and can be unmounted")
 
 	// Profiling flags.
 	g.profileFDs.SetFromFlags(f)
@@ -146,12 +150,29 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm
 		if err := setupRootFS(spec, conf); err != nil {
 			util.Fatalf("Error setting up root FS: %v", err)
 		}
+		if !conf.TestOnlyAllowRunAsCurrentUserWithoutChroot {
+			// /proc is umounted from a forked process, because the
+			// current one may re-execute itself without capabilities.
+			cmd, w := execProcUmounter()
+			defer cmd.Wait()
+			defer w.Close()
+			if g.procMountSyncFD != -1 {
+				panic("procMountSyncFD is set")
+			}
+			g.procMountSyncFD = int(w.Fd())
+
+			// Clear FD_CLOEXEC. This process may be re-executed. procMountSyncFD
+			// should remain open.
+			if _, _, errno := unix.RawSyscall(unix.SYS_FCNTL, w.Fd(), unix.F_SETFD, 0); errno != 0 {
+				util.Fatalf("error clearing CLOEXEC: %v", errno)
+			}
+		}
 	}
 	if g.applyCaps {
 		// Disable caps when calling myself again.
 		// Note: minimal argument handling for the default case to keep it simple.
 		args := os.Args
-		args = append(args, "--apply-caps=false", "--setup-root=false", "--sync-userns-fd=-1")
+		args = append(args, "--apply-caps=false", "--setup-root=false", "--sync-userns-fd=-1", fmt.Sprintf("--proc-mount-sync-fd=%d", g.procMountSyncFD))
 		util.Fatalf("setCapsAndCallSelf(%v, %v): %v", args, goferCaps, setCapsAndCallSelf(args, goferCaps))
 		panic("unreachable")
 	}
@@ -214,6 +235,10 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcomm
 	if err := fsgofer.OpenProcSelfFD(); err != nil {
 		util.Fatalf("failed to open /proc/self/fd: %v", err)
 	}
+	if g.procMountSyncFD != -1 {
+		// procfs isn't needed anymore.
+		umountProc(g.procMountSyncFD)
+	}
 
 	if err := unix.Chroot(root); err != nil {
 		util.Fatalf("failed to chroot to %q: %v", root, err)

runsc/cmd/umount_unsafe.go

@@ -27,7 +27,7 @@ import (
 	"gvisor.dev/gvisor/runsc/flag"
 )
 
-// Umount implements subcommands.Command for the "kill" command.
+// Umount implements subcommands.Command for the "umount" command.
 type Umount struct {
 	syncFD int
 }
@@ -39,7 +39,7 @@ func (*Umount) Name() string {
 
 // Synopsis implements subcommands.Command.Synopsis.
 func (*Umount) Synopsis() string {
-	return "umount the specified directory when one byte is read from synd-fd"
+	return "umount the specified directory lazily when one byte is read from synd-fd"
 }
 
 // Usage implements subcommands.Command.Usage.