From bee75c1f242172cd20e52483c69335d9576ab8bc Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 10 Jul 2021 00:24:47 +0300 Subject: [PATCH] Kernel/ProcFS: Allow a process directory to have a null Process pointer In case we are about to delete the PID directory, we clear the Process pointer. If someone still holds a reference to the PID directory (by opening it), we still need to delete the process, but we can't delete the directory, so we will keep it alive, but any operation on it will fail by propogating the error to userspace about that the Process was deleted and therefore there's no meaning to trying to do operations on the directory. Fixes #8576. --- Kernel/FileSystem/ProcFS.cpp | 2 - Kernel/ProcessExposed.cpp | 4 +- Kernel/ProcessExposed.h | 32 +++++++++++--- Kernel/ProcessSpecificExposed.cpp | 73 ++++++++++++++++++++++++------- 4 files changed, 87 insertions(+), 24 deletions(-) diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 2575024246..bed114a1aa 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -59,8 +59,6 @@ void ProcFSComponentRegistry::unregister_process(Process& deleted_process) process_folder->prepare_for_deletion(); process_folder->m_list_node.remove(); dbgln_if(PROCFS_DEBUG, "ProcFSExposedDirectory ref_count now: {}", process_folder->ref_count()); - // Note: Let's ensure we are the last holder of the ProcFSProcessDirectory object before it can be deleted for good - VERIFY(process_folder->ref_count() == 1); } RefPtr ProcFS::create() diff --git a/Kernel/ProcessExposed.cpp b/Kernel/ProcessExposed.cpp index 9d2679d650..ba32ab8fdd 100644 --- a/Kernel/ProcessExposed.cpp +++ b/Kernel/ProcessExposed.cpp @@ -160,7 +160,9 @@ KResult ProcFSProcessInformation::refresh_data(FileDescription& description) con auto parent_folder = const_cast(*this).m_parent_folder.strong_ref(); if (parent_folder.is_null()) return KResult(EINVAL); - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (!process) + return KResult(ESRCH); process->ptrace_lock().lock(); if (!process->is_dumpable()) { process->ptrace_lock().unlock(); diff --git a/Kernel/ProcessExposed.h b/Kernel/ProcessExposed.h index 2c739978e6..c5a1a2afb7 100644 --- a/Kernel/ProcessExposed.h +++ b/Kernel/ProcessExposed.h @@ -142,19 +142,21 @@ class ProcFSProcessDirectory final public: static NonnullRefPtr create(const Process&); - NonnullRefPtr associated_process() { return m_associated_process; } + RefPtr associated_process() { return m_associated_process; } - virtual uid_t owner_user() const override { return m_associated_process->uid(); } - virtual gid_t owner_group() const override { return m_associated_process->gid(); } + virtual uid_t owner_user() const override { return m_associated_process ? m_associated_process->uid() : 0; } + virtual gid_t owner_group() const override { return m_associated_process ? m_associated_process->gid() : 0; } virtual KResult refresh_data(FileDescription&) const override; virtual RefPtr lookup(StringView name) override; + virtual void prepare_for_deletion() override; + private: void on_attach(); IntrusiveListNode> m_list_node; explicit ProcFSProcessDirectory(const Process&); - NonnullRefPtr m_associated_process; + RefPtr m_associated_process; }; class ProcFSBusDirectory : public ProcFSExposedDirectory { @@ -227,8 +229,26 @@ public: virtual KResultOr read_bytes(off_t offset, size_t count, UserOrKernelBuffer& buffer, FileDescription* description) const override; - virtual uid_t owner_user() const override { return m_parent_folder.strong_ref()->m_associated_process->uid(); } - virtual gid_t owner_group() const override { return m_parent_folder.strong_ref()->m_associated_process->gid(); } + virtual uid_t owner_user() const override + { + auto parent_folder = m_parent_folder.strong_ref(); + if (!parent_folder) + return false; + auto process = parent_folder->associated_process(); + if (!process) + return false; + return process->uid(); + } + virtual gid_t owner_group() const override + { + auto parent_folder = m_parent_folder.strong_ref(); + if (!parent_folder) + return false; + auto process = parent_folder->associated_process(); + if (!process) + return false; + return process->gid(); + } protected: ProcFSProcessInformation(StringView name, const ProcFSProcessDirectory& process_folder) diff --git a/Kernel/ProcessSpecificExposed.cpp b/Kernel/ProcessSpecificExposed.cpp index 81e1e14be9..ad26f1940b 100644 --- a/Kernel/ProcessSpecificExposed.cpp +++ b/Kernel/ProcessSpecificExposed.cpp @@ -93,7 +93,10 @@ KResultOr ProcFSProcessStacks::entries_count() const auto parent_folder = m_process_folder.strong_ref(); if (parent_folder.is_null()) return KResult(EINVAL); - return parent_folder->m_associated_process->thread_count(); + auto process = parent_folder->associated_process(); + if (process.is_null()) + return KResult(ESRCH); + return process->thread_count(); } KResult ProcFSProcessStacks::traverse_as_directory(unsigned fsid, Function callback) const @@ -105,7 +108,9 @@ KResult ProcFSProcessStacks::traverse_as_directory(unsigned fsid, Functioncomponent_index() }, 0 }); - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return KResult(ESRCH); process->for_each_thread([&](const Thread& thread) { int tid = thread.tid().value(); InodeIdentifier identifier = { fsid, thread.global_procfs_inode_index() }; @@ -120,7 +125,9 @@ RefPtr ProcFSProcessStacks::lookup(StringView name) auto parent_folder = m_process_folder.strong_ref(); if (parent_folder.is_null()) return nullptr; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return nullptr; RefPtr procfd_stack; // FIXME: Try to exit the loop earlier process->for_each_thread([&](const Thread& thread) { @@ -195,7 +202,10 @@ KResultOr ProcFSProcessFileDescriptions::entries_count() const auto parent_folder = m_process_folder.strong_ref(); if (parent_folder.is_null()) return KResult(EINVAL); - return parent_folder->m_associated_process->fds().open_count(); + auto process = parent_folder->associated_process(); + if (process.is_null()) + return KResult(ESRCH); + return process->fds().open_count(); } KResult ProcFSProcessFileDescriptions::traverse_as_directory(unsigned fsid, Function callback) const { @@ -206,7 +216,9 @@ KResult ProcFSProcessFileDescriptions::traverse_as_directory(unsigned fsid, Func callback({ ".", { fsid, component_index() }, 0 }); callback({ "..", { fsid, parent_folder->component_index() }, 0 }); - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return KResult(ESRCH); size_t count = 0; process->fds().enumerate([&](auto& file_description_metadata) { if (!file_description_metadata.is_valid()) { @@ -225,7 +237,9 @@ RefPtr ProcFSProcessFileDescriptions::lookup(StringView auto parent_folder = m_process_folder.strong_ref(); if (parent_folder.is_null()) return nullptr; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return nullptr; RefPtr procfd_fd; // FIXME: Try to exit the loop earlier size_t count = 0; @@ -259,7 +273,9 @@ private: auto parent_folder = m_parent_folder.strong_ref(); if (parent_folder.is_null()) return false; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; JsonObjectSerializer obj { builder }; #define __ENUMERATE_PLEDGE_PROMISE(x) \ if (process->has_promised(Pledge::x)) { \ @@ -295,8 +311,11 @@ private: auto parent_folder = m_parent_folder.strong_ref(); if (parent_folder.is_null()) return false; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; JsonArraySerializer array { builder }; - for (auto& unveiled_path : parent_folder->m_associated_process->unveiled_paths()) { + for (auto& unveiled_path : process->unveiled_paths()) { if (!unveiled_path.was_explicitly_unveiled()) continue; auto obj = array.add_object(); @@ -337,7 +356,9 @@ private: auto parent_folder = m_parent_folder.strong_ref(); if (parent_folder.is_null()) return false; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; if (!process->perf_events()) { dbgln("ProcFS: No perf events for {}", process->pid()); return false; @@ -364,7 +385,9 @@ private: if (parent_folder.is_null()) return false; JsonArraySerializer array { builder }; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; if (process->fds().open_count() == 0) { array.finish(); return true; @@ -414,7 +437,10 @@ private: auto parent_folder = m_parent_process_directory.strong_ref(); if (parent_folder.is_null()) return false; - builder.append_bytes(parent_folder->m_associated_process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer()); + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; + builder.append_bytes(process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer()); return true; } WeakPtr m_parent_process_directory; @@ -437,7 +463,9 @@ private: auto parent_folder = m_parent_folder.strong_ref(); if (parent_folder.is_null()) return false; - auto process = parent_folder->m_associated_process; + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; JsonArraySerializer array { builder }; { ScopedSpinLock lock(process->space().get_lock()); @@ -500,7 +528,10 @@ private: auto parent_folder = m_parent_process_directory.strong_ref(); if (parent_folder.is_null()) return false; - builder.append_bytes(parent_folder->m_associated_process->current_directory().absolute_path().bytes()); + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; + builder.append_bytes(process->current_directory().absolute_path().bytes()); return true; } @@ -519,7 +550,10 @@ public: auto parent_folder = m_parent_process_directory.strong_ref(); if (parent_folder.is_null()) return false; - if (!parent_folder->m_associated_process->executable()) + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; + if (!process->executable()) return 0; return ProcFSExposedComponent::required_mode(); } @@ -535,7 +569,10 @@ private: auto parent_folder = m_parent_process_directory.strong_ref(); if (parent_folder.is_null()) return false; - auto* custody = parent_folder->m_associated_process->executable(); + auto process = parent_folder->associated_process(); + if (process.is_null()) + return false; + auto* custody = process->executable(); if (!custody) return false; builder.append(custody->absolute_path().bytes()); @@ -582,6 +619,12 @@ NonnullRefPtr ProcFSProcessDirectory::create(const Proce return adopt_ref_if_nonnull(new (nothrow) ProcFSProcessDirectory(process)).release_nonnull(); } +void ProcFSProcessDirectory::prepare_for_deletion() +{ + ProcFSExposedDirectory::prepare_for_deletion(); + m_associated_process.clear(); +} + ProcFSProcessDirectory::ProcFSProcessDirectory(const Process& process) : ProcFSExposedDirectory(String::formatted("{:d}", process.pid().value()), ProcFSComponentRegistry::the().root_folder()) , m_associated_process(process)