diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index 03a086e57..16ba75647 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -800,25 +800,6 @@ func (fs *filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error if err := vfsObj.PrepareDeleteDentry(mntns, &child.vfsd); err != nil { return err } - // Remove pages used if child being removed is a SymLink or Regular File. - switch impl := child.inode.impl.(type) { - case *symlink: - if len(impl.target) >= shortSymlinkLen { - if err := fs.updatePagesUsed(uint64(len(impl.target)), 0); err != nil { - vfsObj.AbortDeleteDentry(&child.vfsd) - return err - } - } - case *regularFile: - impl.inode.mu.Lock() - if err := fs.updatePagesUsed(impl.size.Load(), 0); err != nil { - impl.inode.mu.Unlock() - vfsObj.AbortDeleteDentry(&child.vfsd) - return err - } - impl.inode.mu.Unlock() - } - // Generate inotify events. Note that this must take place before the link // count of the child is decremented, or else the watches may be dropped // before these events are added. diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index e6c7d8762..b59a4c806 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -443,7 +443,8 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off // Locking f.inode.mu is sufficient for reading f.size. offset = int64(f.size.RacyLoad()) } - if end := offset + srclen; end < offset { + end := offset + srclen + if end < offset { // Overflow. return 0, offset, linuxerr.EINVAL } @@ -452,15 +453,18 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off if err != nil { return 0, offset, err } + maybeSizeInc := false src = src.TakeFirst64(srclen) - reservedSize := f.size.Load() + uint64(srclen) - if err = f.inode.fs.updatePagesUsed(f.size.Load(), reservedSize); err != nil { - return 0, 0, err + if uint64(end) > f.size.Load() { + maybeSizeInc = true + if err = f.inode.fs.updatePagesUsed(f.size.Load(), uint64(end)); err != nil { + return 0, 0, err + } } rw := getRegularFileReadWriter(f, offset) n, err := src.CopyInTo(ctx, rw) - if unwritten := srclen - n; unwritten != 0 { - if err := f.inode.fs.updatePagesUsed(reservedSize, f.size.Load()); err != nil { + if unwritten := srclen - n; maybeSizeInc && unwritten != 0 { + if err := f.inode.fs.updatePagesUsed(uint64(end), f.size.Load()); err != nil { return 0, 0, err } } diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index bcc0cc924..d535f0b73 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -528,12 +528,26 @@ func (i *inode) tryIncRef() bool { func (i *inode) decRef(ctx context.Context) { i.refs.DecRef(func() { i.watches.HandleDeletion(ctx) - if regFile, ok := i.impl.(*regularFile); ok { + // Remove pages used if child being removed is a SymLink or Regular File. + switch impl := i.impl.(type) { + case *symlink: + if len(impl.target) >= shortSymlinkLen { + if err := i.fs.updatePagesUsed(uint64(len(impl.target)), 0); err != nil { + panic(fmt.Sprintf("Encountered error: %v while accounting tmpfs size.", err)) + } + } + case *regularFile: + impl.inode.mu.Lock() + if err := i.fs.updatePagesUsed(impl.size.Load(), 0); err != nil { + panic(fmt.Sprintf("Encountered error: %v while accounting tmpfs size.", err)) + } + impl.inode.mu.Unlock() // Release memory used by regFile to store data. Since regFile is // no longer usable, we don't need to grab any locks or update any // metadata. - regFile.data.DropAll(regFile.memFile) + impl.data.DropAll(impl.memFile) } + }) } diff --git a/test/syscalls/linux/mount.cc b/test/syscalls/linux/mount.cc index 36a90ad93..0b127643f 100644 --- a/test/syscalls/linux/mount.cc +++ b/test/syscalls/linux/mount.cc @@ -582,6 +582,38 @@ TEST(MountTest, TmpfsSymlinkAllocCheck) { EXPECT_THAT(symlink(target.c_str(), pathname.c_str()), SyscallSucceeds()); } +// Tests memory allocation for Hard Links is not double allocated. +TEST(MountTest, TmpfsHardLinkAllocCheck) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + auto const dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); + auto tmpfs_size_opt = absl::StrCat("size=", kPageSize); + auto const mount = ASSERT_NO_ERRNO_AND_VALUE( + Mount("", dir.path(), "tmpfs", 0, tmpfs_size_opt, 0)); + const std::string fileOne = JoinPath(dir.path(), "foo1"); + const std::string fileTwo = JoinPath(dir.path(), "foo2"); + auto const fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(fileOne, O_CREAT | O_RDWR, 0777)); + EXPECT_THAT(link(fileOne.c_str(), fileTwo.c_str()), SyscallSucceeds()); + + // Check that it starts at size zero. + struct stat buf; + ASSERT_THAT(fstat(fd.get(), &buf), SyscallSucceeds()); + EXPECT_EQ(buf.st_size, 0); + + // Grow to 1 Page Size. + ASSERT_THAT(fallocate(fd.get(), 0, 0, kPageSize), SyscallSucceeds()); + ASSERT_THAT(fstat(fd.get(), &buf), SyscallSucceeds()); + EXPECT_EQ(buf.st_size, kPageSize); + + // Grow to size beyond tmpfs allocated bytes. + ASSERT_THAT(fallocate(fd.get(), 0, 0, kPageSize + 1), + SyscallFailsWithErrno(ENOSPC)); + ASSERT_THAT(fstat(fd.get(), &buf), SyscallSucceeds()); + EXPECT_EQ(buf.st_size, kPageSize); + EXPECT_THAT(unlink(fileTwo.c_str()), SyscallSucceeds()); + EXPECT_THAT(unlink(fileOne.c_str()), SyscallSucceeds()); +} + } // namespace } // namespace testing