From 2dfe5751f35d0067747c6615bf139871cc105fa6 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 22 Dec 2020 09:59:14 +0100 Subject: [PATCH] Kernel: Abort core dump generation if any substep fails And make an effort to propagate errors out from the inner parts. This fixes an issue where the kernel would infinitely loop in coredump generation if the TmpFS filled up. --- Kernel/CoreDump.cpp | 48 +++++++++++++++++++++++++---------- Kernel/CoreDump.h | 10 ++++---- Kernel/Process.cpp | 8 ++++-- Kernel/Syscalls/profiling.cpp | 10 +++++--- 4 files changed, 52 insertions(+), 24 deletions(-) diff --git a/Kernel/CoreDump.cpp b/Kernel/CoreDump.cpp index 9d8e39637a4..7a1d8cbe940 100644 --- a/Kernel/CoreDump.cpp +++ b/Kernel/CoreDump.cpp @@ -84,7 +84,7 @@ RefPtr CoreDump::create_target_file(const Process& process, con return fd_or_error.value(); } -void CoreDump::write_elf_header() +KResult CoreDump::write_elf_header() { Elf32_Ehdr elf_file_header; elf_file_header.e_ident[EI_MAG0] = 0x7f; @@ -116,10 +116,13 @@ void CoreDump::write_elf_header() elf_file_header.e_shnum = 0; elf_file_header.e_shstrndx = SHN_UNDEF; - [[maybe_unused]] auto rc = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast(&elf_file_header)), sizeof(Elf32_Ehdr)); + auto result = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast(&elf_file_header)), sizeof(Elf32_Ehdr)); + if (result.is_error()) + return result.error(); + return KSuccess; } -void CoreDump::write_program_headers(size_t notes_size) +KResult CoreDump::write_program_headers(size_t notes_size) { size_t offset = sizeof(Elf32_Ehdr) + m_num_program_headers * sizeof(Elf32_Phdr); for (auto& region : m_process.m_regions) { @@ -155,10 +158,13 @@ void CoreDump::write_program_headers(size_t notes_size) notes_pheader.p_align = 0; notes_pheader.p_flags = 0; - [[maybe_unused]] auto rc = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast(¬es_pheader)), sizeof(Elf32_Phdr)); + auto result = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast(¬es_pheader)), sizeof(Elf32_Phdr)); + if (result.is_error()) + return result.error(); + return KSuccess; } -void CoreDump::write_regions() +KResult CoreDump::write_regions() { for (auto& region : m_process.m_regions) { if (region.is_kernel()) @@ -182,14 +188,20 @@ void CoreDump::write_regions() // (A page may not be backed by a physical page because it has never been faulted in when the process ran). src_buffer = UserOrKernelBuffer::for_kernel_buffer(zero_buffer); } - [[maybe_unused]] auto rc = m_fd->write(src_buffer.value(), PAGE_SIZE); + auto result = m_fd->write(src_buffer.value(), PAGE_SIZE); + if (result.is_error()) + return result.error(); } } + return KSuccess; } -void CoreDump::write_notes_segment(ByteBuffer& notes_segment) +KResult CoreDump::write_notes_segment(ByteBuffer& notes_segment) { - [[maybe_unused]] auto rc = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(notes_segment.data()), notes_segment.size()); + auto result = m_fd->write(UserOrKernelBuffer::for_kernel_buffer(notes_segment.data()), notes_segment.size()); + if (result.is_error()) + return result.error(); + return KSuccess; } ByteBuffer CoreDump::create_notes_threads_data() const @@ -253,18 +265,26 @@ ByteBuffer CoreDump::create_notes_segment_data() const return notes_buffer; } -void CoreDump::write() +KResult CoreDump::write() { ProcessPagingScope scope(m_process); ByteBuffer notes_segment = create_notes_segment_data(); - write_elf_header(); - write_program_headers(notes_segment.size()); - write_regions(); - write_notes_segment(notes_segment); + auto result = write_elf_header(); + if (result.is_error()) + return result; + result = write_program_headers(notes_segment.size()); + if (result.is_error()) + return result; + result = write_regions(); + if (result.is_error()) + return result; + result = write_notes_segment(notes_segment); + if (result.is_error()) + return result; - [[maybe_unused]] auto rc = m_fd->chmod(0400); // Make coredump file readable + return m_fd->chmod(0400); // Make coredump file readable } } diff --git a/Kernel/CoreDump.h b/Kernel/CoreDump.h index 2b975a6e254..354008ff577 100644 --- a/Kernel/CoreDump.h +++ b/Kernel/CoreDump.h @@ -42,16 +42,16 @@ public: static OwnPtr create(Process&, const String& output_path); ~CoreDump(); - void write(); + [[nodiscard]] KResult write(); private: CoreDump(Process&, NonnullRefPtr&&); static RefPtr create_target_file(const Process&, const String& output_path); - void write_elf_header(); - void write_program_headers(size_t notes_size); - void write_regions(); - void write_notes_segment(ByteBuffer&); + [[nodiscard]] KResult write_elf_header(); + [[nodiscard]] KResult write_program_headers(size_t notes_size); + [[nodiscard]] KResult write_regions(); + [[nodiscard]] KResult write_notes_segment(ByteBuffer&); ByteBuffer create_notes_segment_data() const; ByteBuffer create_notes_threads_data() const; diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 7657c0fc663..8f85d2f0991 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -590,7 +590,9 @@ void Process::finalize() if (is_profiling()) { auto coredump = CoreDump::create(*this, String::formatted("/tmp/profiler_coredumps/{}", pid().value())); if (coredump) { - coredump->write(); + auto result = coredump->write(); + if (result.is_error()) + dbgln("Core dump generation failed: {}", result.error()); } else { dbgln("Could not create coredump"); } @@ -601,7 +603,9 @@ void Process::finalize() auto coredump_path = String::formatted("/tmp/coredump/{}_{}_{}", name(), m_pid.value(), RTC::now()); auto coredump = CoreDump::create(*this, coredump_path); if (coredump) { - coredump->write(); + auto result = coredump->write(); + if (result.is_error()) + dbgln("Core dump generation failed: {}", result.error()); } else { dbgln("Could not create coredump"); } diff --git a/Kernel/Syscalls/profiling.cpp b/Kernel/Syscalls/profiling.cpp index 10a11a9dd60..9e9ba0c56bd 100644 --- a/Kernel/Syscalls/profiling.cpp +++ b/Kernel/Syscalls/profiling.cpp @@ -60,10 +60,14 @@ int Process::sys$profiling_disable(pid_t pid) // We explicitly unlock here because we can't hold the lock when writing the coredump VFS lock.unlock(); - if (auto coredump = CoreDump::create(*process, String::formatted("/tmp/profiler_coredumps/{}", pid))) - coredump->write(); - else + if (auto coredump = CoreDump::create(*process, String::formatted("/tmp/profiler_coredumps/{}", pid))) { + auto result = coredump->write(); + if (result.is_error()) + return result.error(); + } else { + // FIXME: Return an error maybe? dbgln("Unable to create profiler coredump for PID {}", pid); + } return 0; }