From 5efb91ec06524331aafc6bf36bfefcdb3206cf61 Mon Sep 17 00:00:00 2001
From: Liav A <liavalb@gmail.com>
Date: Fri, 4 Aug 2023 22:57:25 +0300
Subject: [PATCH] Kernel/VFS: Ensure working with mount entry per a custody is
 safe

Previously we could get a raw pointer to a Mount object which might be
invalid when actually dereferencing it.
To ensure this could not happen, we should just use a callback that will
be used immediately after finding the appropriate Mount entry, while
holding the mount table lock.
---
 Kernel/FileSystem/VirtualFileSystem.cpp | 31 ++++++++++++++-----------
 Kernel/FileSystem/VirtualFileSystem.h   |  3 +--
 2 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp
index fb62e19dc1..a1d5219f69 100644
--- a/Kernel/FileSystem/VirtualFileSystem.cpp
+++ b/Kernel/FileSystem/VirtualFileSystem.cpp
@@ -245,11 +245,9 @@ ErrorOr<void> VirtualFileSystem::remount(Custody& mount_point, int new_flags)
 {
     dbgln("VirtualFileSystem: Remounting inode {}", mount_point.inode().identifier());
 
-    auto* mount = find_mount_for_host_custody(mount_point);
-    if (!mount)
-        return ENODEV;
-
-    mount->set_flags(new_flags);
+    TRY(apply_to_mount_for_host_custody(mount_point, [new_flags](auto& mount) {
+        mount.set_flags(new_flags);
+    }));
     return {};
 }
 
@@ -372,25 +370,28 @@ ErrorOr<void> VirtualFileSystem::mount_root(FileSystem& fs)
     return {};
 }
 
-auto VirtualFileSystem::find_mount_for_host_custody(Custody const& current_custody) -> Mount*
+ErrorOr<void> VirtualFileSystem::apply_to_mount_for_host_custody(Custody const& current_custody, Function<void(Mount&)> callback)
 {
-    return m_mounts.with([&](auto& mounts) -> Mount* {
+    return m_mounts.with([&](auto& mounts) -> ErrorOr<void> {
         // NOTE: We either search for the root mount or for a mount that has a parent custody!
         if (!current_custody.parent()) {
             for (auto& mount : mounts) {
-                if (!mount.host_custody())
-                    return &mount;
+                if (!mount.host_custody()) {
+                    callback(mount);
+                    return {};
+                }
             }
             // NOTE: There must be a root mount entry, so fail if we don't find it.
             VERIFY_NOT_REACHED();
         } else {
             for (auto& mount : mounts) {
                 if (mount.host_custody() && check_matching_absolute_path_hierarchy(*mount.host_custody(), current_custody)) {
-                    return &mount;
+                    callback(mount);
+                    return {};
                 }
             }
         }
-        return nullptr;
+        return Error::from_errno(ENODEV);
     });
 }
 
@@ -1225,9 +1226,11 @@ ErrorOr<NonnullRefPtr<Custody>> VirtualFileSystem::resolve_path_without_veil(Cre
 
         // See if there's something mounted on the child; in that case
         // we would need to return the guest inode, not the host inode.
-        if (auto mount = find_mount_for_host_custody(current_custody)) {
-            child_inode = mount->guest();
-            mount_flags_for_child = mount->flags();
+        auto found_mount_or_error = apply_to_mount_for_host_custody(current_custody, [&child_inode, &mount_flags_for_child](auto& mount) {
+            child_inode = mount.guest();
+            mount_flags_for_child = mount.flags();
+        });
+        if (!found_mount_or_error.is_error()) {
             custody = TRY(Custody::try_create(&parent, part, *child_inode, mount_flags_for_child));
         } else {
             custody = current_custody;
diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h
index ad7c7afb3d..464b721da6 100644
--- a/Kernel/FileSystem/VirtualFileSystem.h
+++ b/Kernel/FileSystem/VirtualFileSystem.h
@@ -114,8 +114,7 @@ private:
     static bool check_matching_absolute_path_hierarchy(Custody const& first_custody, Custody const& second_custody);
     bool mount_point_exists_at_custody(Custody& mount_point);
 
-    // FIXME: This function is totally unsafe as someone could unmount the returned Mount underneath us.
-    Mount* find_mount_for_host_custody(Custody const& current_custody);
+    ErrorOr<void> apply_to_mount_for_host_custody(Custody const& current_custody, Function<void(Mount&)>);
 
     RefPtr<Inode> m_root_inode;