From f98ca35b83d521377d4bce5927fdcb4a764ac02e Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 17 Sep 2020 13:51:09 -0600 Subject: [PATCH] Kernel: Improve ProcFS behavior in low memory conditions When ProcFS could no longer allocate KBuffer objects to serve calls to read, it would just return 0, indicating EOF. This then triggered parsing errors because code assumed it read the file. Because read isn't supposed to return ENOMEM, change ProcFS to populate the file data upon file open or seek to the beginning. This also means that calls to open can now return ENOMEM if needed. This allows the caller to either be able to successfully open the file and read it, or fail to open it in the first place. --- Kernel/FileSystem/FIFO.cpp | 12 +- Kernel/FileSystem/FIFO.h | 4 +- Kernel/FileSystem/File.cpp | 6 +- Kernel/FileSystem/File.h | 3 + Kernel/FileSystem/FileDescription.cpp | 48 +++- Kernel/FileSystem/FileDescription.h | 16 +- Kernel/FileSystem/Inode.h | 3 + Kernel/FileSystem/ProcFS.cpp | 345 +++++++++++++----------- Kernel/FileSystem/ProcFS.h | 12 +- Kernel/FileSystem/VirtualFileSystem.cpp | 22 +- Kernel/KBuffer.h | 34 ++- Kernel/KBufferBuilder.cpp | 55 +++- Kernel/KBufferBuilder.h | 10 +- Kernel/Net/IPv4Socket.cpp | 8 - Kernel/Net/IPv4Socket.h | 2 - Kernel/Net/LocalSocket.cpp | 3 +- Kernel/Net/LocalSocket.h | 2 +- Kernel/Net/Socket.h | 2 - Kernel/Storage/StorageManagement.cpp | 5 +- Kernel/Syscalls/pipe.cpp | 11 +- Kernel/Syscalls/socket.cpp | 25 +- Kernel/Syscalls/watch_file.cpp | 6 +- Kernel/TTY/PTYMultiplexer.cpp | 6 +- Meta/CMake/all_the_debug_macros.cmake | 1 + 24 files changed, 398 insertions(+), 243 deletions(-) diff --git a/Kernel/FileSystem/FIFO.cpp b/Kernel/FileSystem/FIFO.cpp index fe0d8d6cd36..b7ccd05ec4a 100644 --- a/Kernel/FileSystem/FIFO.cpp +++ b/Kernel/FileSystem/FIFO.cpp @@ -52,19 +52,23 @@ NonnullRefPtr FIFO::create(uid_t uid) return adopt(*new FIFO(uid)); } -NonnullRefPtr FIFO::open_direction(FIFO::Direction direction) +KResultOr> FIFO::open_direction(FIFO::Direction direction) { auto description = FileDescription::create(*this); - attach(direction); - description->set_fifo_direction({}, direction); + if (!description.is_error()) { + attach(direction); + description.value()->set_fifo_direction({}, direction); + } return description; } -NonnullRefPtr FIFO::open_direction_blocking(FIFO::Direction direction) +KResultOr> FIFO::open_direction_blocking(FIFO::Direction direction) { Locker locker(m_open_lock); auto description = open_direction(direction); + if (description.is_error()) + return description; if (direction == Direction::Reader) { m_read_open_queue.wake_all(); diff --git a/Kernel/FileSystem/FIFO.h b/Kernel/FileSystem/FIFO.h index 8e3a1460123..7ac6012229e 100644 --- a/Kernel/FileSystem/FIFO.h +++ b/Kernel/FileSystem/FIFO.h @@ -49,8 +49,8 @@ public: uid_t uid() const { return m_uid; } - NonnullRefPtr open_direction(Direction); - NonnullRefPtr open_direction_blocking(Direction); + KResultOr> open_direction(Direction); + KResultOr> open_direction_blocking(Direction); void attach(Direction); void detach(Direction); diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp index 6a192626db8..5b20d339fa6 100644 --- a/Kernel/FileSystem/File.cpp +++ b/Kernel/FileSystem/File.cpp @@ -42,8 +42,10 @@ File::~File() KResultOr> File::open(int options) { auto description = FileDescription::create(*this); - description->set_rw_mode(options); - description->set_file_flags(options); + if (!description.is_error()) { + description.value()->set_rw_mode(options); + description.value()->set_file_flags(options); + } return description; } diff --git a/Kernel/FileSystem/File.h b/Kernel/FileSystem/File.h index a7717db707c..f00f5b8f3e0 100644 --- a/Kernel/FileSystem/File.h +++ b/Kernel/FileSystem/File.h @@ -108,6 +108,9 @@ public: virtual bool can_read(const FileDescription&, size_t) const = 0; virtual bool can_write(const FileDescription&, size_t) const = 0; + virtual KResult attach(FileDescription&) { return KSuccess; } + virtual void detach(FileDescription&) { } + virtual void did_seek(FileDescription&, off_t) { } virtual KResultOr read(FileDescription&, size_t, UserOrKernelBuffer&, size_t) = 0; virtual KResultOr write(FileDescription&, size_t, const UserOrKernelBuffer&, size_t) = 0; virtual int ioctl(FileDescription&, unsigned request, FlatPtr arg); diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index c4ec739f6a1..ac67074c091 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -40,18 +40,35 @@ #include #include +//#define FILEDESCRIPTION_DEBUG + namespace Kernel { -NonnullRefPtr FileDescription::create(Custody& custody) +KResultOr> FileDescription::create(Custody& custody) { auto description = adopt(*new FileDescription(InodeFile::create(custody.inode()))); description->m_custody = custody; + auto result = description->attach(); + if (result.is_error()) { +#ifdef FILEDESCRIPTION_DEBUG + dbg() << "Failed to create file description for custody: " << result; +#endif + return result; + } return description; } -NonnullRefPtr FileDescription::create(File& file) +KResultOr> FileDescription::create(File& file) { - return adopt(*new FileDescription(file)); + auto description = adopt(*new FileDescription(file)); + auto result = description->attach(); + if (result.is_error()) { +#ifdef FILEDESCRIPTION_DEBUG + dbg() << "Failed to create file description for file: " << result; +#endif + return result; + } + return description; } FileDescription::FileDescription(File& file) @@ -59,20 +76,29 @@ FileDescription::FileDescription(File& file) { if (file.is_inode()) m_inode = static_cast(file).inode(); - if (is_socket()) - socket()->attach(*this); + m_is_directory = metadata().is_directory(); } FileDescription::~FileDescription() { - if (is_socket()) - socket()->detach(*this); + m_file->detach(*this); if (is_fifo()) static_cast(m_file.ptr())->detach(m_fifo_direction); // FIXME: Should this error path be observed somehow? - [[maybe_unused]] auto rc = m_file->close(); - m_inode = nullptr; + (void)m_file->close(); + if (m_inode) + m_inode->detach(*this); +} + +KResult FileDescription::attach() +{ + if (m_inode) { + auto result = m_inode->attach(*this); + if (result.is_error()) + return result; + } + return m_file->attach(*this); } Thread::FileBlocker::BlockFlags FileDescription::should_unblock(Thread::FileBlocker::BlockFlags block_flags) const @@ -133,6 +159,10 @@ off_t FileDescription::seek(off_t offset, int whence) // FIXME: Return -EINVAL if attempting to seek past the end of a seekable device. m_current_offset = new_offset; + + m_file->did_seek(*this, new_offset); + if (m_inode) + m_inode->did_seek(*this, new_offset); evaluate_block_conditions(); return m_current_offset; } diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h index d4e9cb2d1db..16bde5890c5 100644 --- a/Kernel/FileSystem/FileDescription.h +++ b/Kernel/FileSystem/FileDescription.h @@ -38,11 +38,16 @@ namespace Kernel { +class FileDescriptionData { +public: + virtual ~FileDescriptionData() { } +}; + class FileDescription : public RefCounted { MAKE_SLAB_ALLOCATED(FileDescription) public: - static NonnullRefPtr create(Custody&); - static NonnullRefPtr create(File&); + static KResultOr> create(Custody&); + static KResultOr> create(File&); ~FileDescription(); Thread::FileBlocker::BlockFlags should_unblock(Thread::FileBlocker::BlockFlags) const; @@ -122,7 +127,7 @@ public: FIFO::Direction fifo_direction() { return m_fifo_direction; } void set_fifo_direction(Badge, FIFO::Direction direction) { m_fifo_direction = direction; } - OwnPtr& generator_cache() { return m_generator_cache; } + OwnPtr& data() { return m_data; } void set_original_inode(Badge, NonnullRefPtr&& inode) { m_inode = move(inode); } @@ -137,7 +142,8 @@ public: private: friend class VFS; explicit FileDescription(File&); - FileDescription(FIFO&, FIFO::Direction); + + KResult attach(); void evaluate_block_conditions() { @@ -150,7 +156,7 @@ private: off_t m_current_offset { 0 }; - OwnPtr m_generator_cache; + OwnPtr m_data; u32 m_file_flags { 0 }; diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 399fb9ecef0..61339d7107d 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -69,6 +69,9 @@ public: KResultOr> read_entire(FileDescription* = nullptr) const; + virtual KResult attach(FileDescription&) { return KSuccess; } + virtual void detach(FileDescription&) { } + virtual void did_seek(FileDescription&, off_t) { } virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const = 0; virtual KResult traverse_as_directory(Function) const = 0; virtual RefPtr lookup(StringView name) = 0; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 4c457c3a219..1f1e6488118 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -253,6 +253,10 @@ static inline bool is_persistent_inode(const InodeIdentifier& identifier) return to_proc_parent_directory(identifier) == PDI_Root_sys; } +struct ProcFSInodeData : public FileDescriptionData { + RefPtr buffer; +}; + NonnullRefPtr ProcFS::create() { return adopt(*new ProcFS); @@ -262,19 +266,18 @@ ProcFS::~ProcFS() { } -static OwnPtr procfs$pid_fds(InodeIdentifier identifier) +static bool procfs$pid_fds(InodeIdentifier identifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; auto process = Process::from_pid(to_pid(identifier)); if (!process) { array.finish(); - return builder.build(); + return true; } if (process->number_of_open_file_descriptors() == 0) { array.finish(); - return builder.build(); + return true; } for (int i = 0; i < process->max_open_file_descriptors(); ++i) { @@ -295,27 +298,27 @@ static OwnPtr procfs$pid_fds(InodeIdentifier identifier) description_object.add("can_write", description->can_write()); } array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$pid_fd_entry(InodeIdentifier identifier) +static bool procfs$pid_fd_entry(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; + return false; int fd = to_fd(identifier); auto description = process->file_description(fd); if (!description) - return {}; - return KBuffer::try_create_with_bytes(description->absolute_path().bytes()); + return false; + builder.append_bytes(description->absolute_path().bytes()); + return true; } -static OwnPtr procfs$pid_vm(InodeIdentifier identifier) +static bool procfs$pid_vm(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; - KBufferBuilder builder; + return false; JsonArraySerializer array { builder }; { ScopedSpinLock lock(process->get_lock()); @@ -357,12 +360,11 @@ static OwnPtr procfs$pid_vm(InodeIdentifier identifier) } } array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$pci(InodeIdentifier) +static bool procfs$pci(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; PCI::enumerate([&array](PCI::Address address, PCI::ID id) { auto obj = array.add_object(); @@ -379,12 +381,11 @@ static OwnPtr procfs$pci(InodeIdentifier) obj.add("subsystem_vendor_id", PCI::get_subsystem_vendor_id(address)); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$interrupts(InodeIdentifier) +static bool procfs$interrupts(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; InterruptManagement::the().enumerate_interrupt_handlers([&array](GenericInterruptHandler& handler) { auto obj = array.add_object(); @@ -396,21 +397,19 @@ static OwnPtr procfs$interrupts(InodeIdentifier) obj.add("call_count", (unsigned)handler.get_invoking_count()); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$keymap(InodeIdentifier) +static bool procfs$keymap(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonObjectSerializer json { builder }; json.add("keymap", KeyboardDevice::the().keymap_name()); json.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$devices(InodeIdentifier) +static bool procfs$devices(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; Device::for_each([&array](auto& device) { auto obj = array.add_object(); @@ -426,28 +425,25 @@ static OwnPtr procfs$devices(InodeIdentifier) ASSERT_NOT_REACHED(); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$uptime(InodeIdentifier) +static bool procfs$uptime(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; builder.appendf("%llu\n", TimeManagement::the().uptime_ms() / 1000); - return builder.build(); + return true; } -static OwnPtr procfs$cmdline(InodeIdentifier) +static bool procfs$cmdline(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; builder.append(kernel_command_line().string()); builder.append('\n'); - return builder.build(); + return true; } -static OwnPtr procfs$modules(InodeIdentifier) +static bool procfs$modules(InodeIdentifier, KBufferBuilder& builder) { extern HashMap>* g_modules; - KBufferBuilder builder; JsonArraySerializer array { builder }; for (auto& it : *g_modules) { auto obj = array.add_object(); @@ -461,13 +457,12 @@ static OwnPtr procfs$modules(InodeIdentifier) obj.add("size", size); } array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$profile(InodeIdentifier) +static bool procfs$profile(InodeIdentifier, KBufferBuilder& builder) { InterruptDisabler disabler; - KBufferBuilder builder; JsonObjectSerializer object(builder); object.add("pid", Profiling::pid().value()); @@ -493,12 +488,11 @@ static OwnPtr procfs$profile(InodeIdentifier) }); array.finish(); object.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$net_adapters(InodeIdentifier) +static bool procfs$net_adapters(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; NetworkAdapter::for_each([&array](auto& adapter) { auto obj = array.add_object(); @@ -519,12 +513,11 @@ static OwnPtr procfs$net_adapters(InodeIdentifier) obj.add("mtu", adapter.mtu()); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$net_arp(InodeIdentifier) +static bool procfs$net_arp(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; LOCKER(arp_table().lock(), Lock::Mode::Shared); for (auto& it : arp_table().resource()) { @@ -533,12 +526,11 @@ static OwnPtr procfs$net_arp(InodeIdentifier) obj.add("ip_address", it.key.to_string()); } array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$net_tcp(InodeIdentifier) +static bool procfs$net_tcp(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; TCPSocket::for_each([&array](auto& socket) { auto obj = array.add_object(); @@ -555,12 +547,11 @@ static OwnPtr procfs$net_tcp(InodeIdentifier) obj.add("bytes_out", socket.bytes_out()); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$net_udp(InodeIdentifier) +static bool procfs$net_udp(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; UDPSocket::for_each([&array](auto& socket) { auto obj = array.add_object(); @@ -570,12 +561,11 @@ static OwnPtr procfs$net_udp(InodeIdentifier) obj.add("peer_port", socket.peer_port()); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$net_local(InodeIdentifier) +static bool procfs$net_local(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; LocalSocket::for_each([&array](auto& socket) { auto obj = array.add_object(); @@ -588,15 +578,14 @@ static OwnPtr procfs$net_local(InodeIdentifier) obj.add("acceptor_gid", socket.acceptor_gid()); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$pid_vmobjects(InodeIdentifier identifier) +static bool procfs$pid_vmobjects(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; - KBufferBuilder builder; + return false; builder.appendf("BEGIN END SIZE NAME\n"); { ScopedSpinLock lock(process->get_lock()); @@ -623,15 +612,14 @@ static OwnPtr procfs$pid_vmobjects(InodeIdentifier identifier) builder.appendf("\n"); } } - return builder.build(); + return true; } -static OwnPtr procfs$pid_unveil(InodeIdentifier identifier) +static bool procfs$pid_unveil(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; - KBufferBuilder builder; + return false; JsonArraySerializer array { builder }; for (auto& unveiled_path : process->unveiled_paths()) { if (!unveiled_path.was_explicitly_unveiled()) @@ -652,57 +640,57 @@ static OwnPtr procfs$pid_unveil(InodeIdentifier identifier) obj.add("permissions", permissions_builder.to_string()); } array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$tid_stack(InodeIdentifier identifier) +static bool procfs$tid_stack(InodeIdentifier identifier, KBufferBuilder& builder) { auto thread = Thread::from_tid(to_tid(identifier)); if (!thread) - return {}; - KBufferBuilder builder; + return false; builder.appendf("Thread %d (%s):\n", thread->tid().value(), thread->name().characters()); builder.append(thread->backtrace()); - return builder.build(); + return true; } -static OwnPtr procfs$pid_exe(InodeIdentifier identifier) +static bool procfs$pid_exe(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; + return false; auto* custody = process->executable(); ASSERT(custody); - return KBuffer::try_create_with_bytes(custody->absolute_path().bytes()); + builder.append(custody->absolute_path().bytes()); + return true; } -static OwnPtr procfs$pid_cwd(InodeIdentifier identifier) +static bool procfs$pid_cwd(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; - return KBuffer::try_create_with_bytes(process->current_directory().absolute_path().bytes()); + return false; + builder.append_bytes(process->current_directory().absolute_path().bytes()); + return true; } -static OwnPtr procfs$pid_root(InodeIdentifier identifier) +static bool procfs$pid_root(InodeIdentifier identifier, KBufferBuilder& builder) { auto process = Process::from_pid(to_pid(identifier)); if (!process) - return {}; - return KBuffer::try_create_with_bytes(process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer()); + return false; + builder.append_bytes(process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer()); + return false; } -static OwnPtr procfs$self(InodeIdentifier) +static bool procfs$self(InodeIdentifier, KBufferBuilder& builder) { - char buffer[16]; - int written = snprintf(buffer, sizeof(buffer), "%d", Process::current()->pid().value()); - return KBuffer::try_create_with_bytes(ReadonlyBytes { buffer, static_cast(written) }); + builder.appendf("%d", Process::current()->pid().value()); + return true; } -OwnPtr procfs$mm(InodeIdentifier) +static bool procfs$mm(InodeIdentifier, KBufferBuilder& builder) { InterruptDisabler disabler; - KBufferBuilder builder; u32 vmobject_count = 0; MemoryManager::for_each_vmobject([&](auto& vmobject) { ++vmobject_count; @@ -716,22 +704,20 @@ OwnPtr procfs$mm(InodeIdentifier) builder.appendf("VMO count: %u\n", vmobject_count); builder.appendf("Free physical pages: %u\n", MM.user_physical_pages() - MM.user_physical_pages_used()); builder.appendf("Free supervisor physical pages: %u\n", MM.super_physical_pages() - MM.super_physical_pages_used()); - return builder.build(); + return true; } -static OwnPtr procfs$dmesg(InodeIdentifier) +static bool procfs$dmesg(InodeIdentifier, KBufferBuilder& builder) { InterruptDisabler disabler; - KBufferBuilder builder; for (char ch : Console::the().logbuffer()) builder.append(ch); - return builder.build(); + return true; } -static OwnPtr procfs$mounts(InodeIdentifier) +static bool procfs$mounts(InodeIdentifier, KBufferBuilder& builder) { // FIXME: This is obviously racy against the VFS mounts changing. - KBufferBuilder builder; VFS::the().for_each_mount([&builder](auto& mount) { auto& fs = mount.guest_fs(); builder.appendf("%s @ ", fs.class_name()); @@ -744,13 +730,12 @@ static OwnPtr procfs$mounts(InodeIdentifier) } builder.append('\n'); }); - return builder.build(); + return true; } -static OwnPtr procfs$df(InodeIdentifier) +static bool procfs$df(InodeIdentifier, KBufferBuilder& builder) { // FIXME: This is obviously racy against the VFS mounts changing. - KBufferBuilder builder; JsonArraySerializer array { builder }; VFS::the().for_each_mount([&array](auto& mount) { auto& fs = mount.guest_fs(); @@ -771,12 +756,11 @@ static OwnPtr procfs$df(InodeIdentifier) fs_object.add("source", "none"); }); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$cpuinfo(InodeIdentifier) +static bool procfs$cpuinfo(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; Processor::for_each( [&](Processor& proc) -> IterationDecision { @@ -796,17 +780,16 @@ static OwnPtr procfs$cpuinfo(InodeIdentifier) return IterationDecision::Continue; }); array.finish(); - return builder.build(); + return true; } -OwnPtr procfs$memstat(InodeIdentifier) +static bool procfs$memstat(InodeIdentifier, KBufferBuilder& builder) { InterruptDisabler disabler; kmalloc_stats stats; get_kmalloc_stats(stats); - KBufferBuilder builder; JsonObjectSerializer json { builder }; json.add("kmalloc_allocated", stats.bytes_allocated); json.add("kmalloc_available", stats.bytes_free); @@ -823,12 +806,11 @@ OwnPtr procfs$memstat(InodeIdentifier) json.add(String::format("%s_num_free", prefix.characters()), num_free); }); json.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$all(InodeIdentifier) +static bool procfs$all(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; JsonArraySerializer array { builder }; // Keep this in sync with CProcessStatistics. @@ -914,18 +896,17 @@ static OwnPtr procfs$all(InodeIdentifier) for (auto& process : processes) build_process(process); array.finish(); - return builder.build(); + return true; } -static OwnPtr procfs$inodes(InodeIdentifier) +static bool procfs$inodes(InodeIdentifier, KBufferBuilder& builder) { - KBufferBuilder builder; InterruptDisabler disabler; ScopedSpinLock all_inodes_lock(Inode::all_inodes_lock()); for (auto& inode : Inode::all_with_lock()) { builder.appendf("Inode{K%x} %02u:%08u (%u)\n", &inode, inode.fsid(), inode.index(), inode.ref_count()); } - return builder.build(); + return true; } struct SysVariable { @@ -969,7 +950,7 @@ SysVariable& SysVariable::for_inode(InodeIdentifier id) return variable; } -static OwnPtr read_sys_bool(InodeIdentifier inode_id) +static bool read_sys_bool(InodeIdentifier inode_id, KBufferBuilder& builder) { auto& variable = SysVariable::for_inode(inode_id); ASSERT(variable.type == SysVariable::Type::Boolean); @@ -981,7 +962,8 @@ static OwnPtr read_sys_bool(InodeIdentifier inode_id) buffer[0] = lockable_bool->resource() ? '1' : '0'; } buffer[1] = '\n'; - return KBuffer::try_create_with_bytes(ReadonlyBytes { buffer, sizeof(buffer) }); + builder.append_bytes(ReadonlyBytes { buffer, sizeof(buffer) }); + return true; } static ssize_t write_sys_bool(InodeIdentifier inode_id, const UserOrKernelBuffer& buffer, size_t size) @@ -1013,14 +995,15 @@ static ssize_t write_sys_bool(InodeIdentifier inode_id, const UserOrKernelBuffer return (ssize_t)size; } -static OwnPtr read_sys_string(InodeIdentifier inode_id) +static bool read_sys_string(InodeIdentifier inode_id, KBufferBuilder& builder) { auto& variable = SysVariable::for_inode(inode_id); ASSERT(variable.type == SysVariable::Type::String); auto* lockable_string = reinterpret_cast*>(variable.address); LOCKER(lockable_string->lock(), Lock::Mode::Shared); - return KBuffer::try_create_with_bytes(lockable_string->resource().bytes()); + builder.append_bytes(lockable_string->resource().bytes()); + return true; } static ssize_t write_sys_string(InodeIdentifier inode_id, const UserOrKernelBuffer& buffer, size_t size) @@ -1130,6 +1113,83 @@ ProcFSInode::~ProcFSInode() fs().m_inodes.remove(it); } +KResult ProcFSInode::refresh_data(FileDescription& description) const +{ + auto& cached_data = description.data(); + auto* directory_entry = fs().get_directory_entry(identifier()); + + bool (*read_callback)(InodeIdentifier, KBufferBuilder&) = nullptr; + if (directory_entry) { + if (directory_entry->proc_file_type > (unsigned)FI_Root) { + read_callback = directory_entry->read_callback; + ASSERT(read_callback); + } else { + return KSuccess; + } + } else { + switch (to_proc_parent_directory(identifier())) { + case PDI_PID_fd: + read_callback = procfs$pid_fd_entry; + break; + case PDI_PID_stacks: + read_callback = procfs$tid_stack; + break; + case PDI_Root_sys: + switch (SysVariable::for_inode(identifier()).type) { + case SysVariable::Type::Invalid: + ASSERT_NOT_REACHED(); + case SysVariable::Type::Boolean: + read_callback = read_sys_bool; + break; + case SysVariable::Type::String: + read_callback = read_sys_string; + break; + } + break; + default: + ASSERT_NOT_REACHED(); + } + + ASSERT(read_callback); + } + + if (!cached_data) + cached_data = new ProcFSInodeData; + auto& buffer = static_cast(*cached_data).buffer; + if (buffer) { + // If we're reusing the buffer, reset the size to 0 first. This + // ensures we don't accidentally leak previously written data. + buffer->set_size(0); + } + KBufferBuilder builder(buffer, true); + if (!read_callback(identifier(), builder)) + return KResult(-ENOENT); + // We don't use builder.build() here, which would steal our buffer + // and turn it into an OwnPtr. Instead, just flush to the buffer so + // that we can read all the data that was written. + if (!builder.flush()) + return KResult(-ENOMEM); + if (!buffer) + return KResult(-ENOMEM); + return KSuccess; +} + +KResult ProcFSInode::attach(FileDescription& description) +{ + return refresh_data(description); +} + +void ProcFSInode::did_seek(FileDescription& description, off_t new_offset) +{ + if (new_offset != 0) + return; + auto result = refresh_data(description); + if (result.is_error()) { + // Subsequent calls to read will return EIO! + dbg() << "ProcFS: Could not refresh contents: " << result.error(); + } +} + InodeMetadata ProcFSInode::metadata() const { #ifdef PROCFS_DEBUG @@ -1215,68 +1275,29 @@ InodeMetadata ProcFSInode::metadata() const ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const { #ifdef PROCFS_DEBUG - dbg() << "ProcFS: read_bytes " << index(); + dbg() << "ProcFS: read_bytes offset: " << offset << " count: " << count; #endif ASSERT(offset >= 0); ASSERT(buffer.user_or_kernel_ptr()); - auto* directory_entry = fs().get_directory_entry(identifier()); - - OwnPtr (*read_callback)(InodeIdentifier) = nullptr; - if (directory_entry) - read_callback = directory_entry->read_callback; - else - switch (to_proc_parent_directory(identifier())) { - case PDI_PID_fd: - read_callback = procfs$pid_fd_entry; - break; - case PDI_PID_stacks: - read_callback = procfs$tid_stack; - break; - case PDI_Root_sys: - switch (SysVariable::for_inode(identifier()).type) { - case SysVariable::Type::Invalid: - ASSERT_NOT_REACHED(); - case SysVariable::Type::Boolean: - read_callback = read_sys_bool; - break; - case SysVariable::Type::String: - read_callback = read_sys_string; - break; - } - break; - default: - ASSERT_NOT_REACHED(); - } - - ASSERT(read_callback); - - OwnPtr descriptionless_generated_data; - KBuffer* data = nullptr; - if (!description) { - descriptionless_generated_data = read_callback(identifier()); - data = descriptionless_generated_data.ptr(); - } else { - if (!description->generator_cache()) - description->generator_cache() = (*read_callback)(identifier()); - data = description->generator_cache().ptr(); + if (!description) + return -EIO; + if (!description->data()) { +#ifdef PROCFS_DEBUG + dbg() << "ProcFS: Do not have cached data!"; +#endif + return -EIO; } - if (!data) - return 0; - if (data->is_null()) { - dbg() << "ProcFS: Not enough memory!"; - return 0; - } + // Be sure to keep a reference to data_buffer while we use it! + RefPtr data_buffer = static_cast(*description->data()).buffer; - if ((size_t)offset >= data->size()) + if (!data_buffer || (size_t)offset >= data_buffer->size()) return 0; - ssize_t nread = min(static_cast(data->size() - offset), static_cast(count)); - if (!buffer.write(data->data() + offset, nread)) + ssize_t nread = min(static_cast(data_buffer->size() - offset), static_cast(count)); + if (!buffer.write(data_buffer->data() + offset, nread)) return -EFAULT; - if (nread == 0 && description && description->generator_cache()) - description->generator_cache().clear(); return nread; } @@ -1599,6 +1620,16 @@ ProcFSProxyInode::~ProcFSProxyInode() { } +KResult ProcFSProxyInode::attach(FileDescription& fd) +{ + return m_fd->inode()->attach(fd); +} + +void ProcFSProxyInode::did_seek(FileDescription& fd, off_t new_offset) +{ + return m_fd->inode()->did_seek(fd, new_offset); +} + InodeMetadata ProcFSProxyInode::metadata() const { InodeMetadata metadata = m_fd->metadata(); diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index f8d80ca45d2..36554b1de19 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -30,7 +30,7 @@ #include #include #include -#include +#include #include namespace Kernel { @@ -59,7 +59,7 @@ private: struct ProcFSDirectoryEntry { ProcFSDirectoryEntry() { } - ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, bool a_supervisor_only, OwnPtr (*read_callback)(InodeIdentifier) = nullptr, ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t) = nullptr, RefPtr&& a_inode = nullptr) + ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, bool a_supervisor_only, bool (*read_callback)(InodeIdentifier, KBufferBuilder&) = nullptr, ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t) = nullptr, RefPtr&& a_inode = nullptr) : name(a_name) , proc_file_type(a_proc_file_type) , supervisor_only(a_supervisor_only) @@ -72,7 +72,7 @@ private: const char* name { nullptr }; unsigned proc_file_type { 0 }; bool supervisor_only { false }; - OwnPtr (*read_callback)(InodeIdentifier); + bool (*read_callback)(InodeIdentifier, KBufferBuilder&); ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t); RefPtr inode; InodeIdentifier identifier(unsigned fsid) const; @@ -96,6 +96,8 @@ public: private: // ^Inode + virtual KResult attach(FileDescription&) override; + virtual void did_seek(FileDescription&, off_t) override; virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; virtual KResult traverse_as_directory(Function) const override; @@ -110,6 +112,8 @@ private: virtual KResult chown(uid_t, gid_t) override; virtual KResultOr> resolve_as_link(Custody& base, RefPtr* out_parent = nullptr, int options = 0, int symlink_recursion_level = 0) const override; + KResult refresh_data(FileDescription&) const; + ProcFS& fs() { return static_cast(Inode::fs()); } const ProcFS& fs() const { return static_cast(Inode::fs()); } ProcFSInode(ProcFS&, unsigned index); @@ -123,6 +127,8 @@ public: private: // ^Inode + virtual KResult attach(FileDescription&) override; + virtual void did_seek(FileDescription&, off_t) override; virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const override { ASSERT_NOT_REACHED(); } virtual InodeMetadata metadata() const override; virtual KResult traverse_as_directory(Function) const override { ASSERT_NOT_REACHED(); } diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index f61cd3d5c93..d10152ff079 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -299,13 +299,19 @@ KResultOr> VFS::open(StringView path, int options if (metadata.is_fifo()) { auto fifo = inode.fifo(); if (options & O_WRONLY) { - auto description = fifo->open_direction_blocking(FIFO::Direction::Writer); + auto open_result = fifo->open_direction_blocking(FIFO::Direction::Writer); + if (open_result.is_error()) + return open_result.error(); + auto& description = open_result.value(); description->set_rw_mode(options); description->set_file_flags(options); description->set_original_inode({}, inode); return description; } else if (options & O_RDONLY) { - auto description = fifo->open_direction_blocking(FIFO::Direction::Reader); + auto open_result = fifo->open_direction_blocking(FIFO::Direction::Reader); + if (open_result.is_error()) + return open_result.error(); + auto& description = open_result.value(); description->set_rw_mode(options); description->set_file_flags(options); description->set_original_inode({}, inode); @@ -340,8 +346,10 @@ KResultOr> VFS::open(StringView path, int options inode.set_mtime(kgettimeofday().tv_sec); } auto description = FileDescription::create(custody); - description->set_rw_mode(options); - description->set_file_flags(options); + if (!description.is_error()) { + description.value()->set_rw_mode(options); + description.value()->set_file_flags(options); + } return description; } @@ -400,8 +408,10 @@ KResultOr> VFS::create(StringView path, int optio auto new_custody = Custody::create(&parent_custody, p.basename(), inode_or_error.value(), parent_custody.mount_flags()); auto description = FileDescription::create(*new_custody); - description->set_rw_mode(options); - description->set_file_flags(options); + if (!description.is_error()) { + description.value()->set_rw_mode(options); + description.value()->set_file_flags(options); + } return description; } diff --git a/Kernel/KBuffer.h b/Kernel/KBuffer.h index b6722d71ba6..53ad60bf882 100644 --- a/Kernel/KBuffer.h +++ b/Kernel/KBuffer.h @@ -48,21 +48,21 @@ namespace Kernel { class KBufferImpl : public RefCounted { public: - static RefPtr try_create_with_size(size_t size, u8 access, const char* name, AllocationStrategy strategy = AllocationStrategy::Reserve) + static RefPtr try_create_with_size(size_t size, u8 access, const char* name = "KBuffer", AllocationStrategy strategy = AllocationStrategy::Reserve) { auto region = MM.allocate_kernel_region(PAGE_ROUND_UP(size), name, access, false, strategy); if (!region) return nullptr; - return adopt(*new KBufferImpl(region.release_nonnull(), size)); + return adopt(*new KBufferImpl(region.release_nonnull(), size, strategy)); } - static RefPtr try_create_with_bytes(ReadonlyBytes bytes, u8 access, const char* name, AllocationStrategy strategy = AllocationStrategy::Reserve) + static RefPtr try_create_with_bytes(ReadonlyBytes bytes, u8 access, const char* name = "KBuffer", AllocationStrategy strategy = AllocationStrategy::Reserve) { auto region = MM.allocate_kernel_region(PAGE_ROUND_UP(bytes.size()), name, access, false, strategy); if (!region) return nullptr; memcpy(region->vaddr().as_ptr(), bytes.data(), bytes.size()); - return adopt(*new KBufferImpl(region.release_nonnull(), bytes.size())); + return adopt(*new KBufferImpl(region.release_nonnull(), bytes.size(), strategy)); } static RefPtr create_with_size(size_t size, u8 access, const char* name, AllocationStrategy strategy = AllocationStrategy::Reserve) @@ -79,6 +79,17 @@ public: return buffer; } + bool expand(size_t new_capacity) + { + auto new_region = MM.allocate_kernel_region(PAGE_ROUND_UP(new_capacity), m_region->name(), m_region->access(), false, m_allocation_strategy); + if (!new_region) + return false; + if (m_region && m_size > 0) + memcpy(new_region->vaddr().as_ptr(), data(), min(m_region->size(), m_size)); + m_region = new_region.release_nonnull(); + return true; + } + u8* data() { return m_region->vaddr().as_ptr(); } const u8* data() const { return m_region->vaddr().as_ptr(); } size_t size() const { return m_size; } @@ -94,18 +105,25 @@ public: Region& region() { return *m_region; } private: - explicit KBufferImpl(NonnullOwnPtr&& region, size_t size) + explicit KBufferImpl(NonnullOwnPtr&& region, size_t size, AllocationStrategy strategy) : m_size(size) + , m_allocation_strategy(strategy) , m_region(move(region)) { } size_t m_size { 0 }; + AllocationStrategy m_allocation_strategy { AllocationStrategy::Reserve }; NonnullOwnPtr m_region; }; class KBuffer { public: + explicit KBuffer(RefPtr&& impl) + : m_impl(move(impl)) + { + } + static OwnPtr try_create_with_size(size_t size, u8 access = Region::Access::Read | Region::Access::Write, const char* name = "KBuffer", AllocationStrategy strategy = AllocationStrategy::Reserve) { auto impl = KBufferImpl::try_create_with_size(size, access, name, strategy); @@ -145,6 +163,7 @@ public: void set_size(size_t size) { m_impl->set_size(size); } const KBufferImpl& impl() const { return *m_impl; } + RefPtr take_impl() { return move(m_impl); } KBuffer(const ByteBuffer& buffer, u8 access = Region::Access::Read | Region::Access::Write, const char* name = "KBuffer") : m_impl(KBufferImpl::copy(buffer.data(), buffer.size(), access, name)) @@ -152,11 +171,6 @@ public: } private: - explicit KBuffer(RefPtr&& impl) - : m_impl(move(impl)) - { - } - RefPtr m_impl; }; diff --git a/Kernel/KBufferBuilder.cpp b/Kernel/KBufferBuilder.cpp index 7823a5f52ae..a5ec9b7c0db 100644 --- a/Kernel/KBufferBuilder.cpp +++ b/Kernel/KBufferBuilder.cpp @@ -31,32 +31,65 @@ namespace Kernel { -inline bool KBufferBuilder::can_append(size_t size) const +inline bool KBufferBuilder::check_expand(size_t size) { if (!m_buffer) return false; - return ((m_size + size) < m_buffer->size()); + if ((m_size + size) < m_buffer->capacity()) + return true; + if (!m_can_expand) + return false; + if (Checked::addition_would_overflow(m_size, size)) + return false; + size_t new_buffer_size = m_size + size; + if (Checked::addition_would_overflow(new_buffer_size, 1 * MiB)) + return false; + new_buffer_size = PAGE_ROUND_UP(new_buffer_size + 1 * MiB); + return m_buffer->expand(new_buffer_size); +} + +bool KBufferBuilder::flush() +{ + if (!m_buffer) + return false; + m_buffer->set_size(m_size); + return true; } OwnPtr KBufferBuilder::build() { - if (!m_buffer) + if (!flush()) return {}; - if (!m_buffer->is_null()) - m_buffer->set_size(m_size); - return m_buffer.release_nonnull(); + return make(move(m_buffer)); } -KBufferBuilder::KBufferBuilder() - : m_buffer(KBuffer::try_create_with_size(4 * MiB, Region::Access::Read | Region::Access::Write)) +KBufferBuilder::KBufferBuilder(bool can_expand) + : m_buffer(KBufferImpl::try_create_with_size(4 * MiB, Region::Access::Read | Region::Access::Write)) + , m_can_expand(can_expand) { } +KBufferBuilder::KBufferBuilder(RefPtr& buffer, bool can_expand) + : m_buffer(buffer) + , m_can_expand(can_expand) +{ + if (!m_buffer) + m_buffer = buffer = KBufferImpl::try_create_with_size(4 * MiB, Region::Access::Read | Region::Access::Write); +} + +void KBufferBuilder::append_bytes(ReadonlyBytes bytes) +{ + if (!check_expand(bytes.size())) + return; + memcpy(insertion_ptr(), bytes.data(), bytes.size()); + m_size += bytes.size(); +} + void KBufferBuilder::append(const StringView& str) { if (str.is_empty()) return; - if (!can_append(str.length())) + if (!check_expand(str.length())) return; memcpy(insertion_ptr(), str.characters_without_null_termination(), str.length()); m_size += str.length(); @@ -66,7 +99,7 @@ void KBufferBuilder::append(const char* characters, int length) { if (!length) return; - if (!can_append(length)) + if (!check_expand(length)) return; memcpy(insertion_ptr(), characters, length); m_size += length; @@ -74,7 +107,7 @@ void KBufferBuilder::append(const char* characters, int length) void KBufferBuilder::append(char ch) { - if (!can_append(1)) + if (!check_expand(1)) return; insertion_ptr()[0] = ch; m_size += 1; diff --git a/Kernel/KBufferBuilder.h b/Kernel/KBufferBuilder.h index 50e77c0f3b3..21e9eb6a563 100644 --- a/Kernel/KBufferBuilder.h +++ b/Kernel/KBufferBuilder.h @@ -36,7 +36,8 @@ class KBufferBuilder { public: using OutputType = KBuffer; - explicit KBufferBuilder(); + explicit KBufferBuilder(bool can_expand = false); + explicit KBufferBuilder(RefPtr&, bool can_expand = false); KBufferBuilder(KBufferBuilder&&) = default; ~KBufferBuilder() { } @@ -47,6 +48,7 @@ public: void appendvf(const char*, va_list); void append_escaped_for_json(const StringView&); + void append_bytes(ReadonlyBytes); template void appendff(StringView fmtstr, const Parameters&... parameters) @@ -56,10 +58,11 @@ public: append(String::formatted(fmtstr, parameters...)); } + bool flush(); OwnPtr build(); private: - bool can_append(size_t) const; + bool check_expand(size_t); u8* insertion_ptr() { if (!m_buffer) @@ -67,8 +70,9 @@ private: return m_buffer->data() + m_size; } - OwnPtr m_buffer; + RefPtr m_buffer; size_t m_size { 0 }; + bool m_can_expand { false }; }; } diff --git a/Kernel/Net/IPv4Socket.cpp b/Kernel/Net/IPv4Socket.cpp index aaf5bfd8283..5c9ba42b685 100644 --- a/Kernel/Net/IPv4Socket.cpp +++ b/Kernel/Net/IPv4Socket.cpp @@ -170,14 +170,6 @@ KResult IPv4Socket::connect(FileDescription& description, Userspace sendto(FileDescription&, const UserOrKernelBuffer&, size_t, int, Userspace, socklen_t) override; diff --git a/Kernel/Net/LocalSocket.cpp b/Kernel/Net/LocalSocket.cpp index b628d2dc6c2..bb15c43d4f2 100644 --- a/Kernel/Net/LocalSocket.cpp +++ b/Kernel/Net/LocalSocket.cpp @@ -224,7 +224,7 @@ KResult LocalSocket::listen(size_t backlog) return KSuccess; } -void LocalSocket::attach(FileDescription& description) +KResult LocalSocket::attach(FileDescription& description) { ASSERT(!m_accept_side_fd_open); if (m_connect_side_role == Role::None) { @@ -236,6 +236,7 @@ void LocalSocket::attach(FileDescription& description) } evaluate_block_conditions(); + return KSuccess; } void LocalSocket::detach(FileDescription& description) diff --git a/Kernel/Net/LocalSocket.h b/Kernel/Net/LocalSocket.h index f0025cdb14f..ab98dd017e7 100644 --- a/Kernel/Net/LocalSocket.h +++ b/Kernel/Net/LocalSocket.h @@ -56,7 +56,7 @@ public: virtual KResult listen(size_t) override; virtual void get_local_address(sockaddr*, socklen_t*) override; virtual void get_peer_address(sockaddr*, socklen_t*) override; - virtual void attach(FileDescription&) override; + virtual KResult attach(FileDescription&) override; virtual void detach(FileDescription&) override; virtual bool can_read(const FileDescription&, size_t) const override; virtual bool can_write(const FileDescription&, size_t) const override; diff --git a/Kernel/Net/Socket.h b/Kernel/Net/Socket.h index 9116617bb19..a4dfaa6d782 100644 --- a/Kernel/Net/Socket.h +++ b/Kernel/Net/Socket.h @@ -105,8 +105,6 @@ public: virtual void get_peer_address(sockaddr*, socklen_t*) = 0; virtual bool is_local() const { return false; } virtual bool is_ipv4() const { return false; } - virtual void attach(FileDescription&) = 0; - virtual void detach(FileDescription&) = 0; virtual KResultOr sendto(FileDescription&, const UserOrKernelBuffer&, size_t, int flags, Userspace, socklen_t) = 0; virtual KResultOr recvfrom(FileDescription&, UserOrKernelBuffer&, size_t, int flags, Userspace, Userspace, timeval&) = 0; diff --git a/Kernel/Storage/StorageManagement.cpp b/Kernel/Storage/StorageManagement.cpp index 30152ba3aaa..539bd2e311b 100644 --- a/Kernel/Storage/StorageManagement.cpp +++ b/Kernel/Storage/StorageManagement.cpp @@ -196,11 +196,12 @@ RefPtr StorageManagement::boot_block_device() const NonnullRefPtr StorageManagement::root_filesystem() const { - if (!boot_block_device()) { + auto boot_device_description = boot_block_device(); + if (!boot_device_description) { klog() << "init_stage2: couldn't find a suitable device to boot from"; Processor::halt(); } - auto e2fs = Ext2FS::create(*FileDescription::create(boot_block_device().release_nonnull())); + auto e2fs = Ext2FS::create(FileDescription::create(boot_device_description.release_nonnull()).value()); if (!e2fs->initialize()) { klog() << "init_stage2: couldn't open root filesystem"; Processor::halt(); diff --git a/Kernel/Syscalls/pipe.cpp b/Kernel/Syscalls/pipe.cpp index 3d82917c8fe..5692f2f1399 100644 --- a/Kernel/Syscalls/pipe.cpp +++ b/Kernel/Syscalls/pipe.cpp @@ -42,14 +42,21 @@ int Process::sys$pipe(int pipefd[2], int flags) u32 fd_flags = (flags & O_CLOEXEC) ? FD_CLOEXEC : 0; auto fifo = FIFO::create(m_uid); + auto open_reader_result = fifo->open_direction(FIFO::Direction::Reader); + if (open_reader_result.is_error()) + return open_reader_result.error(); + auto open_writer_result = fifo->open_direction(FIFO::Direction::Writer); + if (open_writer_result.is_error()) + return open_writer_result.error(); + int reader_fd = alloc_fd(); - m_fds[reader_fd].set(fifo->open_direction(FIFO::Direction::Reader), fd_flags); + m_fds[reader_fd].set(open_reader_result.release_value(), fd_flags); m_fds[reader_fd].description()->set_readable(true); if (!copy_to_user(&pipefd[0], &reader_fd)) return -EFAULT; int writer_fd = alloc_fd(); - m_fds[writer_fd].set(fifo->open_direction(FIFO::Direction::Writer), fd_flags); + m_fds[writer_fd].set(open_writer_result.release_value(), fd_flags); m_fds[writer_fd].description()->set_writable(true); if (!copy_to_user(&pipefd[1], &writer_fd)) return -EFAULT; diff --git a/Kernel/Syscalls/socket.cpp b/Kernel/Syscalls/socket.cpp index ede0d113704..2fdf7f8b3b3 100644 --- a/Kernel/Syscalls/socket.cpp +++ b/Kernel/Syscalls/socket.cpp @@ -52,15 +52,17 @@ int Process::sys$socket(int domain, int type, int protocol) auto result = Socket::create(domain, type, protocol); if (result.is_error()) return result.error(); - auto description = FileDescription::create(*result.value()); - description->set_readable(true); - description->set_writable(true); + auto description_result = FileDescription::create(*result.value()); + if (description_result.is_error()) + return description_result.error(); + description_result.value()->set_readable(true); + description_result.value()->set_writable(true); unsigned flags = 0; if (type & SOCK_CLOEXEC) flags |= FD_CLOEXEC; if (type & SOCK_NONBLOCK) - description->set_blocking(false); - m_fds[fd].set(move(description), flags); + description_result.value()->set_blocking(false); + m_fds[fd].set(description_result.release_value(), flags); return fd; } @@ -132,13 +134,16 @@ int Process::sys$accept(int accepting_socket_fd, Userspace user_addre return -EFAULT; } - auto accepted_socket_description = FileDescription::create(*accepted_socket); - accepted_socket_description->set_readable(true); - accepted_socket_description->set_writable(true); + auto accepted_socket_description_result = FileDescription::create(*accepted_socket); + if (accepted_socket_description_result.is_error()) + return accepted_socket_description_result.error(); + + accepted_socket_description_result.value()->set_readable(true); + accepted_socket_description_result.value()->set_writable(true); // NOTE: The accepted socket inherits fd flags from the accepting socket. // I'm not sure if this matches other systems but it makes sense to me. - accepted_socket_description->set_blocking(accepting_socket_description->is_blocking()); - m_fds[accepted_socket_fd].set(move(accepted_socket_description), m_fds[accepting_socket_fd].flags()); + accepted_socket_description_result.value()->set_blocking(accepting_socket_description->is_blocking()); + m_fds[accepted_socket_fd].set(accepted_socket_description_result.release_value(), m_fds[accepting_socket_fd].flags()); // NOTE: Moving this state to Completed is what causes connect() to unblock on the client side. accepted_socket->set_setup_state(Socket::SetupState::Completed); diff --git a/Kernel/Syscalls/watch_file.cpp b/Kernel/Syscalls/watch_file.cpp index caf8299528a..2f6194bc7ab 100644 --- a/Kernel/Syscalls/watch_file.cpp +++ b/Kernel/Syscalls/watch_file.cpp @@ -52,7 +52,11 @@ int Process::sys$watch_file(Userspace user_path, size_t path_length if (fd < 0) return fd; - m_fds[fd].set(FileDescription::create(*InodeWatcher::create(inode))); + auto description = FileDescription::create(*InodeWatcher::create(inode)); + if (description.is_error()) + return description.error(); + + m_fds[fd].set(description.release_value()); m_fds[fd].description()->set_readable(true); return fd; } diff --git a/Kernel/TTY/PTYMultiplexer.cpp b/Kernel/TTY/PTYMultiplexer.cpp index e4b7394a9b9..7a9789c5440 100644 --- a/Kernel/TTY/PTYMultiplexer.cpp +++ b/Kernel/TTY/PTYMultiplexer.cpp @@ -66,8 +66,10 @@ KResultOr> PTYMultiplexer::open(int options) dbg() << "PTYMultiplexer::open: Vending master " << master->index(); #endif auto description = FileDescription::create(move(master)); - description->set_rw_mode(options); - description->set_file_flags(options); + if (!description.is_error()) { + description.value()->set_rw_mode(options); + description.value()->set_file_flags(options); + } return description; } diff --git a/Meta/CMake/all_the_debug_macros.cmake b/Meta/CMake/all_the_debug_macros.cmake index f651fb506e1..8c14ee234ae 100644 --- a/Meta/CMake/all_the_debug_macros.cmake +++ b/Meta/CMake/all_the_debug_macros.cmake @@ -56,6 +56,7 @@ add_compile_definitions("EXEC_DEBUG") add_compile_definitions("EXT2_DEBUG") add_compile_definitions("EXT2_VERY_DEBUG") add_compile_definitions("FIFO_DEBUG") +add_compile_definitions("FILEDESCRIPTION_DEBUG") add_compile_definitions("FILL_PATH_DEBUG") add_compile_definitions("FORK_DEBUG") add_compile_definitions("GBOXLAYOUT_DEBUG")