From 1fdd39ff14435570081b3681d486fcd52920fa5c Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Thu, 31 Dec 2020 02:10:31 +0100
Subject: [PATCH] Kernel: Sprinkle some lockers in Inode

It did look pretty suspicious the way we were accessing members in some
of these functions without taking the lock first.
---
 Kernel/FileSystem/Inode.cpp             | 11 +++++++++--
 Kernel/FileSystem/Inode.h               |  2 +-
 Kernel/FileSystem/VirtualFileSystem.cpp |  5 +++--
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp
index da877ce0ae..ada6a7ae86 100644
--- a/Kernel/FileSystem/Inode.cpp
+++ b/Kernel/FileSystem/Inode.cpp
@@ -132,18 +132,21 @@ Inode::~Inode()
 
 void Inode::will_be_destroyed()
 {
+    LOCKER(m_lock);
     if (m_metadata_dirty)
         flush_metadata();
 }
 
 void Inode::inode_contents_changed(off_t offset, ssize_t size, const UserOrKernelBuffer& data)
 {
+    LOCKER(m_lock);
     if (auto shared_vmobject = this->shared_vmobject())
         shared_vmobject->inode_contents_changed({}, offset, size, data);
 }
 
 void Inode::inode_size_changed(size_t old_size, size_t new_size)
 {
+    LOCKER(m_lock);
     if (auto shared_vmobject = this->shared_vmobject())
         shared_vmobject->inode_size_changed({}, old_size, new_size);
 }
@@ -175,6 +178,7 @@ KResult Inode::decrement_link_count()
 
 void Inode::set_shared_vmobject(SharedInodeVMObject& vmobject)
 {
+    LOCKER(m_lock);
     m_shared_vmobject = vmobject;
 }
 
@@ -210,8 +214,9 @@ void Inode::unregister_watcher(Badge<InodeWatcher>, InodeWatcher& watcher)
     m_watchers.remove(&watcher);
 }
 
-FIFO& Inode::fifo()
+NonnullRefPtr<FIFO> Inode::fifo()
 {
+    LOCKER(m_lock);
     ASSERT(metadata().is_fifo());
 
     // FIXME: Release m_fifo when it is closed by all readers and writers
@@ -224,6 +229,7 @@ FIFO& Inode::fifo()
 
 void Inode::set_metadata_dirty(bool metadata_dirty)
 {
+    LOCKER(m_lock);
     if (m_metadata_dirty == metadata_dirty)
         return;
 
@@ -231,7 +237,6 @@ void Inode::set_metadata_dirty(bool metadata_dirty)
     if (m_metadata_dirty) {
         // FIXME: Maybe we should hook into modification events somewhere else, I'm not sure where.
         //        We don't always end up on this particular code path, for instance when writing to an ext2fs file.
-        LOCKER(m_lock);
         for (auto& watcher : m_watchers) {
             watcher->notify_inode_event({}, InodeWatcherEvent::Type::Modified);
         }
@@ -271,11 +276,13 @@ KResult Inode::prepare_to_write_data()
 
 RefPtr<SharedInodeVMObject> Inode::shared_vmobject() const
 {
+    LOCKER(m_lock);
     return m_shared_vmobject.strong_ref();
 }
 
 bool Inode::is_shared_vmobject(const SharedInodeVMObject& other) const
 {
+    LOCKER(m_lock);
     return m_shared_vmobject.unsafe_ptr() == &other;
 }
 
diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h
index b3c8a3f700..399fb9ecef 100644
--- a/Kernel/FileSystem/Inode.h
+++ b/Kernel/FileSystem/Inode.h
@@ -113,7 +113,7 @@ public:
     void register_watcher(Badge<InodeWatcher>, InodeWatcher&);
     void unregister_watcher(Badge<InodeWatcher>, InodeWatcher&);
 
-    FIFO& fifo();
+    NonnullRefPtr<FIFO> fifo();
 
     // For InlineLinkedListNode.
     Inode* m_next { nullptr };
diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp
index 392aeba8c3..f61cd3d5c9 100644
--- a/Kernel/FileSystem/VirtualFileSystem.cpp
+++ b/Kernel/FileSystem/VirtualFileSystem.cpp
@@ -297,14 +297,15 @@ KResultOr<NonnullRefPtr<FileDescription>> VFS::open(StringView path, int options
         return *preopen_fd;
 
     if (metadata.is_fifo()) {
+        auto fifo = inode.fifo();
         if (options & O_WRONLY) {
-            auto description = inode.fifo().open_direction_blocking(FIFO::Direction::Writer);
+            auto description = fifo->open_direction_blocking(FIFO::Direction::Writer);
             description->set_rw_mode(options);
             description->set_file_flags(options);
             description->set_original_inode({}, inode);
             return description;
         } else if (options & O_RDONLY) {
-            auto description = inode.fifo().open_direction_blocking(FIFO::Direction::Reader);
+            auto description = fifo->open_direction_blocking(FIFO::Direction::Reader);
             description->set_rw_mode(options);
             description->set_file_flags(options);
             description->set_original_inode({}, inode);