diff options
author | Ayush Ranjan <ayushranjan@google.com> | 2021-11-01 16:48:16 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-11-01 16:51:12 -0700 |
commit | 58017e655399384afed2cedea0e269cb1ad2dd7e (patch) | |
tree | 7612b782dcde958ac12f8844ebff893e01bd3b1b | |
parent | 9776edb3fa9e67a28182aedc9342ef9c1b556d7c (diff) |
Handle UMOUNT_NOFOLLOW in VFS2 umount(2).
Reported-by: syzbot+f9ecb181a4b3abdde9b9@syzkaller.appspotmail.com
Reported-by: syzbot+8c5cb9d7a044a91a513b@syzkaller.appspotmail.com
PiperOrigin-RevId: 406951359
-rw-r--r-- | pkg/sentry/syscalls/linux/vfs2/mount.go | 4 | ||||
-rw-r--r-- | test/syscalls/linux/mount.cc | 34 |
2 files changed, 36 insertions, 2 deletions
diff --git a/pkg/sentry/syscalls/linux/vfs2/mount.go b/pkg/sentry/syscalls/linux/vfs2/mount.go index 4d73d46ef..fd0ab4c76 100644 --- a/pkg/sentry/syscalls/linux/vfs2/mount.go +++ b/pkg/sentry/syscalls/linux/vfs2/mount.go @@ -136,14 +136,14 @@ func Umount2(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysca if err != nil { return 0, nil, err } - tpop, err := getTaskPathOperation(t, linux.AT_FDCWD, path, disallowEmptyPath, nofollowFinalSymlink) + tpop, err := getTaskPathOperation(t, linux.AT_FDCWD, path, disallowEmptyPath, shouldFollowFinalSymlink(flags&linux.UMOUNT_NOFOLLOW == 0)) if err != nil { return 0, nil, err } defer tpop.Release(t) opts := vfs.UmountOptions{ - Flags: uint32(flags), + Flags: uint32(flags &^ linux.UMOUNT_NOFOLLOW), } return 0, nil, t.Kernel().VFS().UmountAt(t, creds, &tpop.pop, &opts) diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 3c7311782..e2a41d172 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -115,6 +115,40 @@ TEST(MountTest, OpenFileBusy) { EXPECT_THAT(umount(dir.path().c_str()), SyscallFailsWithErrno(EBUSY)); } +TEST(MountTest, UmountNoFollow) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + + auto const mountPoint = NewTempAbsPathInDir(dir.path()); + ASSERT_THAT(mkdir(mountPoint.c_str(), 0777), SyscallSucceeds()); + + // Create a symlink in dir which will point to the actual mountpoint. + const std::string symlinkInDir = NewTempAbsPathInDir(dir.path()); + EXPECT_THAT(symlink(mountPoint.c_str(), symlinkInDir.c_str()), + SyscallSucceeds()); + + // Create a symlink to the dir. + const std::string symlinkToDir = NewTempAbsPath(); + EXPECT_THAT(symlink(dir.path().c_str(), symlinkToDir.c_str()), + SyscallSucceeds()); + + // Should fail with ELOOP when UMOUNT_NOFOLLOW is specified and the last + // component is a symlink. + auto mount = ASSERT_NO_ERRNO_AND_VALUE( + Mount("", mountPoint, "tmpfs", 0, "mode=0700", 0)); + EXPECT_THAT(umount2(symlinkInDir.c_str(), UMOUNT_NOFOLLOW), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(unlink(symlinkInDir.c_str()), SyscallSucceeds()); + + // UMOUNT_NOFOLLOW should only apply to the last path component. A symlink in + // non-last path component should be just fine. + EXPECT_THAT(umount2(JoinPath(symlinkToDir, Basename(mountPoint)).c_str(), + UMOUNT_NOFOLLOW), + SyscallSucceeds()); + mount.Release(); +} + TEST(MountTest, UmountDetach) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); |