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.
Cnidarias · Mar 2, 2021 · b425c26 · b425c26
1 parent e7ef729
commit b425c26
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 18 deletions.
diff --git 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<KBuffer> buffer)
+ : m_buffer(move(buffer))
 {
 }
 
@@ -171,4 +171,12 @@ bool PerformanceEventBuffer::to_json(KBufferBuilder& builder, ProcessID pid, con
  return true;
 }
 
+OwnPtr<PerformanceEventBuffer> 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
@@ -58,7 +58,7 @@ struct [[gnu::packed]] PerformanceEvent {
 
 class PerformanceEventBuffer {
 public:
- PerformanceEventBuffer();
+ static OwnPtr<PerformanceEventBuffer> 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 @@ class PerformanceEventBuffer {
  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 @@ class PerformanceEventBuffer {
  bool to_json(KBufferBuilder&, ProcessID, const String& executable_path) const;
 
 private:
+ explicit PerformanceEventBuffer(NonnullOwnPtr<KBuffer>);
+
  PerformanceEvent& at(size_t index);
 
  size_t m_count { 0 };
- OwnPtr<KBuffer> m_buffer;
+ NonnullOwnPtr<KBuffer> m_buffer;
 };
 
 }
diff --git 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<PerformanceEventBuffer>();
- 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
@@ -467,15 +467,14 @@ class Process
  bool add_thread(Thread&);
  bool remove_thread(Thread&);
 
- PerformanceEventBuffer& ensure_perf_events();
-
  Process(RefPtr<Thread>& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> cwd = nullptr, RefPtr<Custody> executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr);
  static ProcessID allocate_pid();
 
  void kill_threads_except_self();
  void kill_all_threads();
  bool dump_core();
  bool dump_perfcore();
+ bool create_perf_events_buffer_if_needed();
 
  KResult do_exec(NonnullRefPtr<FileDescription> main_program_description, Vector<String> arguments, Vector<String> environment, RefPtr<FileDescription> interpreter_description, Thread*& new_main_thread, u32& prev_flags, const Elf32_Ehdr& main_program_header);
  KResultOr<ssize_t> do_write(FileDescription&, const UserOrKernelBuffer&, size_t);

diff --git a/Kernel/Syscalls/perf_event.cpp b/Kernel/Syscalls/perf_event.cpp
@@ -31,7 +31,9 @@ namespace Kernel {
 
 KResultOr<int> 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
@@ -43,7 +43,8 @@ KResultOr<int> 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;
 }