diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index fafd746c372..f9046402d44 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -1441,6 +1441,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa // users. return linuxerr.EINVAL } + exchange := opts.Flags&linux.RENAME_EXCHANGE != 0 newName := rp.Component() if newName == "." || newName == ".." { @@ -1501,7 +1502,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { - if opts.MustBeDir || rp.MustBeDir() { + if !exchange && (opts.MustBeDir || rp.MustBeDir()) { return linuxerr.ENOTDIR } } @@ -1529,7 +1530,26 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } replacedVFSD = &replaced.vfsd - if replaced.isDir() { + if exchange { + // The exchanged files may differ in type, and a directory being + // exchanged may be non-empty; but exchanging a file with an + // ancestor directory would disconnect the latter from the tree. + if genericIsAncestorDentry(fs, replaced, renamed) { + return linuxerr.EINVAL + } + if rp.MustBeDir() && !replaced.isDir() { + return linuxerr.ENOTDIR + } + if opts.MustBeDir && !renamed.isDir() { + return linuxerr.ENOTDIR + } + if oldParent != newParent && replaced.isDir() { + // Writability is needed to change replaced's "..". + if err := replaced.checkPermissions(creds, vfs.MayWrite); err != nil { + return err + } + } + } else if replaced.isDir() { if !renamed.isDir() { return linuxerr.EISDIR } @@ -1542,7 +1562,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { // replaced == nil - if opts.Flags&linux.RENAME_EXCHANGE != 0 { + if exchange { // RENAME_EXCHANGE requires that the target file exist. return linuxerr.ENOENT } @@ -1564,7 +1584,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa vfsObj.AbortRenameDentry(&handle, &renamed.vfsd, replacedVFSD) return err } - } else if replaced != nil && !replaced.inode.isSynthetic() && opts.Flags&linux.RENAME_EXCHANGE == 0 { + } else if replaced != nil && !replaced.inode.isSynthetic() && !exchange { // We are replacing an existing real file with a synthetic one, so we // need to unlink the former. flags := uint32(0) @@ -1586,7 +1606,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } vfsObj.RenameBegin(&handle) - if opts.Flags&linux.RENAME_EXCHANGE != 0 { + if exchange { if oldParent != newParent { switch { case replaced.inode.isSynthetic() && !renamed.inode.isSynthetic(): @@ -1616,66 +1636,85 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa oldParent.clearDirentsLocked() oldParent.touchCMtime() } - if oldParent != newParent && newParent.inode.cachedMetadataAuthoritative() { - newParent.clearDirentsLocked() - newParent.touchCMtime() + if oldParent != newParent { + if newParent.inode.cachedMetadataAuthoritative() { + newParent.clearDirentsLocked() + newParent.touchCMtime() + } + // If exactly one of the exchanged files is a directory, its ".." + // entry moves from one parent directory to the other. + if renamed.isDir() && !replaced.isDir() { + if oldParent.inode.cachedMetadataAuthoritative() { + oldParent.decLinks() + } + if newParent.inode.cachedMetadataAuthoritative() { + newParent.incLinks() + } + } else if !renamed.isDir() && replaced.isDir() { + if newParent.inode.cachedMetadataAuthoritative() { + newParent.decLinks() + } + if oldParent.inode.cachedMetadataAuthoritative() { + oldParent.incLinks() + } + } } // Sends notifications for both the renamed and replaced dentries. vfs.InotifyRename(ctx, &renamed.inode.watches, &oldParent.inode.watches, &newParent.inode.watches, oldName, newName, renamed.isDir()) vfs.InotifyRename(ctx, &replaced.inode.watches, &newParent.inode.watches, &oldParent.inode.watches, newName, oldName, replaced.isDir()) - } else { - if replaced != nil { - replaced.setDeleted() - // If an extra reference is held on replaced as described by the - // comment for dentry.refs, drop that reference now. We can't race with - // fs.unlinkAt() or invalidation since fs.renameMu has been locked for - // writing since before we obtained replaced. - if replaced.inode.isSynthetic() { - newParent.syntheticChildren-- - replaced.decRefNoCaching() - } else if replaced.inode.endpoint != nil { - replaced.decRefNoCaching() - } - ds = appendDentry(ds, replaced) - // Remove the replaced entry from its parent's cache. - delete(newParent.children, newName) - } - oldParent.cacheNegativeLookupLocked(oldName) // +checklocksforce: oldParent.childrenMu is held if oldParent != newParent. - if renamed.inode.isSynthetic() { - oldParent.syntheticChildren-- - newParent.syntheticChildren++ - } - // We have d.opMu for writing, so no need to check for existence of a - // child with the given name. We could not have raced. - newParent.cacheNewChildLocked(renamed, newName) - oldParent.decRefNoCaching() - if oldParent != newParent { - ds = appendDentry(ds, newParent) - ds = appendDentry(ds, oldParent) - } - toDecRef = vfsObj.CommitRenameReplaceDentry(ctx, &handle, &renamed.vfsd, replacedVFSD) + return nil + } + if replaced != nil { + replaced.setDeleted() + // If an extra reference is held on replaced as described by the + // comment for dentry.refs, drop that reference now. We can't race with + // fs.unlinkAt() or invalidation since fs.renameMu has been locked for + // writing since before we obtained replaced. + if replaced.inode.isSynthetic() { + newParent.syntheticChildren-- + replaced.decRefNoCaching() + } else if replaced.inode.endpoint != nil { + replaced.decRefNoCaching() + } + ds = appendDentry(ds, replaced) + // Remove the replaced entry from its parent's cache. + delete(newParent.children, newName) + } + oldParent.cacheNegativeLookupLocked(oldName) // +checklocksforce: oldParent.childrenMu is held if oldParent != newParent. + if renamed.inode.isSynthetic() { + oldParent.syntheticChildren-- + newParent.syntheticChildren++ + } + // We have d.opMu for writing, so no need to check for existence of a + // child with the given name. We could not have raced. + newParent.cacheNewChildLocked(renamed, newName) + oldParent.decRefNoCaching() + if oldParent != newParent { + ds = appendDentry(ds, newParent) + ds = appendDentry(ds, oldParent) + } + toDecRef = vfsObj.CommitRenameReplaceDentry(ctx, &handle, &renamed.vfsd, replacedVFSD) - // Update metadata. - if renamed.inode.cachedMetadataAuthoritative() { - renamed.touchCtime() - } - if oldParent.inode.cachedMetadataAuthoritative() { - oldParent.clearDirentsLocked() - oldParent.touchCMtime() - if renamed.isDir() { - oldParent.decLinks() - } + // Update metadata. + if renamed.inode.cachedMetadataAuthoritative() { + renamed.touchCtime() + } + if oldParent.inode.cachedMetadataAuthoritative() { + oldParent.clearDirentsLocked() + oldParent.touchCMtime() + if renamed.isDir() { + oldParent.decLinks() } - if newParent.inode.cachedMetadataAuthoritative() { - newParent.clearDirentsLocked() - newParent.touchCMtime() - if renamed.isDir() && (replaced == nil || !replaced.isDir()) { - // Increase the link count if we did not replace another directory. - newParent.incLinks() - } + } + if newParent.inode.cachedMetadataAuthoritative() { + newParent.clearDirentsLocked() + newParent.touchCMtime() + if renamed.isDir() && (replaced == nil || !replaced.isDir()) { + // Increase the link count if we did not replace another directory. + newParent.incLinks() } - vfs.InotifyRename(ctx, &renamed.inode.watches, &oldParent.inode.watches, &newParent.inode.watches, oldName, newName, renamed.isDir()) } + vfs.InotifyRename(ctx, &renamed.inode.watches, &oldParent.inode.watches, &newParent.inode.watches, oldName, newName, renamed.isDir()) return nil } diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index cf830400944..7fc91e3c7e7 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -1090,9 +1090,13 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } - if opts.Flags&^linux.RENAME_NOREPLACE != 0 { + if opts.Flags&^(linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE) != 0 { return linuxerr.EINVAL } + if opts.Flags&(linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE) == linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE { + return linuxerr.EINVAL + } + exchange := opts.Flags&linux.RENAME_EXCHANGE != 0 newName := rp.Component() if newName == "." || newName == ".." { @@ -1142,7 +1146,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { - if opts.MustBeDir || rp.MustBeDir() { + if !exchange && (opts.MustBeDir || rp.MustBeDir()) { return linuxerr.ENOTDIR } } @@ -1175,7 +1179,26 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } replacedVFSD = &replaced.vfsd - if replaced.isDir() { + if exchange { + // The exchanged files may differ in type, and a directory being + // exchanged may be non-empty; but exchanging a file with an + // ancestor directory would disconnect the latter from the tree. + if genericIsAncestorDentry(fs, replaced, renamed) { + return linuxerr.EINVAL + } + if rp.MustBeDir() && !replaced.isDir() { + return linuxerr.ENOTDIR + } + if opts.MustBeDir && !renamed.isDir() { + return linuxerr.ENOTDIR + } + if oldParent != newParent && replaced.isDir() { + // Writability is needed to change replaced's "..". + if err := replaced.checkPermissions(creds, vfs.MayWrite); err != nil { + return err + } + } + } else if replaced.isDir() { if !renamed.isDir() { return linuxerr.EISDIR } @@ -1193,6 +1216,9 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return linuxerr.ENOTDIR } } + } else if exchange { + // RENAME_EXCHANGE requires that the target file exist. + return linuxerr.ENOENT } if oldParent == newParent && oldName == newName { @@ -1216,10 +1242,24 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa if err := newParent.copyUpLocked(ctx); err != nil { return err } - // If replaced exists, it doesn't need to be copied-up, but we do need to - // serialize with copy-up. Holding renameMu for writing should be - // sufficient, but out of an abundance of caution... - if replaced != nil { + if exchange { + // replaced is also renamed on the upper layer, so it (and all of its + // descendants if it's a directory) must be copied-up too. + if err := replaced.copyUpLocked(ctx); err != nil { + return err + } + if replaced.isDir() { + replaced.dirMu.NestedLock(dirLockReplaced) + err := replaced.copyUpDescendantsLocked(ctx, &ds) + replaced.dirMu.NestedUnlock(dirLockReplaced) + if err != nil { + return err + } + } + } else if replaced != nil { + // replaced doesn't need to be copied-up, but we do need to serialize + // with copy-up. Holding renameMu for writing should be sufficient, but + // out of an abundance of caution... replaced.copyMu.RLock() defer replaced.copyMu.RUnlock() } @@ -1256,7 +1296,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } - if renamed.isDir() { + if !exchange && renamed.isDir() { if replacedLayer == lookupLayerUpper { // Remove whiteouts from the directory being replaced. needRecreateWhiteouts = true @@ -1304,6 +1344,45 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } + if exchange { + // Below this point, renamed is at newpop and replaced is at oldpop. + // Commit the exchange, update the overlay filesystem tree, and abandon + // attempts to recover from errors. + vfsObj.RenameBegin(&handle) + genericSetParentAndName(fs, renamed, newParent, newName) + genericSetParentAndName(fs, replaced, oldParent, oldName) + // References held by renamed and replaced on their parents are + // exchanged as well; the counts on each parent are unchanged. + oldParent.children[oldName] = replaced + newParent.children[newName] = renamed + oldParent.dirents = nil + newParent.dirents = nil + vfsObj.CommitRenameExchangeDentry(&handle, &renamed.vfsd, replacedVFSD) + + // An exchanged directory's contents can no longer be merged with + // lower layer directories at its new location. + if renamed.isDir() { + if err := vfsObj.SetXattrAt(ctx, fs.creds, &newpop, &vfs.SetXattrOptions{ + Name: fs.xattrOpaque, + Value: "y", + }); err != nil { + panic(fmt.Sprintf("unrecoverable overlayfs inconsistency: failed to make exchanged directory opaque: %v", err)) + } + } + if replaced.isDir() { + if err := vfsObj.SetXattrAt(ctx, fs.creds, &oldpop, &vfs.SetXattrOptions{ + Name: fs.xattrOpaque, + Value: "y", + }); err != nil { + panic(fmt.Sprintf("unrecoverable overlayfs inconsistency: failed to make exchanged directory opaque: %v", err)) + } + } + + vfs.InotifyRename(ctx, &renamed.watches, &oldParent.watches, &newParent.watches, oldName, newName, renamed.isDir()) + vfs.InotifyRename(ctx, &replaced.watches, &newParent.watches, &oldParent.watches, newName, oldName, replaced.isDir()) + return nil + } + // Below this point, the renamed dentry is now at newpop, and anything we // replaced is gone forever. Commit the rename, update the overlay // filesystem tree, and abandon attempts to recover from errors. diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index e57b929c024..1bb63a3dd41 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -562,10 +562,14 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } - if opts.Flags&^linux.RENAME_NOREPLACE != 0 { - // TODO(b/145974740): Support other renameat2 flags. + if opts.Flags&^(linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE) != 0 { + // TODO(b/145974740): Support RENAME_WHITEOUT. return linuxerr.EINVAL } + if opts.Flags&(linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE) == linux.RENAME_NOREPLACE|linux.RENAME_EXCHANGE { + return linuxerr.EINVAL + } + exchange := opts.Flags&linux.RENAME_EXCHANGE != 0 newName := rp.Component() if newName == "." || newName == ".." { @@ -611,7 +615,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { - if opts.MustBeDir || rp.MustBeDir() { + if !exchange && (opts.MustBeDir || rp.MustBeDir()) { return linuxerr.ENOTDIR } } @@ -627,8 +631,33 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa if err := newParentDir.mayDelete(rp.Credentials(), replaced); err != nil { return err } - replacedDir, ok := replaced.inode.impl.(*directory) - if ok { + if exchange { + // The exchanged files may differ in type, and a directory being + // exchanged may be non-empty; but exchanging a file with an + // ancestor directory would disconnect the latter from the tree. + if genericIsAncestorDentry(fs, replaced, renamed) { + return linuxerr.EINVAL + } + if rp.MustBeDir() && !replaced.inode.isDir() { + return linuxerr.ENOTDIR + } + if opts.MustBeDir && !renamed.inode.isDir() { + return linuxerr.ENOTDIR + } + if oldParentDir != newParentDir { + if replaced.inode.isDir() { + // Writability is needed to change replaced's "..". + if err := replaced.inode.checkPermissions(rp.Credentials(), vfs.MayWrite); err != nil { + return err + } + if !renamed.inode.isDir() && oldParentDir.inode.nlink.Load() == maxLinks { + return linuxerr.EMLINK + } + } else if renamed.inode.isDir() && newParentDir.inode.nlink.Load() == maxLinks { + return linuxerr.EMLINK + } + } + } else if replacedDir, ok := replaced.inode.impl.(*directory); ok { if !renamed.inode.isDir() { return linuxerr.EISDIR } @@ -644,6 +673,10 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa } } } else { + if exchange { + // RENAME_EXCHANGE requires that the target file exist. + return linuxerr.ENOENT + } if renamed.inode.isDir() && newParentDir.inode.nlink.Load() == maxLinks { return linuxerr.EMLINK } @@ -673,6 +706,36 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa return err } vfsObj.RenameBegin(&handle) + if exchange { + oldParentDir.removeChildLocked(renamed) + newParentDir.removeChildLocked(replaced) + newParentDir.insertChildLocked(renamed, newName) + oldParentDir.insertChildLocked(replaced, oldName) + vfsObj.CommitRenameExchangeDentry(&handle, &renamed.vfsd, replacedVFSD) + if oldParentDir != newParentDir { + // If exactly one of the exchanged files is a directory, its ".." + // entry (and the reference that it holds on its parent, see + // MkdirAt) moves from one parent directory to the other. + if renamed.inode.isDir() && !replaced.inode.isDir() { + oldParentDir.inode.decLinksLocked(ctx) + newParentDir.inode.incLinksLocked() + oldParentDir.inode.decRef(ctx) + newParentDir.inode.incRef() + } else if !renamed.inode.isDir() && replaced.inode.isDir() { + newParentDir.inode.decLinksLocked(ctx) + oldParentDir.inode.incLinksLocked() + newParentDir.inode.decRef(ctx) + oldParentDir.inode.incRef() + } + newParentDir.inode.touchCMtime() + } + oldParentDir.inode.touchCMtime() + renamed.inode.touchCtime() + replaced.inode.touchCtime() + vfs.InotifyRename(ctx, &renamed.inode.watches, &oldParentDir.inode.watches, &newParentDir.inode.watches, oldName, newName, renamed.inode.isDir()) + vfs.InotifyRename(ctx, &replaced.inode.watches, &newParentDir.inode.watches, &oldParentDir.inode.watches, newName, oldName, replaced.inode.isDir()) + return nil + } if replaced != nil { newParentDir.removeChildLocked(replaced) if replaced.inode.isDir() { diff --git a/test/syscalls/linux/rename.cc b/test/syscalls/linux/rename.cc index d954501e4d1..d00c5f62e41 100644 --- a/test/syscalls/linux/rename.cc +++ b/test/syscalls/linux/rename.cc @@ -15,6 +15,8 @@ #include #include #include +#include +#include #include @@ -31,7 +33,6 @@ #include "test/util/test_util.h" using ::testing::AnyOf; -using ::testing::Matches; namespace gvisor { namespace testing { @@ -503,7 +504,17 @@ TEST(Renameat2Test, NoReplaceDot) { SyscallFailsWithErrno(AnyOf(ENOSYS, EINVAL, EEXIST))); } +// gVisor only supports RENAME_EXCHANGE when running under runsc, and its FUSE +// implementation does not support it at all. +bool RenameExchangeSupported() { + if (IsRunningOnGvisor() && !IsRunningOnRunsc()) { + return false; + } + return absl::NullSafeStringView(getenv("GVISOR_FUSE_TEST")) != "TRUE"; +} + TEST(Renameat2Test, ExchangeDirectories) { + SKIP_IF(!RenameExchangeSupported()); absl::string_view c1 = "xyz"; absl::string_view c2 = "abc"; const auto d1 = @@ -514,45 +525,32 @@ TEST(Renameat2Test, ExchangeDirectories) { TempPath::CreateFileWith(d1.path(), c1, TempPath::kDefaultFileMode)); auto f2 = ASSERT_NO_ERRNO_AND_VALUE( TempPath::CreateFileWith(d2.path(), c2, TempPath::kDefaultFileMode)); - // renameat2 returns EINVAL if the flag is unsupported. - auto rename_result = renameat2(AT_FDCWD, d1.path().c_str(), AT_FDCWD, - d2.path().c_str(), RENAME_EXCHANGE); - EXPECT_THAT(rename_result, - AnyOf(SyscallFailsWithErrno(EINVAL), SyscallSucceeds())); - - if (Matches(SyscallSucceeds())(rename_result)) { - EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f2.path()))), - IsPosixErrorOkAndHolds(true)); - EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f1.path()))), - IsPosixErrorOkAndHolds(false)); - EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f1.path()))), - IsPosixErrorOkAndHolds(true)); - EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f2.path()))), - IsPosixErrorOkAndHolds(false)); - } else { - EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f2.path()))), - IsPosixErrorOkAndHolds(false)); - EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f1.path()))), - IsPosixErrorOkAndHolds(true)); - EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f1.path()))), - IsPosixErrorOkAndHolds(false)); - EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f2.path()))), - IsPosixErrorOkAndHolds(true)); - } + ASSERT_THAT(renameat2(AT_FDCWD, d1.path().c_str(), AT_FDCWD, + d2.path().c_str(), RENAME_EXCHANGE), + SyscallSucceeds()); + + EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f2.path()))), + IsPosixErrorOkAndHolds(true)); + EXPECT_THAT(Exists(JoinPath(d1.path(), Basename(f1.path()))), + IsPosixErrorOkAndHolds(false)); + EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f1.path()))), + IsPosixErrorOkAndHolds(true)); + EXPECT_THAT(Exists(JoinPath(d2.path(), Basename(f2.path()))), + IsPosixErrorOkAndHolds(false)); } TEST(Renameat2Test, PathDoesNotExist) { + SKIP_IF(!RenameExchangeSupported()); const auto d1 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(GetAbsoluteTestTmpdir())); // d2 doesn't exist. const auto d2 = NewTempAbsPathInDir(GetAbsoluteTestTmpdir()); EXPECT_THAT(renameat2(AT_FDCWD, d1.path().c_str(), AT_FDCWD, d2.c_str(), RENAME_EXCHANGE), - SyscallFailsWithErrno(AnyOf(ENOSYS, EINVAL, ENOENT))); - // renameat2 returns EINVAL if the flag is unsupported. + SyscallFailsWithErrno(ENOENT)); EXPECT_THAT(renameat2(AT_FDCWD, d2.c_str(), AT_FDCWD, d1.path().c_str(), RENAME_EXCHANGE), - SyscallFailsWithErrno(AnyOf(EINVAL, ENOENT))); + SyscallFailsWithErrno(ENOENT)); } TEST(Renameat2Test, InvalidFlags) { @@ -567,6 +565,7 @@ TEST(Renameat2Test, InvalidFlags) { } TEST(Renameat2Test, ExchangeFiles) { + SKIP_IF(!RenameExchangeSupported()); absl::string_view c1 = "xyz"; absl::string_view c2 = "abc"; const auto p1 = @@ -577,19 +576,78 @@ TEST(Renameat2Test, ExchangeFiles) { TempPath::CreateFileWith(p1.path(), c1, TempPath::kDefaultFileMode)); const auto f2 = ASSERT_NO_ERRNO_AND_VALUE( TempPath::CreateFileWith(p2.path(), c2, TempPath::kDefaultFileMode)); - auto rename_result = renameat2(AT_FDCWD, f1.path().c_str(), AT_FDCWD, - f2.path().c_str(), RENAME_EXCHANGE); - // renameat2 returns EINVAL if the flag is unsupported. - EXPECT_THAT(rename_result, - AnyOf(SyscallFailsWithErrno(EINVAL), SyscallSucceeds())); - - if (Matches(SyscallSucceeds())(rename_result)) { - EXPECT_THAT(GetContents(f2.path()), IsPosixErrorOkAndHolds(c1)); - EXPECT_THAT(GetContents(f1.path()), IsPosixErrorOkAndHolds(c2)); - } else { - EXPECT_THAT(GetContents(f1.path()), IsPosixErrorOkAndHolds(c1)); - EXPECT_THAT(GetContents(f2.path()), IsPosixErrorOkAndHolds(c2)); - } + ASSERT_THAT(renameat2(AT_FDCWD, f1.path().c_str(), AT_FDCWD, + f2.path().c_str(), RENAME_EXCHANGE), + SyscallSucceeds()); + + EXPECT_THAT(GetContents(f2.path()), IsPosixErrorOkAndHolds(c1)); + EXPECT_THAT(GetContents(f1.path()), IsPosixErrorOkAndHolds(c2)); +} + +TEST(Renameat2Test, ExchangeFilesSameParent) { + SKIP_IF(!RenameExchangeSupported()); + absl::string_view c1 = "xyz"; + absl::string_view c2 = "abc"; + const auto parent = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(GetAbsoluteTestTmpdir())); + const auto f1 = ASSERT_NO_ERRNO_AND_VALUE( + TempPath::CreateFileWith(parent.path(), c1, TempPath::kDefaultFileMode)); + const auto f2 = ASSERT_NO_ERRNO_AND_VALUE( + TempPath::CreateFileWith(parent.path(), c2, TempPath::kDefaultFileMode)); + ASSERT_THAT(renameat2(AT_FDCWD, f1.path().c_str(), AT_FDCWD, + f2.path().c_str(), RENAME_EXCHANGE), + SyscallSucceeds()); + + EXPECT_THAT(GetContents(f2.path()), IsPosixErrorOkAndHolds(c1)); + EXPECT_THAT(GetContents(f1.path()), IsPosixErrorOkAndHolds(c2)); +} + +TEST(Renameat2Test, ExchangeFileAndDirectory) { + SKIP_IF(!RenameExchangeSupported()); + absl::string_view c = "xyz"; + const auto p1 = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(GetAbsoluteTestTmpdir())); + const auto p2 = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(GetAbsoluteTestTmpdir())); + const auto dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(p1.path())); + // The exchanged directory need not be empty. + const auto dir_child = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path())); + const auto file = ASSERT_NO_ERRNO_AND_VALUE( + TempPath::CreateFileWith(p2.path(), c, TempPath::kDefaultFileMode)); + ASSERT_THAT(renameat2(AT_FDCWD, dir.path().c_str(), AT_FDCWD, + file.path().c_str(), RENAME_EXCHANGE), + SyscallSucceeds()); + + const struct stat dir_st = ASSERT_NO_ERRNO_AND_VALUE(Stat(dir.path())); + EXPECT_TRUE(S_ISREG(dir_st.st_mode)); + const struct stat file_st = ASSERT_NO_ERRNO_AND_VALUE(Stat(file.path())); + EXPECT_TRUE(S_ISDIR(file_st.st_mode)); + EXPECT_THAT(GetContents(dir.path()), IsPosixErrorOkAndHolds(c)); + EXPECT_THAT(Exists(JoinPath(file.path(), Basename(dir_child.path()))), + IsPosixErrorOkAndHolds(true)); +} + +TEST(Renameat2Test, ExchangeWithAncestorDirectory) { + SKIP_IF(!RenameExchangeSupported()); + const auto ancestor = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(GetAbsoluteTestTmpdir())); + const auto descendant = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(ancestor.path())); + const auto deep_descendant = + ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(descendant.path())); + EXPECT_THAT(renameat2(AT_FDCWD, ancestor.path().c_str(), AT_FDCWD, + descendant.path().c_str(), RENAME_EXCHANGE), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(renameat2(AT_FDCWD, descendant.path().c_str(), AT_FDCWD, + ancestor.path().c_str(), RENAME_EXCHANGE), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(renameat2(AT_FDCWD, ancestor.path().c_str(), AT_FDCWD, + deep_descendant.path().c_str(), RENAME_EXCHANGE), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(renameat2(AT_FDCWD, deep_descendant.path().c_str(), AT_FDCWD, + ancestor.path().c_str(), RENAME_EXCHANGE), + SyscallFailsWithErrno(EINVAL)); } } // namespace