From c88cc8557f20668f575dc07d197b8189c94e46af Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 6 Aug 2022 04:22:20 +0300 Subject: [PATCH] Kernel/FileSystem: Make Inode::{write,read}_bytes methods non-virtual We make these methods non-virtual because we want to ensure we properly enforce locking of the m_inode_lock mutex. Also, for write operations, we want to call prepare_to_write_data before the actual write. The previous design required us to ensure the callers do that at various places which lead to hard-to-find bugs. By moving everything to a place where we call prepare_to_write_data only once, we eliminate a possibilty of forgeting to call it on some code path in the kernel. --- Kernel/FileSystem/DevPtsFS.cpp | 4 ++-- Kernel/FileSystem/DevPtsFS.h | 4 ++-- Kernel/FileSystem/DevTmpFS.cpp | 21 ++++++++++----------- Kernel/FileSystem/DevTmpFS.h | 12 ++++++------ Kernel/FileSystem/Ext2FileSystem.cpp | 16 ++++++++-------- Kernel/FileSystem/Ext2FileSystem.h | 4 ++-- Kernel/FileSystem/ISO9660FileSystem.cpp | 6 +++--- Kernel/FileSystem/ISO9660FileSystem.h | 6 ++++-- Kernel/FileSystem/Inode.cpp | 13 +++++++++++++ Kernel/FileSystem/Inode.h | 8 ++++++-- Kernel/FileSystem/InodeFile.cpp | 7 +------ Kernel/FileSystem/Plan9FileSystem.cpp | 4 ++-- Kernel/FileSystem/Plan9FileSystem.h | 6 ++++-- Kernel/FileSystem/ProcFS.cpp | 14 +++++++------- Kernel/FileSystem/ProcFS.h | 12 ++++++------ Kernel/FileSystem/SysFS.cpp | 4 ++-- Kernel/FileSystem/SysFS.h | 4 ++-- Kernel/FileSystem/TmpFS.cpp | 6 +++--- Kernel/FileSystem/TmpFS.h | 6 ++++-- Kernel/FileSystem/VirtualFileSystem.cpp | 2 -- 20 files changed, 87 insertions(+), 72 deletions(-) diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp index d73442c8fe..1643cddadc 100644 --- a/Kernel/FileSystem/DevPtsFS.cpp +++ b/Kernel/FileSystem/DevPtsFS.cpp @@ -78,12 +78,12 @@ DevPtsFSInode::DevPtsFSInode(DevPtsFS& fs, InodeIndex index, SlavePTY* pty) DevPtsFSInode::~DevPtsFSInode() = default; -ErrorOr DevPtsFSInode::read_bytes(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const +ErrorOr DevPtsFSInode::read_bytes_locked(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const { VERIFY_NOT_REACHED(); } -ErrorOr DevPtsFSInode::write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) +ErrorOr DevPtsFSInode::write_bytes_locked(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) { VERIFY_NOT_REACHED(); } diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h index 18d4032148..9b3951820b 100644 --- a/Kernel/FileSystem/DevPtsFS.h +++ b/Kernel/FileSystem/DevPtsFS.h @@ -47,12 +47,12 @@ private: DevPtsFSInode(DevPtsFS&, InodeIndex, SlavePTY*); // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode&, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; diff --git a/Kernel/FileSystem/DevTmpFS.cpp b/Kernel/FileSystem/DevTmpFS.cpp index 5e0c84a157..ea41a1bb7e 100644 --- a/Kernel/FileSystem/DevTmpFS.cpp +++ b/Kernel/FileSystem/DevTmpFS.cpp @@ -51,7 +51,7 @@ DevTmpFSInode::DevTmpFSInode(DevTmpFS& fs, MajorNumber major_number, MinorNumber { } -ErrorOr DevTmpFSInode::read_bytes(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const +ErrorOr DevTmpFSInode::read_bytes_locked(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const { VERIFY_NOT_REACHED(); } @@ -71,7 +71,7 @@ ErrorOr DevTmpFSInode::flush_metadata() return {}; } -ErrorOr DevTmpFSInode::write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) +ErrorOr DevTmpFSInode::write_bytes_locked(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) { VERIFY_NOT_REACHED(); } @@ -171,20 +171,19 @@ DevTmpFSLinkInode::DevTmpFSLinkInode(DevTmpFS& fs, NonnullOwnPtr name) { } -ErrorOr DevTmpFSLinkInode::read_bytes(off_t offset, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const +ErrorOr DevTmpFSLinkInode::read_bytes_locked(off_t offset, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const { - MutexLocker locker(m_inode_lock); + VERIFY(m_inode_lock.is_locked()); VERIFY(offset == 0); VERIFY(m_link); TRY(buffer.write(m_link->characters() + offset, m_link->length())); return m_link->length(); } -ErrorOr DevTmpFSLinkInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription*) +ErrorOr DevTmpFSLinkInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription*) { + VERIFY(m_inode_lock.is_locked()); auto new_string = TRY(buffer.try_copy_into_kstring(count)); - - MutexLocker locker(m_inode_lock); VERIFY(offset == 0); VERIFY(buffer.is_kernel_buffer()); m_link = move(new_string); @@ -303,9 +302,9 @@ StringView DevTmpFSDeviceInode::name() const return m_name->view(); } -ErrorOr DevTmpFSDeviceInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const +ErrorOr DevTmpFSDeviceInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const { - MutexLocker locker(m_inode_lock); + VERIFY(m_inode_lock.is_locked()); VERIFY(!!description); LockRefPtr device = DeviceManagement::the().get_device(m_major_number, m_minor_number); if (!device) @@ -318,9 +317,9 @@ ErrorOr DevTmpFSDeviceInode::read_bytes(off_t offset, size_t count, User return result.value(); } -ErrorOr DevTmpFSDeviceInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* description) +ErrorOr DevTmpFSDeviceInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* description) { - MutexLocker locker(m_inode_lock); + VERIFY(m_inode_lock.is_locked()); VERIFY(!!description); LockRefPtr device = DeviceManagement::the().get_device(m_major_number, m_minor_number); if (!device) diff --git a/Kernel/FileSystem/DevTmpFS.h b/Kernel/FileSystem/DevTmpFS.h index 67f8bc02d1..034cd7cde5 100644 --- a/Kernel/FileSystem/DevTmpFS.h +++ b/Kernel/FileSystem/DevTmpFS.h @@ -48,12 +48,12 @@ public: protected: explicit DevTmpFSInode(DevTmpFS&); DevTmpFSInode(DevTmpFS&, MajorNumber, MinorNumber); - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; virtual InodeMetadata metadata() const override final; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode&, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; @@ -95,8 +95,8 @@ private: virtual Type node_type() const override { return m_block_device ? Type::BlockDevice : Type::CharacterDevice; } // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; NonnullOwnPtr m_name; bool const m_block_device; @@ -116,8 +116,8 @@ protected: virtual Type node_type() const override { return Type::Link; } // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; NonnullOwnPtr m_name; OwnPtr m_link; diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index 1a09ee2309..87bfc6de7c 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -811,9 +811,9 @@ ErrorOr Ext2FSInode::compute_block_list_with_exclusive_locking() return {}; } -ErrorOr Ext2FSInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const +ErrorOr Ext2FSInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const { - MutexLocker inode_locker(m_inode_lock); + VERIFY(m_inode_lock.is_locked()); VERIFY(offset >= 0); if (m_raw_inode.i_size == 0) return 0; @@ -952,7 +952,7 @@ ErrorOr Ext2FSInode::resize(u64 new_size) return {}; } -ErrorOr Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& data, OpenFileDescription* description) +ErrorOr Ext2FSInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& data, OpenFileDescription* description) { VERIFY(m_inode_lock.is_locked()); VERIFY(offset >= 0); @@ -963,7 +963,7 @@ ErrorOr Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne if (is_symlink()) { VERIFY(offset == 0); if (max((size_t)(offset + count), (size_t)m_raw_inode.i_size) < max_inline_symlink_length) { - dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes(): Poking into i_block array for inline symlink ({} bytes)", identifier(), count); + dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Poking into i_block array for inline symlink ({} bytes)", identifier(), count); TRY(data.read(((u8*)m_raw_inode.i_block) + offset, count)); if ((size_t)(offset + count) > (size_t)m_raw_inode.i_size) m_raw_inode.i_size = offset + count; @@ -997,14 +997,14 @@ ErrorOr Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne size_t nwritten = 0; auto remaining_count = min((off_t)count, (off_t)new_size - offset); - dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes(): Writing {} bytes, {} bytes into inode from {}", identifier(), count, offset, data.user_or_kernel_ptr()); + dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Writing {} bytes, {} bytes into inode from {}", identifier(), count, offset, data.user_or_kernel_ptr()); for (auto bi = first_block_logical_index; remaining_count && bi <= last_block_logical_index; bi = bi.value() + 1) { size_t offset_into_block = (bi == first_block_logical_index) ? offset_into_first_block : 0; size_t num_bytes_to_copy = min((size_t)block_size - offset_into_block, (size_t)remaining_count); - dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes(): Writing block {} (offset_into_block: {})", identifier(), m_block_list[bi.value()], offset_into_block); + dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): Writing block {} (offset_into_block: {})", identifier(), m_block_list[bi.value()], offset_into_block); if (auto result = fs().write_block(m_block_list[bi.value()], data.offset(nwritten), num_bytes_to_copy, offset_into_block, allow_cache); result.is_error()) { - dbgln("Ext2FSInode[{}]::write_bytes(): Failed to write block {} (index {})", identifier(), m_block_list[bi.value()], bi); + dbgln("Ext2FSInode[{}]::write_bytes_locked(): Failed to write block {} (index {})", identifier(), m_block_list[bi.value()], bi); return result.release_error(); } remaining_count -= num_bytes_to_copy; @@ -1013,7 +1013,7 @@ ErrorOr Ext2FSInode::write_bytes(off_t offset, size_t count, UserOrKerne did_modify_contents(); - dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes(): After write, i_size={}, i_blocks={} ({} blocks in list)", identifier(), size(), m_raw_inode.i_blocks, m_block_list.size()); + dbgln_if(EXT2_VERY_DEBUG, "Ext2FSInode[{}]::write_bytes_locked(): After write, i_size={}, i_blocks={} ({} blocks in list)", identifier(), size(), m_raw_inode.i_blocks, m_block_list.size()); return nwritten; } diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h index 8a5abff095..9c65ee68f4 100644 --- a/Kernel/FileSystem/Ext2FileSystem.h +++ b/Kernel/FileSystem/Ext2FileSystem.h @@ -35,12 +35,12 @@ public: private: // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode& child, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; diff --git a/Kernel/FileSystem/ISO9660FileSystem.cpp b/Kernel/FileSystem/ISO9660FileSystem.cpp index 528cebc5bb..c7fbf99f09 100644 --- a/Kernel/FileSystem/ISO9660FileSystem.cpp +++ b/Kernel/FileSystem/ISO9660FileSystem.cpp @@ -398,9 +398,9 @@ u32 ISO9660FS::calculate_directory_entry_cache_key(ISO::DirectoryRecordHeader co return LittleEndian { record.extent_location.little }; } -ErrorOr ISO9660Inode::read_bytes(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const +ErrorOr ISO9660Inode::read_bytes_locked(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const { - MutexLocker inode_locker(m_inode_lock); + VERIFY(m_inode_lock.is_locked()); u32 data_length = LittleEndian { m_record.data_length.little }; u32 extent_location = LittleEndian { m_record.extent_location.little }; @@ -493,7 +493,7 @@ ErrorOr ISO9660Inode::flush_metadata() return {}; } -ErrorOr ISO9660Inode::write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) +ErrorOr ISO9660Inode::write_bytes_locked(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) { return EROFS; } diff --git a/Kernel/FileSystem/ISO9660FileSystem.h b/Kernel/FileSystem/ISO9660FileSystem.h index 9480f2d40d..db97de8480 100644 --- a/Kernel/FileSystem/ISO9660FileSystem.h +++ b/Kernel/FileSystem/ISO9660FileSystem.h @@ -347,12 +347,10 @@ public: ISO9660FS const& fs() const { return static_cast(Inode::fs()); } // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode&, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; @@ -367,6 +365,10 @@ private: // without any problems, so let's allow it anyway. static constexpr size_t max_file_identifier_length = 256 - sizeof(ISO::DirectoryRecordHeader); + // ^Inode + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + ISO9660Inode(ISO9660FS&, ISO::DirectoryRecordHeader const& record, StringView name); static ErrorOr> try_create_from_directory_record(ISO9660FS&, ISO::DirectoryRecordHeader const& record, StringView name); diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index faa5050f86..85ac2715d4 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -106,6 +106,19 @@ void Inode::will_be_destroyed() (void)flush_metadata(); } +ErrorOr Inode::write_bytes(off_t offset, size_t length, UserOrKernelBuffer const& target_buffer, OpenFileDescription* open_description) +{ + MutexLocker locker(m_inode_lock); + TRY(prepare_to_write_data()); + return write_bytes_locked(offset, length, target_buffer, open_description); +} + +ErrorOr Inode::read_bytes(off_t offset, size_t length, UserOrKernelBuffer& buffer, OpenFileDescription* open_description) const +{ + MutexLocker locker(m_inode_lock, Mutex::Mode::Shared); + return read_bytes_locked(offset, length, buffer, open_description); +} + ErrorOr Inode::update_timestamps([[maybe_unused]] Optional atime, [[maybe_unused]] Optional ctime, [[maybe_unused]] Optional mtime) { return ENOTIMPL; diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index a01df47246..a1016b8822 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -56,13 +56,14 @@ public: ErrorOr> read_entire(OpenFileDescription* = nullptr) const; + ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*); + ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const; + virtual ErrorOr attach(OpenFileDescription&) { return {}; } virtual void detach(OpenFileDescription&) { } virtual void did_seek(OpenFileDescription&, off_t) { } - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const = 0; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const = 0; virtual ErrorOr> lookup(StringView name) = 0; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) = 0; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) = 0; virtual ErrorOr add_child(Inode&, StringView name, mode_t) = 0; virtual ErrorOr remove_child(StringView name) = 0; @@ -119,6 +120,9 @@ protected: mutable Mutex m_inode_lock { "Inode"sv }; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) = 0; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const = 0; + private: ErrorOr try_apply_flock(Process const&, OpenFileDescription const&, flock const&); diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index a61f3c5675..b09e7df66f 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -42,12 +42,7 @@ ErrorOr InodeFile::write(OpenFileDescription& description, u64 offset, U if (Checked::addition_would_overflow(offset, count)) return EOVERFLOW; - size_t nwritten = 0; - { - MutexLocker locker(m_inode->m_inode_lock); - TRY(m_inode->prepare_to_write_data()); - nwritten = TRY(m_inode->write_bytes(offset, count, data, &description)); - } + size_t nwritten = TRY(m_inode->write_bytes(offset, count, data, &description)); if (nwritten > 0) { auto mtime_result = m_inode->update_timestamps({}, {}, kgettimeofday().to_truncated_seconds()); Thread::current()->did_file_write(nwritten); diff --git a/Kernel/FileSystem/Plan9FileSystem.cpp b/Kernel/FileSystem/Plan9FileSystem.cpp index 941440a1c6..882faaf35b 100644 --- a/Kernel/FileSystem/Plan9FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FileSystem.cpp @@ -718,7 +718,7 @@ ErrorOr Plan9FSInode::ensure_open_for_mode(int mode) return fs().post_message_and_wait_for_a_reply(message); } -ErrorOr Plan9FSInode::read_bytes(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const +ErrorOr Plan9FSInode::read_bytes_locked(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const { TRY(const_cast(*this).ensure_open_for_mode(O_RDONLY)); @@ -750,7 +750,7 @@ ErrorOr Plan9FSInode::read_bytes(off_t offset, size_t size, UserOrKernel return nread; } -ErrorOr Plan9FSInode::write_bytes(off_t offset, size_t size, UserOrKernelBuffer const& data, OpenFileDescription*) +ErrorOr Plan9FSInode::write_bytes_locked(off_t offset, size_t size, UserOrKernelBuffer const& data, OpenFileDescription*) { TRY(ensure_open_for_mode(O_WRONLY)); size = fs().adjust_buffer_size(size); diff --git a/Kernel/FileSystem/Plan9FileSystem.h b/Kernel/FileSystem/Plan9FileSystem.h index 3ee42f1c76..e5c59b90ce 100644 --- a/Kernel/FileSystem/Plan9FileSystem.h +++ b/Kernel/FileSystem/Plan9FileSystem.h @@ -156,8 +156,6 @@ public: // ^Inode virtual InodeMetadata metadata() const override; virtual ErrorOr flush_metadata() override; - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; @@ -168,6 +166,10 @@ public: virtual ErrorOr truncate(u64) override; private: + // ^Inode + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& data, OpenFileDescription*) override; + Plan9FSInode(Plan9FS&, u32 fid); static ErrorOr> try_create(Plan9FS&, u32 fid); diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 5de534fd49..214e7d00b5 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -120,7 +120,7 @@ ErrorOr ProcFSGlobalInode::attach(OpenFileDescription& description) return m_associated_component->refresh_data(description); } -ErrorOr ProcFSGlobalInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* fd) const +ErrorOr ProcFSGlobalInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* fd) const { return m_associated_component->read_bytes(offset, count, buffer, fd); } @@ -163,7 +163,7 @@ InodeMetadata ProcFSGlobalInode::metadata() const return metadata; } -ErrorOr ProcFSGlobalInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* fd) +ErrorOr ProcFSGlobalInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* fd) { return m_associated_component->write_bytes(offset, count, buffer, fd); } @@ -233,7 +233,7 @@ ProcFSProcessAssociatedInode::ProcFSProcessAssociatedInode(ProcFS const& fs, Pro { } -ErrorOr ProcFSProcessAssociatedInode::write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) +ErrorOr ProcFSProcessAssociatedInode::write_bytes_locked(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) { return ENOTSUP; } @@ -271,7 +271,7 @@ InodeMetadata ProcFSProcessDirectoryInode::metadata() const return metadata; } -ErrorOr ProcFSProcessDirectoryInode::read_bytes(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const +ErrorOr ProcFSProcessDirectoryInode::read_bytes_locked(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const { VERIFY_NOT_REACHED(); } @@ -327,7 +327,7 @@ ProcFSProcessSubDirectoryInode::ProcFSProcessSubDirectoryInode(ProcFS const& pro { } -ErrorOr ProcFSProcessSubDirectoryInode::read_bytes(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const +ErrorOr ProcFSProcessSubDirectoryInode::read_bytes_locked(off_t, size_t, UserOrKernelBuffer&, OpenFileDescription*) const { VERIFY_NOT_REACHED(); } @@ -490,9 +490,9 @@ ErrorOr ProcFSProcessPropertyInode::traverse_as_directory(Function ProcFSProcessPropertyInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const +ErrorOr ProcFSProcessPropertyInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* description) const { - dbgln_if(PROCFS_DEBUG, "ProcFS ProcessInformation: read_bytes offset: {} count: {}", offset, count); + dbgln_if(PROCFS_DEBUG, "ProcFS ProcessInformation: read_bytes_locked offset: {} count: {}", offset, count); VERIFY(offset >= 0); VERIFY(buffer.user_or_kernel_ptr()); diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index fe2f18532c..58e384709b 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -77,8 +77,8 @@ protected: // ^Inode virtual ErrorOr attach(OpenFileDescription& description) override final; - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override final; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override final; virtual void did_seek(OpenFileDescription&, off_t) override final; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; @@ -123,7 +123,7 @@ protected: ProcessID associated_pid() const { return m_pid; } // ^Inode - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override final; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override final; private: const ProcessID m_pid; @@ -142,7 +142,7 @@ private: virtual void did_seek(OpenFileDescription&, off_t) override { } virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; virtual ErrorOr> lookup(StringView name) override; }; @@ -159,7 +159,7 @@ private: virtual void did_seek(OpenFileDescription&, off_t) override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; virtual ErrorOr> lookup(StringView name) override; const SegmentedProcFSIndex::ProcessSubDirectory m_sub_directory_type; @@ -185,7 +185,7 @@ private: virtual void did_seek(OpenFileDescription&, off_t) override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override final; virtual ErrorOr> lookup(StringView name) override final; ErrorOr refresh_data(OpenFileDescription& description); diff --git a/Kernel/FileSystem/SysFS.cpp b/Kernel/FileSystem/SysFS.cpp index 8da22c5afd..b679d885d3 100644 --- a/Kernel/FileSystem/SysFS.cpp +++ b/Kernel/FileSystem/SysFS.cpp @@ -58,7 +58,7 @@ ErrorOr SysFSInode::attach(OpenFileDescription& description) return m_associated_component->refresh_data(description); } -ErrorOr SysFSInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* fd) const +ErrorOr SysFSInode::read_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer& buffer, OpenFileDescription* fd) const { return m_associated_component->read_bytes(offset, count, buffer, fd); } @@ -91,7 +91,7 @@ ErrorOr SysFSInode::flush_metadata() return {}; } -ErrorOr SysFSInode::write_bytes(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* fd) +ErrorOr SysFSInode::write_bytes_locked(off_t offset, size_t count, UserOrKernelBuffer const& buffer, OpenFileDescription* fd) { return m_associated_component->write_bytes(offset, count, buffer, fd); } diff --git a/Kernel/FileSystem/SysFS.h b/Kernel/FileSystem/SysFS.h index 8729b905a2..a3b274cfa4 100644 --- a/Kernel/FileSystem/SysFS.h +++ b/Kernel/FileSystem/SysFS.h @@ -42,12 +42,12 @@ public: protected: SysFSInode(SysFS const&, SysFSComponent const&); - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; virtual InodeMetadata metadata() const override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const&, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode&, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp index 9dc3bf211b..738a3e406f 100644 --- a/Kernel/FileSystem/TmpFS.cpp +++ b/Kernel/FileSystem/TmpFS.cpp @@ -87,9 +87,9 @@ ErrorOr TmpFSInode::traverse_as_directory(Function(FileSyste return {}; } -ErrorOr TmpFSInode::read_bytes(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const +ErrorOr TmpFSInode::read_bytes_locked(off_t offset, size_t size, UserOrKernelBuffer& buffer, OpenFileDescription*) const { - MutexLocker locker(m_inode_lock, Mutex::Mode::Shared); + VERIFY(m_inode_lock.is_locked()); VERIFY(!is_directory()); VERIFY(offset >= 0); @@ -106,7 +106,7 @@ ErrorOr TmpFSInode::read_bytes(off_t offset, size_t size, UserOrKernelBu return size; } -ErrorOr TmpFSInode::write_bytes(off_t offset, size_t size, UserOrKernelBuffer const& buffer, OpenFileDescription*) +ErrorOr TmpFSInode::write_bytes_locked(off_t offset, size_t size, UserOrKernelBuffer const& buffer, OpenFileDescription*) { VERIFY(m_inode_lock.is_locked()); VERIFY(!is_directory()); diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h index 92cf714617..7c3cc8e8a5 100644 --- a/Kernel/FileSystem/TmpFS.h +++ b/Kernel/FileSystem/TmpFS.h @@ -47,12 +47,10 @@ public: TmpFS const& fs() const { return static_cast(Inode::fs()); } // ^Inode - virtual ErrorOr read_bytes(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; virtual InodeMetadata metadata() const override; virtual ErrorOr traverse_as_directory(Function(FileSystem::DirectoryEntryView const&)>) const override; virtual ErrorOr> lookup(StringView name) override; virtual ErrorOr flush_metadata() override; - virtual ErrorOr write_bytes(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; virtual ErrorOr> create_child(StringView name, mode_t, dev_t, UserID, GroupID) override; virtual ErrorOr add_child(Inode&, StringView name, mode_t) override; virtual ErrorOr remove_child(StringView name) override; @@ -66,6 +64,10 @@ private: static ErrorOr> try_create(TmpFS&, InodeMetadata const& metadata, LockWeakPtr parent); static ErrorOr> try_create_root(TmpFS&); + // ^Inode + virtual ErrorOr read_bytes_locked(off_t, size_t, UserOrKernelBuffer& buffer, OpenFileDescription*) const override; + virtual ErrorOr write_bytes_locked(off_t, size_t, UserOrKernelBuffer const& buffer, OpenFileDescription*) override; + struct Child { NonnullOwnPtr name; NonnullLockRefPtr inode; diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index b10645d74c..1a120a9d1f 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -703,8 +703,6 @@ ErrorOr VirtualFileSystem::symlink(Credentials const& credentials, StringV auto inode = TRY(parent_inode.create_child(basename, S_IFLNK | 0644, 0, credentials.euid(), credentials.egid())); auto target_buffer = UserOrKernelBuffer::for_kernel_buffer(const_cast((u8 const*)target.characters_without_null_termination())); - MutexLocker locker(inode->m_inode_lock); - TRY(inode->prepare_to_write_data()); TRY(inode->write_bytes(0, target.length(), target_buffer, nullptr)); return {}; }