From 79e22acb225d8aaa4d20540afa50faac17ea29bf Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 5 Aug 2019 11:35:49 +0200 Subject: [PATCH] Kernel: Use KBuffers for ProcFS and SynthFS Instead of generating ByteBuffers and keeping those lying around, have these filesystems generate KBuffers instead. These are way less spooky to leave around for a while. Since FileDescription will keep a generated file buffer around until userspace has read the whole thing, this prevents trivially exhausting the kmalloc heap by opening many files in /proc for example. The code responsible for generating each /proc file is not perfectly efficient and many of them still use ByteBuffers internally but they at least go away when we return now. :^) --- Kernel/FileSystem/FileDescription.h | 5 +- Kernel/FileSystem/ProcFS.cpp | 56 +++++++++++------------ Kernel/FileSystem/ProcFS.h | 5 +- Kernel/FileSystem/SyntheticFileSystem.cpp | 14 ++++-- Kernel/FileSystem/SyntheticFileSystem.h | 5 +- Kernel/KBuffer.h | 9 +++- Kernel/VM/MemoryManager.h | 7 +-- 7 files changed, 58 insertions(+), 43 deletions(-) diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h index ebf9e44e8f..80cf993f2b 100644 --- a/Kernel/FileSystem/FileDescription.h +++ b/Kernel/FileSystem/FileDescription.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -90,7 +91,7 @@ public: SharedMemory* shared_memory(); const SharedMemory* shared_memory() const; - ByteBuffer& generator_cache() { return m_generator_cache; } + Optional& generator_cache() { return m_generator_cache; } void set_original_inode(Badge, NonnullRefPtr&& inode) { m_inode = move(inode); } @@ -114,7 +115,7 @@ private: off_t m_current_offset { 0 }; - ByteBuffer m_generator_cache; + Optional m_generator_cache; u32 m_file_flags { 0 }; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 84131b5786..3398921ccf 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -185,7 +185,7 @@ ProcFS::~ProcFS() { } -ByteBuffer procfs$pid_fds(InodeIdentifier identifier) +Optional procfs$pid_fds(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -209,7 +209,7 @@ ByteBuffer procfs$pid_fds(InodeIdentifier identifier) return array.serialized().to_byte_buffer(); } -ByteBuffer procfs$pid_fd_entry(InodeIdentifier identifier) +Optional procfs$pid_fd_entry(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -222,7 +222,7 @@ ByteBuffer procfs$pid_fd_entry(InodeIdentifier identifier) return description->absolute_path().to_byte_buffer(); } -ByteBuffer procfs$pid_vm(InodeIdentifier identifier) +Optional procfs$pid_vm(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -242,7 +242,7 @@ ByteBuffer procfs$pid_vm(InodeIdentifier identifier) return array.serialized().to_byte_buffer(); } -ByteBuffer procfs$pci(InodeIdentifier) +Optional procfs$pci(InodeIdentifier) { StringBuilder builder; PCI::enumerate_all([&builder](PCI::Address address, PCI::ID id) { @@ -251,21 +251,21 @@ ByteBuffer procfs$pci(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$uptime(InodeIdentifier) +Optional procfs$uptime(InodeIdentifier) { StringBuilder builder; builder.appendf("%u\n", (u32)(g_uptime / 1000)); return builder.to_byte_buffer(); } -ByteBuffer procfs$cmdline(InodeIdentifier) +Optional procfs$cmdline(InodeIdentifier) { StringBuilder builder; builder.appendf("%s\n", KParams::the().cmdline().characters()); return builder.to_byte_buffer(); } -ByteBuffer procfs$netadapters(InodeIdentifier) +Optional procfs$netadapters(InodeIdentifier) { StringBuilder builder; NetworkAdapter::for_each([&builder](auto& adapter) { @@ -278,7 +278,7 @@ ByteBuffer procfs$netadapters(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$pid_vmo(InodeIdentifier identifier) +Optional procfs$pid_vmo(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -309,7 +309,7 @@ ByteBuffer procfs$pid_vmo(InodeIdentifier identifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$pid_stack(InodeIdentifier identifier) +Optional procfs$pid_stack(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -318,7 +318,7 @@ ByteBuffer procfs$pid_stack(InodeIdentifier identifier) return process.backtrace(*handle).to_byte_buffer(); } -ByteBuffer procfs$pid_regs(InodeIdentifier identifier) +Optional procfs$pid_regs(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -344,7 +344,7 @@ ByteBuffer procfs$pid_regs(InodeIdentifier identifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$pid_exe(InodeIdentifier identifier) +Optional procfs$pid_exe(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -355,7 +355,7 @@ ByteBuffer procfs$pid_exe(InodeIdentifier identifier) return custody->absolute_path().to_byte_buffer(); } -ByteBuffer procfs$pid_cwd(InodeIdentifier identifier) +Optional procfs$pid_cwd(InodeIdentifier identifier) { auto handle = ProcessInspectionHandle::from_pid(to_pid(identifier)); if (!handle) @@ -363,14 +363,14 @@ ByteBuffer procfs$pid_cwd(InodeIdentifier identifier) return handle->process().current_directory().absolute_path().to_byte_buffer(); } -ByteBuffer procfs$self(InodeIdentifier) +Optional procfs$self(InodeIdentifier) { char buffer[16]; ksprintf(buffer, "%u", current->pid()); - return ByteBuffer::copy((const u8*)buffer, strlen(buffer)); + return KBuffer::copy((const u8*)buffer, strlen(buffer)); } -ByteBuffer procfs$mm(InodeIdentifier) +Optional procfs$mm(InodeIdentifier) { // FIXME: Implement InterruptDisabler disabler; @@ -389,7 +389,7 @@ ByteBuffer procfs$mm(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$dmesg(InodeIdentifier) +Optional procfs$dmesg(InodeIdentifier) { InterruptDisabler disabler; StringBuilder builder; @@ -398,7 +398,7 @@ ByteBuffer procfs$dmesg(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$mounts(InodeIdentifier) +Optional procfs$mounts(InodeIdentifier) { // FIXME: This is obviously racy against the VFS mounts changing. StringBuilder builder; @@ -417,7 +417,7 @@ ByteBuffer procfs$mounts(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$df(InodeIdentifier) +Optional procfs$df(InodeIdentifier) { // FIXME: This is obviously racy against the VFS mounts changing. JsonArray json; @@ -435,7 +435,7 @@ ByteBuffer procfs$df(InodeIdentifier) return json.serialized().to_byte_buffer(); } -ByteBuffer procfs$cpuinfo(InodeIdentifier) +Optional procfs$cpuinfo(InodeIdentifier) { StringBuilder builder; { @@ -498,7 +498,7 @@ ByteBuffer procfs$cpuinfo(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$kmalloc(InodeIdentifier) +Optional procfs$kmalloc(InodeIdentifier) { StringBuilder builder; builder.appendf( @@ -511,7 +511,7 @@ ByteBuffer procfs$kmalloc(InodeIdentifier) return builder.to_byte_buffer(); } -ByteBuffer procfs$memstat(InodeIdentifier) +Optional procfs$memstat(InodeIdentifier) { InterruptDisabler disabler; JsonObject json; @@ -527,7 +527,7 @@ ByteBuffer procfs$memstat(InodeIdentifier) return json.serialized().to_byte_buffer(); } -ByteBuffer procfs$all(InodeIdentifier) +Optional procfs$all(InodeIdentifier) { InterruptDisabler disabler; auto processes = Process::all_processes(); @@ -563,7 +563,7 @@ ByteBuffer procfs$all(InodeIdentifier) return array.serialized().to_byte_buffer(); } -ByteBuffer procfs$inodes(InodeIdentifier) +Optional procfs$inodes(InodeIdentifier) { extern HashTable& all_inodes(); StringBuilder builder; @@ -819,8 +819,8 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, u8* buffer, FileDes auto* directory_entry = fs().get_directory_entry(identifier()); - Function callback_tmp; - Function* read_callback { nullptr }; + Function(InodeIdentifier)> callback_tmp; + Function(InodeIdentifier)>* read_callback { nullptr }; if (directory_entry) { read_callback = &directory_entry->read_callback; } else { @@ -832,7 +832,7 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, u8* buffer, FileDes ASSERT(read_callback); - ByteBuffer generated_data; + Optional generated_data; if (!description) { generated_data = (*read_callback)(identifier()); } else { @@ -842,8 +842,8 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, u8* buffer, FileDes } auto& data = generated_data; - ssize_t nread = min(static_cast(data.size() - offset), static_cast(count)); - memcpy(buffer, data.pointer() + offset, nread); + ssize_t nread = min(static_cast(data.value().size() - offset), static_cast(count)); + memcpy(buffer, data.value().data() + offset, nread); if (nread == 0 && description && description->generator_cache()) description->generator_cache().clear(); return nread; diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index 14019b4fd5..dc54cc65b5 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -3,6 +3,7 @@ #include #include #include +#include #include class Process; @@ -36,7 +37,7 @@ private: struct ProcFSDirectoryEntry { ProcFSDirectoryEntry() {} - ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, Function&& a_read_callback = nullptr, Function&& a_write_callback = nullptr, RefPtr&& a_inode = nullptr) + ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, Function(InodeIdentifier)>&& a_read_callback = nullptr, Function&& a_write_callback = nullptr, RefPtr&& a_inode = nullptr) : name(a_name) , proc_file_type(a_proc_file_type) , read_callback(move(a_read_callback)) @@ -47,7 +48,7 @@ private: const char* name { nullptr }; unsigned proc_file_type { 0 }; - Function read_callback; + Function(InodeIdentifier)> read_callback; Function write_callback; RefPtr inode; InodeIdentifier identifier(unsigned fsid) const; diff --git a/Kernel/FileSystem/SyntheticFileSystem.cpp b/Kernel/FileSystem/SyntheticFileSystem.cpp index 3bc66be0ee..a1f660c6a9 100644 --- a/Kernel/FileSystem/SyntheticFileSystem.cpp +++ b/Kernel/FileSystem/SyntheticFileSystem.cpp @@ -50,7 +50,7 @@ NonnullRefPtr SynthFS::create_text_file(String&& name, ByteBuffer& auto file = adopt(*new SynthFSInode(*this, generate_inode_index())); file->m_data = contents; file->m_name = move(name); - file->m_metadata.size = file->m_data.size(); + file->m_metadata.size = file->m_data.value().size(); file->m_metadata.uid = 100; file->m_metadata.gid = 200; file->m_metadata.mode = mode; @@ -194,7 +194,7 @@ ssize_t SynthFSInode::read_bytes(off_t offset, ssize_t count, u8* buffer, FileDe ASSERT(offset >= 0); ASSERT(buffer); - ByteBuffer generated_data; + Optional generated_data; if (m_generator) { if (!description) { generated_data = m_generator(const_cast(*this)); @@ -205,9 +205,15 @@ ssize_t SynthFSInode::read_bytes(off_t offset, ssize_t count, u8* buffer, FileDe } } - auto* data = generated_data ? &generated_data : &m_data; + const KBuffer* data_to_use = nullptr; + if (generated_data.has_value()) + data_to_use = &generated_data.value(); + else if (m_data.has_value()) + data_to_use = &m_data.value(); + else + ASSERT_NOT_REACHED(); ssize_t nread = min(static_cast(data->size() - offset), static_cast(count)); - memcpy(buffer, data->pointer() + offset, nread); + memcpy(buffer, data->data() + offset, nread); if (nread == 0 && description && description->generator_cache()) description->generator_cache().clear(); return nread; diff --git a/Kernel/FileSystem/SyntheticFileSystem.h b/Kernel/FileSystem/SyntheticFileSystem.h index 8582223995..92d8e64115 100644 --- a/Kernel/FileSystem/SyntheticFileSystem.h +++ b/Kernel/FileSystem/SyntheticFileSystem.h @@ -3,6 +3,7 @@ #include #include #include +#include #include class SynthFSInode; @@ -75,8 +76,8 @@ private: String m_name; InodeIdentifier m_parent; - ByteBuffer m_data; - Function m_generator; + Optional m_data; + Function m_generator; Function m_write_callback; Vector m_children; InodeMetadata m_metadata; diff --git a/Kernel/KBuffer.h b/Kernel/KBuffer.h index 4d718d6ebe..71f71bbb66 100644 --- a/Kernel/KBuffer.h +++ b/Kernel/KBuffer.h @@ -65,11 +65,16 @@ public: const KBufferImpl& impl() const { return m_impl; } - KBuffer(NonnullRefPtr&& impl) - : m_impl(move(impl)) + KBuffer(const ByteBuffer& buffer) + : m_impl(KBufferImpl::copy(buffer.data(), buffer.size())) { } private: + explicit KBuffer(NonnullRefPtr&& impl) + : m_impl(move(impl)) + { + } + NonnullRefPtr m_impl; }; diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h index da714f5b76..9e9d6e1230 100644 --- a/Kernel/VM/MemoryManager.h +++ b/Kernel/VM/MemoryManager.h @@ -6,8 +6,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -20,6 +20,7 @@ #define PAGE_ROUND_UP(x) ((((u32)(x)) + PAGE_SIZE - 1) & (~(PAGE_SIZE - 1))) +class KBuffer; class SynthFSInode; enum class PageFaultResponse { @@ -36,8 +37,8 @@ class MemoryManager { friend class PhysicalRegion; friend class Region; friend class VMObject; - friend ByteBuffer procfs$mm(InodeIdentifier); - friend ByteBuffer procfs$memstat(InodeIdentifier); + friend Optional procfs$mm(InodeIdentifier); + friend Optional procfs$memstat(InodeIdentifier); public: static MemoryManager& the();