From eb6b3ac00b805f03a117f9c1e8c77d693ee05cea Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Fri, 18 Aug 2023 15:32:11 -0700 Subject: [PATCH] vfs: MountNamespace.Root() has to return a top mount of / A few mounts can be mounted on top of `/`. PiperOrigin-RevId: 558264274 --- pkg/sentry/control/fs.go | 2 +- pkg/sentry/fsimpl/devtmpfs/devtmpfs.go | 3 +-- pkg/sentry/fsimpl/devtmpfs/devtmpfs_test.go | 3 +-- pkg/sentry/fsimpl/proc/tasks_test.go | 3 +-- pkg/sentry/fsimpl/testutil/testutil.go | 3 +-- pkg/sentry/fsimpl/tmpfs/benchmark_test.go | 6 ++---- pkg/sentry/fsimpl/tmpfs/pipe_test.go | 3 +-- pkg/sentry/fsimpl/tmpfs/tmpfs_test.go | 3 +-- pkg/sentry/fsimpl/user/path.go | 3 +-- pkg/sentry/fsimpl/user/user.go | 3 +-- pkg/sentry/fsimpl/user/user_test.go | 3 +-- pkg/sentry/kernel/kernel.go | 9 +++------ pkg/sentry/kernel/task.go | 3 ++- pkg/sentry/kernel/task_clone.go | 3 +-- pkg/sentry/syscalls/linux/points.go | 3 ++- pkg/sentry/vfs/namespace.go | 19 ++++++++++++++++--- runsc/boot/loader.go | 5 ++++- runsc/boot/loader_test.go | 3 +-- runsc/boot/procfs/dump.go | 3 ++- runsc/boot/vfs.go | 18 ++++++------------ 20 files changed, 49 insertions(+), 52 deletions(-) diff --git a/pkg/sentry/control/fs.go b/pkg/sentry/control/fs.go index 0a30606a4..1db69dd61 100644 --- a/pkg/sentry/control/fs.go +++ b/pkg/sentry/control/fs.go @@ -76,7 +76,7 @@ func cat(k *kernel.Kernel, path string, output *os.File) error { ctx := k.SupervisorContext() creds := auth.NewRootCredentials(k.RootUserNamespace()) mns := k.GlobalInit().Leader().MountNamespace() - root := mns.Root() + root := mns.Root(ctx) defer root.DecRef(ctx) fd, err := k.VFS().OpenAt(ctx, creds, &vfs.PathOperation{ diff --git a/pkg/sentry/fsimpl/devtmpfs/devtmpfs.go b/pkg/sentry/fsimpl/devtmpfs/devtmpfs.go index 33c975dc1..45f58861a 100644 --- a/pkg/sentry/fsimpl/devtmpfs/devtmpfs.go +++ b/pkg/sentry/fsimpl/devtmpfs/devtmpfs.go @@ -96,8 +96,7 @@ func NewAccessor(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth return nil, err } // Pass a reference on root to the Accessor. - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) return &Accessor{ vfsObj: vfsObj, mntns: mntns, diff --git a/pkg/sentry/fsimpl/devtmpfs/devtmpfs_test.go b/pkg/sentry/fsimpl/devtmpfs/devtmpfs_test.go index 7d80db7fd..9fe4c56c9 100644 --- a/pkg/sentry/fsimpl/devtmpfs/devtmpfs_test.go +++ b/pkg/sentry/fsimpl/devtmpfs/devtmpfs_test.go @@ -52,8 +52,7 @@ func setupDevtmpfs(t *testing.T) (context.Context, *auth.Credentials, *vfs.Virtu if err != nil { t.Fatalf("failed to create tmpfs root mount: %v", err) } - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) devpop := vfs.PathOperation{ Root: root, Start: root, diff --git a/pkg/sentry/fsimpl/proc/tasks_test.go b/pkg/sentry/fsimpl/proc/tasks_test.go index ae9fa7c3c..4b671a9a9 100644 --- a/pkg/sentry/fsimpl/proc/tasks_test.go +++ b/pkg/sentry/fsimpl/proc/tasks_test.go @@ -114,8 +114,7 @@ func setup(t *testing.T) *testutil.System { if err != nil { t.Fatalf("NewMountNamespace(): %v", err) } - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) defer root.DecRef(ctx) pop := &vfs.PathOperation{ Root: root, diff --git a/pkg/sentry/fsimpl/testutil/testutil.go b/pkg/sentry/fsimpl/testutil/testutil.go index 59e6f9c92..bef96ef61 100644 --- a/pkg/sentry/fsimpl/testutil/testutil.go +++ b/pkg/sentry/fsimpl/testutil/testutil.go @@ -51,8 +51,7 @@ type System struct { // Precondition: Caller must hold a reference on mns, whose ownership // is transferred to the new System. func NewSystem(ctx context.Context, t *testing.T, v *vfs.VirtualFilesystem, mns *vfs.MountNamespace) *System { - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) s := &System{ t: t, Ctx: ctx, diff --git a/pkg/sentry/fsimpl/tmpfs/benchmark_test.go b/pkg/sentry/fsimpl/tmpfs/benchmark_test.go index 33389d489..b2c4578c0 100644 --- a/pkg/sentry/fsimpl/tmpfs/benchmark_test.go +++ b/pkg/sentry/fsimpl/tmpfs/benchmark_test.go @@ -72,8 +72,7 @@ func BenchmarkTmpfsStat(b *testing.B) { filePathBuilder.WriteByte('/') // Create nested directories with given depth. - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) defer root.DecRef(ctx) vd := root vd.IncRef() @@ -164,8 +163,7 @@ func BenchmarkTmpfsMountStat(b *testing.B) { filePathBuilder.WriteByte('/') // Create the mount point. - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) defer root.DecRef(ctx) pop := vfs.PathOperation{ Root: root, diff --git a/pkg/sentry/fsimpl/tmpfs/pipe_test.go b/pkg/sentry/fsimpl/tmpfs/pipe_test.go index 617e58172..6f1516694 100644 --- a/pkg/sentry/fsimpl/tmpfs/pipe_test.go +++ b/pkg/sentry/fsimpl/tmpfs/pipe_test.go @@ -164,8 +164,7 @@ func setup(t *testing.T) (context.Context, *auth.Credentials, *vfs.VirtualFilesy } // Create the pipe. - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) pop := vfs.PathOperation{ Root: root, Start: root, diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go b/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go index eaf08747b..be3ffc8af 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs_test.go @@ -46,8 +46,7 @@ func newTmpfsRoot(ctx context.Context) (*vfs.VirtualFilesystem, vfs.VirtualDentr if err != nil { return nil, vfs.VirtualDentry{}, nil, fmt.Errorf("failed to create tmpfs root mount: %v", err) } - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) return vfsObj, root, func() { root.DecRef(ctx) mntns.DecRef(ctx) diff --git a/pkg/sentry/fsimpl/user/path.go b/pkg/sentry/fsimpl/user/path.go index 0a240ee64..aa4f8df3c 100644 --- a/pkg/sentry/fsimpl/user/path.go +++ b/pkg/sentry/fsimpl/user/path.go @@ -73,8 +73,7 @@ func ResolveExecutablePath(ctx context.Context, args *kernel.CreateProcessArgs) } func resolve(ctx context.Context, creds *auth.Credentials, mns *vfs.MountNamespace, paths []string, name string) (string, error) { - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) for _, p := range paths { if !path.IsAbs(p) { diff --git a/pkg/sentry/fsimpl/user/user.go b/pkg/sentry/fsimpl/user/user.go index 372bc581d..2ada6b854 100644 --- a/pkg/sentry/fsimpl/user/user.go +++ b/pkg/sentry/fsimpl/user/user.go @@ -44,8 +44,7 @@ func (r *fileReader) Read(buf []byte) (int, error) { func getExecUserHome(ctx context.Context, mns *vfs.MountNamespace, uid auth.KUID) (string, error) { const defaultHome = "/" - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) creds := auth.CredentialsFromContext(ctx) diff --git a/pkg/sentry/fsimpl/user/user_test.go b/pkg/sentry/fsimpl/user/user_test.go index 2dccbf1a9..869929ae7 100644 --- a/pkg/sentry/fsimpl/user/user_test.go +++ b/pkg/sentry/fsimpl/user/user_test.go @@ -125,8 +125,7 @@ func TestGetExecUserHome(t *testing.T) { t.Fatalf("failed to create tmpfs root mount: %v", err) } defer mns.DecRef(ctx) - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) if err := createEtcPasswd(ctx, &vfsObj, creds, root, tc.passwdContents, tc.passwdMode); err != nil { diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index 34a8efbf1..c7fc5ad94 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -783,8 +783,7 @@ func (ctx *createProcessContext) Value(key any) any { if ctx.args.MountNamespace == nil { return nil } - root := ctx.args.MountNamespace.Root() - root.IncRef() + root := ctx.args.MountNamespace.Root(ctx) return root case vfs.CtxMountNamespace: if ctx.kernel.globalInit == nil { @@ -858,8 +857,7 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID, mntns.IncRef() } // Get the root directory from the MountNamespace. - root := mntns.Root() - root.IncRef() + root := mntns.Root(ctx) defer root.DecRef(ctx) // Grab the working directory. @@ -1567,8 +1565,7 @@ func (ctx *supervisorContext) Value(key any) any { if ctx.Kernel.globalInit == nil { return vfs.VirtualDentry{} } - root := ctx.Kernel.GlobalInit().Leader().MountNamespace().Root() - root.IncRef() + root := ctx.Kernel.GlobalInit().Leader().MountNamespace().Root(ctx) return root case vfs.CtxMountNamespace: if ctx.Kernel.globalInit == nil { diff --git a/pkg/sentry/kernel/task.go b/pkg/sentry/kernel/task.go index 93284958f..5fd3644aa 100644 --- a/pkg/sentry/kernel/task.go +++ b/pkg/sentry/kernel/task.go @@ -709,7 +709,8 @@ func (t *Task) SyscallRestartBlock() SyscallRestartBlock { // Preconditions: The caller must be running on the task goroutine, or t.mu // must be locked. func (t *Task) IsChrooted() bool { - realRoot := t.mountNamespace.Root() + realRoot := t.mountNamespace.Root(t) + defer realRoot.DecRef(t) root := t.fsContext.RootDirectory() defer root.DecRef(t) return root != realRoot diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go index 5885cfeb9..cbbde2c08 100644 --- a/pkg/sentry/kernel/task_clone.go +++ b/pkg/sentry/kernel/task_clone.go @@ -486,8 +486,7 @@ func (t *Task) Setns(fd *vfs.FileDescription, flags int32) error { fsContext := oldFSContext.Fork() fsContext.root.DecRef(t) fsContext.cwd.DecRef(t) - vd := ns.Root() - vd.IncRef() + vd := ns.Root(t) fsContext.root = vd vd.IncRef() fsContext.cwd = vd diff --git a/pkg/sentry/syscalls/linux/points.go b/pkg/sentry/syscalls/linux/points.go index 12e646c7d..06c2283d6 100644 --- a/pkg/sentry/syscalls/linux/points.go +++ b/pkg/sentry/syscalls/linux/points.go @@ -51,7 +51,8 @@ func getFilePath(t *kernel.Task, fd int32) string { } defer file.DecRef(t) - root := t.MountNamespace().Root() + root := t.MountNamespace().Root(t) + defer root.DecRef(t) path, err := t.Kernel().VFS().PathnameWithDeleted(t, root, file.VirtualDentry()) if err != nil { return fmt.Sprintf("[err: %v]", err) diff --git a/pkg/sentry/vfs/namespace.go b/pkg/sentry/vfs/namespace.go index d5488143a..f9edd723d 100644 --- a/pkg/sentry/vfs/namespace.go +++ b/pkg/sentry/vfs/namespace.go @@ -250,12 +250,25 @@ func (mntns *MountNamespace) TryIncRef() bool { return mntns.Refs.TryIncRef() } -// Root returns mntns' root. It does not take a reference on the returned -// Dentry. -func (mntns *MountNamespace) Root() VirtualDentry { +// Root returns mntns' root. If the root is over-mounted, it returns the top +// mount. +func (mntns *MountNamespace) Root(ctx context.Context) VirtualDentry { + vfs := mntns.root.fs.VirtualFilesystem() vd := VirtualDentry{ mount: mntns.root, dentry: mntns.root.root, } + vd.IncRef() + if !vd.dentry.isMounted() { + return vd + } + m := vfs.getMountAt(ctx, vd.mount, vd.dentry) + if m == nil { + return vd + } + vd.DecRef(ctx) + vd.mount = m + vd.dentry = m.root + vd.dentry.IncRef() return vd } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 4cd3b5273..7e6689cad 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -1097,7 +1097,10 @@ func (l *Loader) executeAsync(args *control.ExecArgs) (kernel.ThreadID, error) { } // Add the HOME environment variable if it is not already set. - ctx := vfs.WithRoot(l.k.SupervisorContext(), args.MountNamespace.Root()) + sctx := l.k.SupervisorContext() + root := args.MountNamespace.Root(sctx) + defer root.DecRef(sctx) + ctx := vfs.WithRoot(sctx, root) defer args.MountNamespace.DecRef(ctx) args.Envv, err = user.MaybeAddExecUserHome(ctx, args.MountNamespace, args.KUID, args.Envv) if err != nil { diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index 1fa050757..48d273872 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -487,8 +487,7 @@ func TestCreateMountNamespace(t *testing.T) { t.Fatalf("mountAll: %v", err) } - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) for _, p := range tc.expectedPaths { target := &vfs.PathOperation{ diff --git a/runsc/boot/procfs/dump.go b/runsc/boot/procfs/dump.go index a41f4a55f..05178b5ca 100644 --- a/runsc/boot/procfs/dump.go +++ b/runsc/boot/procfs/dump.go @@ -215,7 +215,8 @@ func getFDs(ctx context.Context, t *kernel.Task, pid kernel.ThreadID) []FDInfo { } func getRoot(t *kernel.Task, pid kernel.ThreadID) string { - realRoot := t.MountNamespace().Root() + realRoot := t.MountNamespace().Root(t) + defer realRoot.DecRef(t) root := t.FSContext().RootDirectory() defer root.DecRef(t) path, err := t.Kernel().VFS().PathnameWithDeleted(t, realRoot, root) diff --git a/runsc/boot/vfs.go b/runsc/boot/vfs.go index 5db421829..48e195113 100644 --- a/runsc/boot/vfs.go +++ b/runsc/boot/vfs.go @@ -199,8 +199,7 @@ func setupContainerVFS(ctx context.Context, info *containerInfo, mntr *container } procArgs.MountNamespace = mns - mnsRoot := mns.Root() - mnsRoot.IncRef() + mnsRoot := mns.Root(rootCtx) defer mnsRoot.DecRef(rootCtx) if err := createDeviceFiles(rootCtx, rootCreds, info, mntr.k.VFS(), mnsRoot); err != nil { @@ -436,8 +435,7 @@ func (c *containerMounter) mountAll(rootCtx context.Context, rootCreds *auth.Cre } rootProcArgs.MountNamespace = mns - root := mns.Root() - root.IncRef() + root := mns.Root(rootCtx) defer root.DecRef(rootCtx) if root.Mount().ReadOnly() { // Switch to ReadWrite while we setup submounts. @@ -756,8 +754,7 @@ func (c *containerMounter) mountSubmount(ctx context.Context, conf *config.Confi fsName = overlay.Name } - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) target := &vfs.PathOperation{ Root: root, @@ -886,8 +883,7 @@ func (c *containerMounter) mountTmp(ctx context.Context, conf *config.Config, cr } } - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) pop := vfs.PathOperation{ Root: root, @@ -994,8 +990,7 @@ func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config newMnt := c.k.VFS().NewDisconnectedMount(source.vfsMount.Filesystem(), source.vfsMount.Root(), opts) defer newMnt.DecRef(ctx) - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) target := &vfs.PathOperation{ Root: root, @@ -1015,8 +1010,7 @@ func (c *containerMounter) mountSharedSubmount(ctx context.Context, conf *config } func (c *containerMounter) makeMountPoint(ctx context.Context, creds *auth.Credentials, mns *vfs.MountNamespace, dest string) error { - root := mns.Root() - root.IncRef() + root := mns.Root(ctx) defer root.DecRef(ctx) target := &vfs.PathOperation{ Root: root,