From 75bfc5e0e9cc68c87c0a4195b73cc786db2a9920 Mon Sep 17 00:00:00 2001 From: Lucas Manning Date: Fri, 10 Dec 2021 17:08:00 -0800 Subject: [PATCH] Resolve to the last mount in the stack during umount. This is done in linux with the LOOKUP_MOUNTPOINT flag. We emulate this behavior in gVisor with getMountAt(). PiperOrigin-RevId: 415642095 --- pkg/sentry/vfs/mount.go | 19 +++++++++++++++---- test/syscalls/linux/mount.cc | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/pkg/sentry/vfs/mount.go b/pkg/sentry/vfs/mount.go index 9ab9a8fca..6ad0e0e3b 100644 --- a/pkg/sentry/vfs/mount.go +++ b/pkg/sentry/vfs/mount.go @@ -297,10 +297,21 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti if err != nil { return err } - defer vd.DecRef(ctx) - if vd.dentry != vd.mount.root { + defer func() { + vd.DecRef(ctx) + }() + // Linux passes the LOOKUP_MOUNPOINT flag to user_path_at in ksys_umount to resolve to the + // toppmost mount in the stack located at the specified path. vfs.GetMountAt() imitiates this + // behavior. See fs/namei.c:user_path_at(...) and fs/namespace.c:ksys_umount(...). + if vd.dentry.isMounted() { + if realmnt := vfs.getMountAt(ctx, vd.mount, vd.dentry); realmnt != nil { + vd.mount.DecRef(ctx) + vd.mount = realmnt + } + } else if vd.dentry != vd.mount.root { return linuxerr.EINVAL } + vfs.mountMu.Lock() if mntns := MountNamespaceFromContext(ctx); mntns != nil { defer mntns.DecRef(ctx) @@ -346,8 +357,8 @@ func (vfs *VirtualFilesystem) UmountAt(ctx context.Context, creds *auth.Credenti for _, vd := range vdsToDecRef { vd.DecRef(ctx) } - for _, mnt := range mountsToDecRef { - mnt.DecRef(ctx) + for _, m := range mountsToDecRef { + m.DecRef(ctx) } return nil } diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index e2a41d172..4289b3cf2 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -220,6 +220,28 @@ TEST(MountTest, UmountDetach) { OpenAt(mounted_dir.get(), "..", O_DIRECTORY | O_RDONLY)); } +TEST(MountTest, UmountMountsStackedOnDot) { + SKIP_IF(IsRunningWithVFS1()); + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + // Verify that unmounting at "." properly unmounts the mount at the top of + // mount stack. + auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + TEST_CHECK_SUCCESS(chdir(dir.path().c_str())); + const struct stat before = ASSERT_NO_ERRNO_AND_VALUE(Stat(".")); + + TEST_CHECK_SUCCESS(mount("", dir.path().c_str(), "tmpfs", 0, "mode=0700")); + TEST_CHECK_SUCCESS(mount("", dir.path().c_str(), "tmpfs", 0, "mode=0700")); + + // Unmount the second mount at "." + TEST_CHECK_SUCCESS(umount2(".", MNT_DETACH)); + + // Unmount the first mount at "."; this will fail if umount does not resolve + // "." to the topmost mount. + TEST_CHECK_SUCCESS(umount2(".", MNT_DETACH)); + const struct stat after2 = ASSERT_NO_ERRNO_AND_VALUE(Stat(".")); + EXPECT_TRUE(before.st_dev == after2.st_dev && before.st_ino == after2.st_ino); +} + TEST(MountTest, ActiveSubmountBusy) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN)));