From e490c17bdedf48ea252a1f9fe04b748e63cbd9b7 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 4 Sep 2021 16:23:45 +0300 Subject: [PATCH] Kernel/SysFS: Ensure data stability when reading from Inodes Like with the ProcFS, description data can change at anytime, so it's wise to ensure that when the userland reads from an Inode, data is consistent unless the userland indicated it wants to refresh the data (by seeking to offset 0, or re-attaching the Inode). Otherwise, if the data changes in the middle of the reading, it can cause silent corruption in output which can lead to random crashes. --- Kernel/Bus/USB/SysFSUSB.cpp | 54 ++++++++++++++++++++++++++---- Kernel/Bus/USB/SysFSUSB.h | 7 ++++ Kernel/FileSystem/SysFS.cpp | 16 +++++++++ Kernel/FileSystem/SysFS.h | 3 ++ Kernel/FileSystem/SysFSComponent.h | 6 ++++ 5 files changed, 79 insertions(+), 7 deletions(-) diff --git a/Kernel/Bus/USB/SysFSUSB.cpp b/Kernel/Bus/USB/SysFSUSB.cpp index 6c629194af..4448f3b48e 100644 --- a/Kernel/Bus/USB/SysFSUSB.cpp +++ b/Kernel/Bus/USB/SysFSUSB.cpp @@ -23,9 +23,9 @@ SysFSUSBDeviceInformation::~SysFSUSBDeviceInformation() { } -KResultOr SysFSUSBDeviceInformation::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription*) const +bool SysFSUSBDeviceInformation::output(KBufferBuilder& builder) { - KBufferBuilder builder; + VERIFY(m_lock.is_locked()); JsonArraySerializer array { builder }; auto obj = array.add_object(); @@ -44,14 +44,54 @@ KResultOr SysFSUSBDeviceInformation::read_bytes(off_t offset, size_t cou obj.add("num_configurations", m_device->device_descriptor().num_configurations); obj.finish(); array.finish(); + return true; +} - auto data = builder.build(); - if (!data) +KResult SysFSUSBDeviceInformation::refresh_data(FileDescription& description) const +{ + MutexLocker lock(m_lock); + auto& cached_data = description.data(); + if (!cached_data) { + cached_data = adopt_own_if_nonnull(new (nothrow) SysFSInodeData); + if (!cached_data) + return ENOMEM; + } + KBufferBuilder builder; + if (!const_cast(*this).output(builder)) + return ENOENT; + auto& typed_cached_data = static_cast(*cached_data); + typed_cached_data.buffer = builder.build(); + if (!typed_cached_data.buffer) return ENOMEM; + return KSuccess; +} - ssize_t nread = min(static_cast(data->size() - offset), static_cast(count)); - if (!buffer.write(data->data() + offset, nread)) - return EFAULT; +KResultOr SysFSUSBDeviceInformation::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription* description) const +{ + dbgln_if(PROCFS_DEBUG, "SysFSUSBDeviceInformation @ {}: read_bytes offset: {} count: {}", name(), offset, count); + + VERIFY(offset >= 0); + VERIFY(buffer.user_or_kernel_ptr()); + + if (!description) + return KResult(EIO); + + MutexLocker locker(m_lock); + + if (!description->data()) { + dbgln("SysFSUSBDeviceInformation: Do not have cached data!"); + return KResult(EIO); + } + + auto& typed_cached_data = static_cast(*description->data()); + auto& data_buffer = typed_cached_data.buffer; + + if (!data_buffer || (size_t)offset >= data_buffer->size()) + return 0; + + ssize_t nread = min(static_cast(data_buffer->size() - offset), static_cast(count)); + if (!buffer.write(data_buffer->data() + offset, nread)) + return KResult(EFAULT); return nread; } diff --git a/Kernel/Bus/USB/SysFSUSB.h b/Kernel/Bus/USB/SysFSUSB.h index 5986fac4af..ff8cf9a76a 100644 --- a/Kernel/Bus/USB/SysFSUSB.h +++ b/Kernel/Bus/USB/SysFSUSB.h @@ -8,6 +8,8 @@ #include #include +#include +#include namespace Kernel::USB { @@ -29,6 +31,11 @@ protected: IntrusiveListNode> m_list_node; NonnullRefPtr m_device; + +private: + bool output(KBufferBuilder& builder); + virtual KResult refresh_data(FileDescription& description) const override; + mutable Mutex m_lock { "SysFSUSBDeviceInformation" }; }; class SysFSUSBBusDirectory final : public SysFSDirectory { diff --git a/Kernel/FileSystem/SysFS.cpp b/Kernel/FileSystem/SysFS.cpp index db6f3e26cd..a7c769aca0 100644 --- a/Kernel/FileSystem/SysFS.cpp +++ b/Kernel/FileSystem/SysFS.cpp @@ -96,6 +96,22 @@ SysFSInode::SysFSInode(SysFS const& fs, SysFSComponent const& component) { } +void SysFSInode::did_seek(FileDescription& description, off_t new_offset) +{ + if (new_offset != 0) + return; + auto result = m_associated_component->refresh_data(description); + if (result.is_error()) { + // Subsequent calls to read will return EIO! + dbgln("SysFS: Could not refresh contents: {}", result.error()); + } +} + +KResult SysFSInode::attach(FileDescription& description) +{ + return m_associated_component->refresh_data(description); +} + KResultOr SysFSInode::read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription* fd) const { return m_associated_component->read_bytes(offset, count, buffer, fd); diff --git a/Kernel/FileSystem/SysFS.h b/Kernel/FileSystem/SysFS.h index 32c441df5a..e6836343be 100644 --- a/Kernel/FileSystem/SysFS.h +++ b/Kernel/FileSystem/SysFS.h @@ -96,6 +96,9 @@ protected: virtual KResult chown(UserID, GroupID) override; virtual KResult truncate(u64) override; + virtual KResult attach(FileDescription& description) override final; + virtual void did_seek(FileDescription&, off_t) override final; + NonnullRefPtr m_associated_component; }; diff --git a/Kernel/FileSystem/SysFSComponent.h b/Kernel/FileSystem/SysFSComponent.h index cefa342650..36ced1874e 100644 --- a/Kernel/FileSystem/SysFSComponent.h +++ b/Kernel/FileSystem/SysFSComponent.h @@ -12,12 +12,17 @@ #include #include #include +#include #include #include #include namespace Kernel { +struct SysFSInodeData : public FileDescriptionData { + OwnPtr buffer; +}; + class SysFSComponent : public RefCounted { public: virtual StringView name() const { return m_name->view(); } @@ -25,6 +30,7 @@ public: virtual KResult traverse_as_directory(unsigned, Function) const { VERIFY_NOT_REACHED(); } virtual RefPtr lookup(StringView) { VERIFY_NOT_REACHED(); }; virtual KResultOr write_bytes(off_t, size_t, UserOrKernelBuffer const&, FileDescription*) { return EROFS; } + virtual KResult refresh_data(FileDescription&) const { return KSuccess; } virtual NonnullRefPtr to_inode(SysFS const&) const;