diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index b4ad1b054..aa5a6650d 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -967,7 +967,7 @@ func (fs *filesystem) checkFillAllocation(pagesReqd, pagesAlloced uint64) { // The returned value is guaranteed to be <= pagesInc. If the size mount option is // not set, then pagesInc will be returned. func (fs *filesystem) accountPagesPartial(pagesInc uint64) uint64 { - if fs.maxSizeInPages == 0 || pagesInc == 0 { + if pagesInc == 0 { return pagesInc } @@ -993,7 +993,7 @@ func (fs *filesystem) accountPagesPartial(pagesInc uint64) uint64 { // is mounted with size option. We return a false when the maxSizeInPages // has been exhausted and no more allocation can be done. func (fs *filesystem) accountPages(pagesInc uint64) bool { - if fs.maxSizeInPages == 0 || pagesInc == 0 { + if pagesInc == 0 { return true // No accounting needed. } @@ -1017,7 +1017,7 @@ func (fs *filesystem) accountPages(pagesInc uint64) bool { // unaccountPages decreases the pagesUsed in filesystem struct if tmpfs // is mounted with size option. func (fs *filesystem) unaccountPages(pagesDec uint64) { - if fs.maxSizeInPages == 0 || pagesDec == 0 { + if pagesDec == 0 { return } diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 6daec5bdd..a4cfef541 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -300,28 +300,22 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap. if optional.End > pgend { optional.End = pgend } - var pagesReqd uint64 - if rf.inode.fs.maxSizeInPages > 0 { - pagesReqd = rf.data.PagesToFill(required, optional) + pagesReqd := rf.data.PagesToFill(required, optional) + if !rf.inode.fs.accountPages(pagesReqd) { + // If we can not accommodate pagesReqd pages, then retry with just + // the required range. Because optional may be larger than required. + // Only error out if even the required range can not be allocated for. + pagesReqd = rf.data.PagesToFill(required, required) if !rf.inode.fs.accountPages(pagesReqd) { - // If we can not accommodate pagesReqd pages, then retry with just - // the required range. Because optional may be larger than required. - // Only error out if even the required range can not be allocated for. - pagesReqd = rf.data.PagesToFill(required, required) - if !rf.inode.fs.accountPages(pagesReqd) { - return nil, &memmap.BusError{linuxerr.ENOSPC} - } - optional = required + return nil, &memmap.BusError{linuxerr.ENOSPC} } + optional = required } pagesAlloced, cerr := rf.data.Fill(ctx, required, optional, rf.size.RacyLoad(), rf.memFile, rf.memoryUsageKind, false /* populate */, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) { // Newly-allocated pages are zeroed, so we don't need to do anything. return dsts.NumBytes(), nil }) - - if rf.inode.fs.maxSizeInPages > 0 { - rf.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced) - } + rf.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced) var ts []memmap.Translation var translatedEnd uint64 @@ -389,12 +383,9 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint return linuxerr.EFBIG } required := memmap.MappableRange{Start: uint64(pgstartaddr), End: uint64(pgendaddr)} - var pagesReqd uint64 - if f.inode.fs.maxSizeInPages > 0 { - pagesReqd = f.data.PagesToFill(required, required) - if !f.inode.fs.accountPages(pagesReqd) { - return linuxerr.ENOSPC - } + pagesReqd := f.data.PagesToFill(required, required) + if !f.inode.fs.accountPages(pagesReqd) { + return linuxerr.ENOSPC } // Pass populate = true here despite the fact that we don't touch these pages // both for consistency with the expected behavior of fallocate(2) and in @@ -404,15 +395,10 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint return dsts.NumBytes(), nil }) if err != nil && err != io.EOF { - if f.inode.fs.maxSizeInPages > 0 { - f.inode.fs.unaccountPages(pagesReqd) - } + f.inode.fs.unaccountPages(pagesReqd) return err } - - if f.inode.fs.maxSizeInPages > 0 { - f.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced) - } + f.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced) oldSize := f.size.Load() if oldSize >= newSize { diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index c9be9136e..9aabc06a0 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -202,8 +202,16 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt } rootKGID = kgid } + // In Linux, the default size limit is set to 50% of physical RAM. Such a + // default is not suitable in gVisor, because we don't want to reveal the + // host's RAM size. If Linux tmpfs is mounted with no size option, then + // statfs(2) returns f_blocks == f_bfree == f_bavail == 0. + // However, many applications treat this as having a size limit + // of 0. To work around these, set a very large but non-zero default size + // limit, chosen to ensure that BlockSize * Blocks does not overflow int64 + // (which applications may also handle incorrectly). + maxSizeInPages := uint64(math.MaxInt64 / hostarch.PageSize) maxSizeStr, ok := mopts["size"] - var maxSizeInPages uint64 if ok { delete(mopts, "size") maxSizeInBytes, err := parseSize(maxSizeStr) @@ -313,27 +321,11 @@ func (fs *filesystem) statFS() linux.Statfs { NameLength: linux.NAME_MAX, } - // tmpfs supports configurable size limits. - // In Linux, if tmpfs is mounted with size option, - // we return the block sizes as set by the user. - if fs.maxSizeInPages > 0 { - // If size is set for tmpfs return set values. - st.Blocks = fs.maxSizeInPages - pagesUsed := fs.pagesUsed.Load() - st.BlocksFree = fs.maxSizeInPages - pagesUsed - st.BlocksAvailable = fs.maxSizeInPages - pagesUsed - return st - } - // In Linux, if tmpfs is mounted with no size option, - // such a tmpfs mount will return - // f_blocks == f_bfree == f_bavail == 0 from statfs(2). - // However, many applications treat this as having a size limit - // of 0. To work around this, claim to have a very large but non-zero size, - // chosen to ensure that BlockSize * Blocks does not overflow int64 (which - // applications may also handle incorrectly). - st.Blocks = math.MaxInt64 / hostarch.PageSize - st.BlocksFree = math.MaxInt64 / hostarch.PageSize - st.BlocksAvailable = math.MaxInt64 / hostarch.PageSize + // If size is set for tmpfs return set values. + st.Blocks = fs.maxSizeInPages + pagesUsed := fs.pagesUsed.Load() + st.BlocksFree = fs.maxSizeInPages - pagesUsed + st.BlocksAvailable = fs.maxSizeInPages - pagesUsed return st } diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 282cd5257..12f44de26 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -3799,6 +3799,7 @@ cc_binary( "//test/util:file_descriptor", "@com_google_absl//absl/strings", gtest, + "//test/util:fs_util", "//test/util:posix_error", "//test/util:temp_path", "//test/util:test_main", diff --git a/test/syscalls/linux/statfs.cc b/test/syscalls/linux/statfs.cc index 57015511b..09bb3dd7a 100644 --- a/test/syscalls/linux/statfs.cc +++ b/test/syscalls/linux/statfs.cc @@ -19,6 +19,7 @@ #include "gtest/gtest.h" #include "test/util/file_descriptor.h" +#include "test/util/fs_util.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" @@ -89,6 +90,24 @@ TEST(FstatfsTest, InternalDevShm) { EXPECT_TRUE(st.f_type == TMPFS_MAGIC || st.f_type == OVERLAYFS_SUPER_MAGIC); } +// Tests that the number of blocks free in the filesystem, as reported by +// statfs(2) updates appropriately when pages are allocated. +TEST(FstatfsTest, BlocksFree) { + const std::string file_path = NewTempAbsPath(); + const std::string dir = std::string(Dirname(file_path)); + struct statfs st_before; + EXPECT_THAT(statfs(dir.c_str(), &st_before), SyscallSucceeds()); + // Only test for tmpfs. Passthru gofer does not expose host filesystem + // statfs(2) results. It always returns 0 for blocks free. + SKIP_IF(st_before.f_type != TMPFS_MAGIC); + + ASSERT_NO_ERRNO(CreateWithContents(file_path, "abcd")); + struct statfs st_after; + EXPECT_THAT(statfs(dir.c_str(), &st_after), SyscallSucceeds()); + EXPECT_GT(st_before.f_bfree, st_after.f_bfree); + EXPECT_GT(st_before.f_bavail, st_after.f_bavail); +} + } // namespace } // namespace testing