From 416b0374fb59827db45dfac2d67e7e637b2d5fff Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 28 Dec 2021 12:33:27 +0100 Subject: [PATCH] Kernel: Fix race condition in TmpFSInode::notify_watchers() We were doing this dance in notify_watchers(): set_metadata_dirty(true); set_metadata_dirty(false); This was done in order to force out inode watcher events immediately. Unfortunately, this was racy, as if SyncTask got scheduled at the wrong moment, it would try to flush metadata for a clean inode. This then got trapped by the VERIFY() statement in Inode::sync_all(): VERIFY(inode.is_metadata_dirty()); This patch fixes the issue by replacing notify_watchers() with lazy metadata notifications like all other filesystems. --- Kernel/FileSystem/TmpFS.cpp | 20 +++++++------------- Kernel/FileSystem/TmpFS.h | 1 - 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp index 587e9505e2..c42802d77d 100644 --- a/Kernel/FileSystem/TmpFS.cpp +++ b/Kernel/FileSystem/TmpFS.cpp @@ -176,7 +176,7 @@ ErrorOr TmpFSInode::write_bytes(off_t offset, size_t size, const UserOrK m_content = move(tmp); } m_metadata.size = new_size; - notify_watchers(); + set_metadata_dirty(true); } TRY(buffer.read(m_content->data() + offset, size)); // TODO: partial reads? @@ -210,12 +210,6 @@ TmpFSInode::Child* TmpFSInode::find_child_by_name(StringView name) return nullptr; } -void TmpFSInode::notify_watchers() -{ - set_metadata_dirty(true); - set_metadata_dirty(false); -} - ErrorOr TmpFSInode::flush_metadata() { // We don't really have any metadata that could become dirty. @@ -232,7 +226,7 @@ ErrorOr TmpFSInode::chmod(mode_t mode) MutexLocker locker(m_inode_lock); m_metadata.mode = mode; - notify_watchers(); + set_metadata_dirty(true); return {}; } @@ -242,7 +236,7 @@ ErrorOr TmpFSInode::chown(UserID uid, GroupID gid) m_metadata.uid = uid; m_metadata.gid = gid; - notify_watchers(); + set_metadata_dirty(true); return {}; } @@ -332,7 +326,7 @@ ErrorOr TmpFSInode::truncate(u64 size) } m_metadata.size = size; - notify_watchers(); + set_metadata_dirty(true); return {}; } @@ -341,7 +335,7 @@ ErrorOr TmpFSInode::set_atime(time_t time) MutexLocker locker(m_inode_lock); m_metadata.atime = time; - notify_watchers(); + set_metadata_dirty(true); return {}; } @@ -350,7 +344,7 @@ ErrorOr TmpFSInode::set_ctime(time_t time) MutexLocker locker(m_inode_lock); m_metadata.ctime = time; - notify_watchers(); + set_metadata_dirty(true); return {}; } @@ -359,7 +353,7 @@ ErrorOr TmpFSInode::set_mtime(time_t t) MutexLocker locker(m_inode_lock); m_metadata.mtime = t; - notify_watchers(); + set_metadata_dirty(true); return {}; } diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h index eca20a43e3..741575f2ca 100644 --- a/Kernel/FileSystem/TmpFS.h +++ b/Kernel/FileSystem/TmpFS.h @@ -73,7 +73,6 @@ private: TmpFSInode(TmpFS& fs, const InodeMetadata& metadata, InodeIdentifier parent); static ErrorOr> try_create(TmpFS&, InodeMetadata const& metadata, InodeIdentifier parent); static ErrorOr> try_create_root(TmpFS&); - void notify_watchers(); struct Child { NonnullOwnPtr name;