From b425c2602c6dd10b5509fcd8d81c765b9c76993a Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 2 Mar 2021 16:55:54 +0100 Subject: [PATCH] Kernel: Better handling of allocation failure in profiling If we can't allocate a PerformanceEventBuffer to store the profiling events, we now fail sys$profiling_enable() and sys$perf_event() with ENOMEM instead of carrying on with a broken buffer. --- Kernel/PerformanceEventBuffer.cpp | 12 ++++++++++-- Kernel/PerformanceEventBuffer.h | 13 +++++-------- Kernel/Process.cpp | 9 +++++---- Kernel/Process.h | 3 +-- Kernel/Syscalls/perf_event.cpp | 4 +++- Kernel/Syscalls/profiling.cpp | 3 ++- 6 files changed, 26 insertions(+), 18 deletions(-) diff --git a/Kernel/PerformanceEventBuffer.cpp b/Kernel/PerformanceEventBuffer.cpp index 81b6bbf6680..0af0269d50a 100644 --- a/Kernel/PerformanceEventBuffer.cpp +++ b/Kernel/PerformanceEventBuffer.cpp @@ -34,8 +34,8 @@ namespace Kernel { -PerformanceEventBuffer::PerformanceEventBuffer() - : m_buffer(KBuffer::try_create_with_size(4 * MiB, Region::Access::Read | Region::Access::Write, "Performance events", AllocationStrategy::AllocateNow)) +PerformanceEventBuffer::PerformanceEventBuffer(NonnullOwnPtr buffer) + : m_buffer(move(buffer)) { } @@ -171,4 +171,12 @@ bool PerformanceEventBuffer::to_json(KBufferBuilder& builder, ProcessID pid, con return true; } +OwnPtr PerformanceEventBuffer::try_create_with_size(size_t buffer_size) +{ + auto buffer = KBuffer::try_create_with_size(buffer_size, Region::Access::Read | Region::Access::Write, "Performance events", AllocationStrategy::AllocateNow); + if (!buffer) + return {}; + return adopt_own(*new PerformanceEventBuffer(buffer.release_nonnull())); +} + } diff --git a/Kernel/PerformanceEventBuffer.h b/Kernel/PerformanceEventBuffer.h index c1cb429266b..81911a17844 100644 --- a/Kernel/PerformanceEventBuffer.h +++ b/Kernel/PerformanceEventBuffer.h @@ -58,7 +58,7 @@ struct [[gnu::packed]] PerformanceEvent { class PerformanceEventBuffer { public: - PerformanceEventBuffer(); + static OwnPtr try_create_with_size(size_t buffer_size); KResult append(int type, FlatPtr arg1, FlatPtr arg2); KResult append_with_eip_and_ebp(u32 eip, u32 ebp, int type, FlatPtr arg1, FlatPtr arg2); @@ -68,12 +68,7 @@ public: m_count = 0; } - size_t capacity() const - { - if (!m_buffer) - return 0; - return m_buffer->size() / sizeof(PerformanceEvent); - } + size_t capacity() const { return m_buffer->size() / sizeof(PerformanceEvent); } size_t count() const { return m_count; } const PerformanceEvent& at(size_t index) const { @@ -84,10 +79,12 @@ public: bool to_json(KBufferBuilder&, ProcessID, const String& executable_path) const; private: + explicit PerformanceEventBuffer(NonnullOwnPtr); + PerformanceEvent& at(size_t index); size_t m_count { 0 }; - OwnPtr m_buffer; + NonnullOwnPtr m_buffer; }; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 45037536ea6..4cba181984c 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -667,11 +667,12 @@ void Process::tracer_trap(Thread& thread, const RegisterState& regs) thread.send_urgent_signal_to_self(SIGTRAP); } -PerformanceEventBuffer& Process::ensure_perf_events() +bool Process::create_perf_events_buffer_if_needed() { - if (!m_perf_event_buffer) - m_perf_event_buffer = make(); - return *m_perf_event_buffer; + if (!m_perf_event_buffer) { + m_perf_event_buffer = PerformanceEventBuffer::try_create_with_size(4 * MiB); + } + return !!m_perf_event_buffer; } bool Process::remove_thread(Thread& thread) diff --git a/Kernel/Process.h b/Kernel/Process.h index 0a64d40209d..626072baa84 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -467,8 +467,6 @@ private: bool add_thread(Thread&); bool remove_thread(Thread&); - PerformanceEventBuffer& ensure_perf_events(); - Process(RefPtr& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr cwd = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); static ProcessID allocate_pid(); @@ -476,6 +474,7 @@ private: void kill_all_threads(); bool dump_core(); bool dump_perfcore(); + bool create_perf_events_buffer_if_needed(); KResult do_exec(NonnullRefPtr main_program_description, Vector arguments, Vector environment, RefPtr interpreter_description, Thread*& new_main_thread, u32& prev_flags, const Elf32_Ehdr& main_program_header); KResultOr do_write(FileDescription&, const UserOrKernelBuffer&, size_t); diff --git a/Kernel/Syscalls/perf_event.cpp b/Kernel/Syscalls/perf_event.cpp index 36229bc232a..3e90b95a778 100644 --- a/Kernel/Syscalls/perf_event.cpp +++ b/Kernel/Syscalls/perf_event.cpp @@ -31,7 +31,9 @@ namespace Kernel { KResultOr Process::sys$perf_event(int type, FlatPtr arg1, FlatPtr arg2) { - return ensure_perf_events().append(type, arg1, arg2); + if (!create_perf_events_buffer_if_needed()) + return ENOMEM; + return perf_events()->append(type, arg1, arg2); } } diff --git a/Kernel/Syscalls/profiling.cpp b/Kernel/Syscalls/profiling.cpp index c02cf73215b..42e1381697b 100644 --- a/Kernel/Syscalls/profiling.cpp +++ b/Kernel/Syscalls/profiling.cpp @@ -43,7 +43,8 @@ KResultOr Process::sys$profiling_enable(pid_t pid) return ESRCH; if (!is_superuser() && process->uid() != m_euid) return EPERM; - process->ensure_perf_events(); + if (!process->create_perf_events_buffer_if_needed()) + return ENOMEM; process->set_profiling(true); return 0; }