From 5a2b4ed67155dec1b68b5b5b4297dbc2ff01fa0b Mon Sep 17 00:00:00 2001 From: matthewvon Date: Thu, 10 Jun 2021 11:10:42 -0700 Subject: [PATCH] BugFix: fs_posix.cc GetFreeSpace uses wrong value non-root users (#8370) Summary: fs_posix.cc GetFreeSpace() calculates free space based upon a call to statvfs(). However, there are two extremely different values in statvfs's returned structure: f_bfree which is free space for root and f_bavail which is free space for non-root users. The existing code uses f_bfree. Many disks have 5 to 10% of the total disk space reserved for root only. Therefore GetFreeSpace() does not realize that non-root users may not have storage available. This PR detects whether the effective posix user is root or not, then selects the appropriate available space value. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8370 Reviewed By: mrambacher Differential Revision: D29032710 Pulled By: jay-zhuang fbshipit-source-id: 57feba34ed035615a479956d28f98d85735281c0 --- HISTORY.md | 2 ++ env/fs_posix.cc | 27 +++++++++++++++++---------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 604b09673..1cd302d7e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -3,6 +3,8 @@ ### Behavior Changes * Added API comments clarifying safe usage of Disable/EnableManualCompaction and EventListener callbacks for compaction. * Obsolete keys in the bottommost level that were preserved for a snapshot will now be cleaned upon snapshot release in all cases. This form of compaction (snapshot release triggered compaction) previously had an artificial limitation that multiple tombstones needed to be present. +### Bug Fixes +* fs_posix.cc GetFreeSpace() always report disk space available to root even when running as non-root. Linux defaults often have disk mounts with 5 to 10 percent of total space reserved only for root. Out of space could result for non-root users. ## 6.21.0 (2021-05-21) ### Bug Fixes diff --git a/env/fs_posix.cc b/env/fs_posix.cc index 84bab192b..a3e360806 100644 --- a/env/fs_posix.cc +++ b/env/fs_posix.cc @@ -15,7 +15,6 @@ #endif #include #include - #include #include #include @@ -32,6 +31,7 @@ #include #include #include + #include // Get nano time includes #if defined(OS_LINUX) || defined(OS_FREEBSD) @@ -81,9 +81,7 @@ inline mode_t GetDBFileMode(bool allow_non_owner_access) { return allow_non_owner_access ? 0644 : 0600; } -static uint64_t gettid() { - return Env::Default()->GetThreadID(); -} +static uint64_t gettid() { return Env::Default()->GetThreadID(); } // list of pathnames that are locked // Only used for error message. @@ -267,8 +265,7 @@ class PosixFileSystem : public FileSystem { } virtual IOStatus OpenWritableFile(const std::string& fname, - const FileOptions& options, - bool reopen, + const FileOptions& options, bool reopen, std::unique_ptr* result, IODebugContext* /*dbg*/) { result->reset(); @@ -551,8 +548,8 @@ class PosixFileSystem : public FileSystem { } IOStatus NewLogger(const std::string& fname, const IOOptions& /*opts*/, - std::shared_ptr* result, - IODebugContext* /*dbg*/) override { + std::shared_ptr* result, + IODebugContext* /*dbg*/) override { FILE* f = nullptr; int fd; { @@ -909,7 +906,17 @@ class PosixFileSystem : public FileSystem { return IOError("While doing statvfs", fname, errno); } - *free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bfree); + // sbuf.bfree is total free space available to root + // sbuf.bavail is total free space available to unprivileged user + // sbuf.bavail <= sbuf.bfree ... pick correct based upon effective user id + if (geteuid()) { + // non-zero user is unprivileged, or -1 if error. take more conservative + // size + *free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bavail); + } else { + // root user can access all disk space + *free_space = ((uint64_t)sbuf.f_bsize * sbuf.f_bfree); + } return IOStatus::OK(); } @@ -938,7 +945,7 @@ class PosixFileSystem : public FileSystem { } FileOptions OptimizeForLogWrite(const FileOptions& file_options, - const DBOptions& db_options) const override { + const DBOptions& db_options) const override { FileOptions optimized = file_options; optimized.use_mmap_writes = false; optimized.use_direct_writes = false;