소스 검색

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.
Andreas Kling 4 년 전
부모
커밋
1fdd39ff14
3개의 변경된 파일13개의 추가작업 그리고 5개의 파일을 삭제
  1. 9 2
      Kernel/FileSystem/Inode.cpp
  2. 1 1
      Kernel/FileSystem/Inode.h
  3. 3 2
      Kernel/FileSystem/VirtualFileSystem.cpp

+ 9 - 2
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;
 }
 

+ 1 - 1
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 };

+ 3 - 2
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);