diff --git a/pkg/sentry/vfs/anonfs.go b/pkg/sentry/vfs/anonfs.go index 255d3992e..f34770c77 100644 --- a/pkg/sentry/vfs/anonfs.go +++ b/pkg/sentry/vfs/anonfs.go @@ -100,7 +100,7 @@ func (fs *anonFilesystem) Sync(ctx context.Context) error { // AccessAt implements vfs.Filesystem.Impl.AccessAt. func (fs *anonFilesystem) AccessAt(ctx context.Context, rp *ResolvingPath, creds *auth.Credentials, ats AccessTypes) error { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linuxerr.ENOTDIR } return GenericCheckPermissions(creds, ats, anonFileMode, anonFileUID, anonFileGID) @@ -108,7 +108,7 @@ func (fs *anonFilesystem) AccessAt(ctx context.Context, rp *ResolvingPath, creds // GetDentryAt implements FilesystemImpl.GetDentryAt. func (fs *anonFilesystem) GetDentryAt(ctx context.Context, rp *ResolvingPath, opts GetDentryOptions) (*Dentry, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return nil, linuxerr.ENOTDIR } if opts.CheckSearchable { @@ -153,7 +153,7 @@ func (fs *anonFilesystem) MknodAt(ctx context.Context, rp *ResolvingPath, opts M // OpenAt implements FilesystemImpl.OpenAt. func (fs *anonFilesystem) OpenAt(ctx context.Context, rp *ResolvingPath, opts OpenOptions) (*FileDescription, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return nil, linuxerr.ENOTDIR } return nil, linuxerr.ENODEV @@ -161,7 +161,7 @@ func (fs *anonFilesystem) OpenAt(ctx context.Context, rp *ResolvingPath, opts Op // ReadlinkAt implements FilesystemImpl.ReadlinkAt. func (fs *anonFilesystem) ReadlinkAt(ctx context.Context, rp *ResolvingPath) (string, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return "", linuxerr.ENOTDIR } return "", linuxerr.EINVAL @@ -185,7 +185,7 @@ func (fs *anonFilesystem) RmdirAt(ctx context.Context, rp *ResolvingPath) error // SetStatAt implements FilesystemImpl.SetStatAt. func (fs *anonFilesystem) SetStatAt(ctx context.Context, rp *ResolvingPath, opts SetStatOptions) error { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linuxerr.ENOTDIR } // Linux actually permits anon_inode_inode's metadata to be set, which is @@ -196,7 +196,7 @@ func (fs *anonFilesystem) SetStatAt(ctx context.Context, rp *ResolvingPath, opts // StatAt implements FilesystemImpl.StatAt. func (fs *anonFilesystem) StatAt(ctx context.Context, rp *ResolvingPath, opts StatOptions) (linux.Statx, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linux.Statx{}, linuxerr.ENOTDIR } // See fs/anon_inodes.c:anon_inode_init() => fs/libfs.c:alloc_anon_inode(). @@ -217,7 +217,7 @@ func (fs *anonFilesystem) StatAt(ctx context.Context, rp *ResolvingPath, opts St // StatFSAt implements FilesystemImpl.StatFSAt. func (fs *anonFilesystem) StatFSAt(ctx context.Context, rp *ResolvingPath) (linux.Statfs, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linux.Statfs{}, linuxerr.ENOTDIR } return linux.Statfs{ @@ -255,7 +255,7 @@ func (fs *anonFilesystem) BoundEndpointAt(ctx context.Context, rp *ResolvingPath // ListXattrAt implements FilesystemImpl.ListXattrAt. func (fs *anonFilesystem) ListXattrAt(ctx context.Context, rp *ResolvingPath, size uint64) ([]string, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return nil, linuxerr.ENOTDIR } return nil, nil @@ -263,7 +263,7 @@ func (fs *anonFilesystem) ListXattrAt(ctx context.Context, rp *ResolvingPath, si // GetXattrAt implements FilesystemImpl.GetXattrAt. func (fs *anonFilesystem) GetXattrAt(ctx context.Context, rp *ResolvingPath, opts GetXattrOptions) (string, error) { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return "", linuxerr.ENOTDIR } return "", linuxerr.ENOTSUP @@ -271,7 +271,7 @@ func (fs *anonFilesystem) GetXattrAt(ctx context.Context, rp *ResolvingPath, opt // SetXattrAt implements FilesystemImpl.SetXattrAt. func (fs *anonFilesystem) SetXattrAt(ctx context.Context, rp *ResolvingPath, opts SetXattrOptions) error { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linuxerr.ENOTDIR } return linuxerr.EPERM @@ -279,7 +279,7 @@ func (fs *anonFilesystem) SetXattrAt(ctx context.Context, rp *ResolvingPath, opt // RemoveXattrAt implements FilesystemImpl.RemoveXattrAt. func (fs *anonFilesystem) RemoveXattrAt(ctx context.Context, rp *ResolvingPath, name string) error { - if !rp.Done() { + if !rp.Done() || rp.MustBeDir() { return linuxerr.ENOTDIR } return linuxerr.EPERM diff --git a/pkg/sentry/vfs/resolving_path.go b/pkg/sentry/vfs/resolving_path.go index 028801956..128c66fcf 100644 --- a/pkg/sentry/vfs/resolving_path.go +++ b/pkg/sentry/vfs/resolving_path.go @@ -377,7 +377,7 @@ func (rp *ResolvingPath) relpathPrepend(path fspath.Path) { // HandleJump is called when the current path component is a "magic" link to // the given VirtualDentry, like /proc/[pid]/fd/[fd]. If the calling Filesystem -// method should continue path traversal, HandleMagicSymlink updates the path +// method should continue path traversal, HandleJump updates the path // component stream to reflect the magic link target and returns nil. Otherwise // it returns a non-nil error. // diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 9919c710d..f496864d5 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1777,6 +1777,7 @@ cc_binary( "@com_google_absl//absl/synchronization", "@com_google_absl//absl/time", gtest, + "//test/util:eventfd_util", "//test/util:memory_util", "//test/util:multiprocess_util", "//test/util:posix_error", diff --git a/test/syscalls/linux/proc.cc b/test/syscalls/linux/proc.cc index 7a8190dda..52921334e 100644 --- a/test/syscalls/linux/proc.cc +++ b/test/syscalls/linux/proc.cc @@ -63,6 +63,7 @@ #include "absl/time/time.h" #include "test/util/capability_util.h" #include "test/util/cleanup.h" +#include "test/util/eventfd_util.h" #include "test/util/file_descriptor.h" #include "test/util/fs_util.h" #include "test/util/memory_util.h" @@ -1662,32 +1663,30 @@ TEST(ProcPidStatusTest, HasBasicFields) { Pair("PPid", absl::StrCat(getppid())), })); - uid_t ruid, euid, suid; - ASSERT_THAT(getresuid(&ruid, &euid, &suid), SyscallSucceeds()); - gid_t rgid, egid, sgid; - ASSERT_THAT(getresgid(&rgid, &egid, &sgid), SyscallSucceeds()); - std::vector supplementary_gids; - int ngids = getgroups(0, nullptr); - supplementary_gids.resize(ngids); - ASSERT_THAT(getgroups(ngids, supplementary_gids.data()), - SyscallSucceeds()); + uid_t ruid, euid, suid; + ASSERT_THAT(getresuid(&ruid, &euid, &suid), SyscallSucceeds()); + gid_t rgid, egid, sgid; + ASSERT_THAT(getresgid(&rgid, &egid, &sgid), SyscallSucceeds()); + std::vector supplementary_gids; + int ngids = getgroups(0, nullptr); + supplementary_gids.resize(ngids); + ASSERT_THAT(getgroups(ngids, supplementary_gids.data()), SyscallSucceeds()); - EXPECT_THAT( - status, - IsSupersetOf(std::vector< - ::testing::Matcher>>{ - // gVisor doesn't support fsuid/gid, and even if it did there is - // no getfsuid/getfsgid(). - Pair("Uid", StartsWith(absl::StrFormat("%d\t%d\t%d\t", ruid, euid, - suid))), - Pair("Gid", StartsWith(absl::StrFormat("%d\t%d\t%d\t", rgid, egid, - sgid))), - // ParseProcStatus strips leading whitespace for each value, - // so if the Groups line is empty then the trailing space is - // stripped. - Pair("Groups", - StartsWith(absl::StrJoin(supplementary_gids, " "))), - })); + EXPECT_THAT( + status, + IsSupersetOf(std::vector< + ::testing::Matcher>>{ + // gVisor doesn't support fsuid/gid, and even if it did there is + // no getfsuid/getfsgid(). + Pair("Uid", + StartsWith(absl::StrFormat("%d\t%d\t%d\t", ruid, euid, suid))), + Pair("Gid", + StartsWith(absl::StrFormat("%d\t%d\t%d\t", rgid, egid, sgid))), + // ParseProcStatus strips leading whitespace for each value, + // so if the Groups line is empty then the trailing space is + // stripped. + Pair("Groups", StartsWith(absl::StrJoin(supplementary_gids, " "))), + })); }); } @@ -2734,6 +2733,16 @@ TEST(Proc, ResolveSymlinkToProc) { EXPECT_EQ(target, JoinPath("/proc/", absl::StrCat(getpid()), "/cmdline")); } +// NOTE(b/236035339): Tests that opening /proc/[pid]/fd/[eventFDNum] with +// O_DIRECTORY leads to ENOTDIR. +TEST(Proc, RegressionTestB236035339) { + FileDescriptor efd = + ASSERT_NO_ERRNO_AND_VALUE(NewEventFD(0, EFD_NONBLOCK | EFD_CLOEXEC)); + const auto path = JoinPath("/proc/self/fd/", absl::StrCat(efd.get())); + EXPECT_THAT(open(path.c_str(), O_RDONLY | O_CLOEXEC | O_DIRECTORY), + SyscallFailsWithErrno(ENOTDIR)); +} + } // namespace } // namespace testing } // namespace gvisor