From 631447da574ec6421c77d37da54ad04b50102688 Mon Sep 17 00:00:00 2001
From: Ben Wiederhake <BenWiederhake.GitHub@gmx.de>
Date: Sun, 7 Nov 2021 00:09:48 +0100
Subject: [PATCH] Kernel: Fix TOCTOU in fstatvfs

In particular, fstatvfs used to assume that a file that was earlier
opened using some path will forever be at that path. This is wrong, and
in the meantime new mounts and new filesystems could take up the
filename or directories, leading to a completely inaccurate result.
This commit improves the situation:
- All filesystem information is now always accurate.
- The mount flags *might* be erroneously zero, if the custody for the
  open file is not available. I don't know when that might happen, but
  it is definitely not the typical case.
---
 Kernel/Process.h            |  2 +-
 Kernel/Syscalls/statvfs.cpp | 47 ++++++++++++-------------------------
 2 files changed, 16 insertions(+), 33 deletions(-)

diff --git a/Kernel/Process.h b/Kernel/Process.h
index bb26b29d3c..c6e3ae6453 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -535,7 +535,7 @@ private:
     ErrorOr<void> do_exec(NonnullRefPtr<OpenFileDescription> main_program_description, NonnullOwnPtrVector<KString> arguments, NonnullOwnPtrVector<KString> environment, RefPtr<OpenFileDescription> interpreter_description, Thread*& new_main_thread, u32& prev_flags, const ElfW(Ehdr) & main_program_header);
     ErrorOr<FlatPtr> do_write(OpenFileDescription&, const UserOrKernelBuffer&, size_t);
 
-    ErrorOr<FlatPtr> do_statvfs(StringView path, statvfs* buf);
+    ErrorOr<FlatPtr> do_statvfs(FileSystem const& path, Custody const*, statvfs* buf);
 
     ErrorOr<RefPtr<OpenFileDescription>> find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size);
 
diff --git a/Kernel/Syscalls/statvfs.cpp b/Kernel/Syscalls/statvfs.cpp
index 678ac6f448..c0478a3985 100644
--- a/Kernel/Syscalls/statvfs.cpp
+++ b/Kernel/Syscalls/statvfs.cpp
@@ -10,12 +10,8 @@
 
 namespace Kernel {
 
-ErrorOr<FlatPtr> Process::do_statvfs(StringView path, statvfs* buf)
+ErrorOr<FlatPtr> Process::do_statvfs(FileSystem const& fs, Custody const* custody, statvfs* buf)
 {
-    auto custody = TRY(VirtualFileSystem::the().resolve_path(path, current_directory(), nullptr, 0));
-    auto& inode = custody->inode();
-    auto& fs = inode.fs();
-
     statvfs kernelbuf = {};
 
     kernelbuf.f_bsize = static_cast<u64>(fs.block_size());
@@ -34,29 +30,8 @@ ErrorOr<FlatPtr> Process::do_statvfs(StringView path, statvfs* buf)
 
     kernelbuf.f_namemax = 255;
 
-    Custody* current_custody = custody;
-
-    while (current_custody) {
-        VirtualFileSystem::the().for_each_mount([&kernelbuf, &current_custody](auto& mount) {
-            if (&current_custody->inode() == &mount.guest()) {
-                int mountflags = mount.flags();
-                int flags = 0;
-                if (mountflags & MS_RDONLY)
-                    flags = flags | ST_RDONLY;
-                if (mountflags & MS_NOSUID)
-                    flags = flags | ST_NOSUID;
-
-                kernelbuf.f_flag = flags;
-                current_custody = nullptr;
-                return IterationDecision::Break;
-            }
-            return IterationDecision::Continue;
-        });
-
-        if (current_custody) {
-            current_custody = current_custody->parent();
-        }
-    }
+    if (custody)
+        kernelbuf.f_flag = custody->mount_flags();
 
     TRY(copy_to_user(buf, &kernelbuf));
     return 0;
@@ -69,7 +44,12 @@ ErrorOr<FlatPtr> Process::sys$statvfs(Userspace<const Syscall::SC_statvfs_params
     auto params = TRY(copy_typed_from_user(user_params));
 
     auto path = TRY(get_syscall_path_argument(params.path));
-    return do_statvfs(path->view(), params.buf);
+
+    auto custody = TRY(VirtualFileSystem::the().resolve_path(path->view(), current_directory(), nullptr, 0));
+    auto& inode = custody->inode();
+    auto const& fs = inode.fs();
+
+    return do_statvfs(fs, custody, params.buf);
 }
 
 ErrorOr<FlatPtr> Process::sys$fstatvfs(int fd, statvfs* buf)
@@ -78,9 +58,12 @@ ErrorOr<FlatPtr> Process::sys$fstatvfs(int fd, statvfs* buf)
     REQUIRE_PROMISE(stdio);
 
     auto description = TRY(fds().open_file_description(fd));
-    auto absolute_path = TRY(description->original_absolute_path());
-    // FIXME: TOCTOU bug! The file connected to the fd may or may not have been moved, and the name possibly taken by a different file.
-    return do_statvfs(absolute_path->view(), buf);
+    auto inode = description->inode();
+    if (inode == nullptr)
+        return ENOENT;
+
+    // FIXME: The custody that we pass in might be outdated. However, this only affects the mount flags.
+    return do_statvfs(inode->fs(), description->custody(), buf);
 }
 
 }