Kernel+LibCoredump: Write spec-compliant PT_NOTE sections in coredumps

Note sections should start with a header containing information like the
name, a type and the size of the remaining bytes in the section. This
makes `readelf` stop complaining about invalids notes.

We also need to teach LibCoreDump to read these sections at the same
time to avoid regressions.

Kernel/Tasks/Coredump.cpp

@@ -377,10 +377,33 @@ ErrorOr<void> Coredump::create_notes_metadata_data(auto& builder) const
     return {};
 }
 
+static ErrorOr<void> add_elf_note_header(KBufferBuilder& builder)
+{
+    // https://gabi.xinuos.com/elf/07-pheader.html#note-sections
+
+    static constexpr StringView name = "SerenityOS\0"sv;
+    Elf_Note note;
+    note.namesz = name.length();
+    note.type = 0; // Arbitrary chosen, we already have our own `type` field in NotesEntryHeader.
+
+    static_assert(sizeof(Elf_Note) == 12);
+    TRY(builder.append_bytes(ReadonlyBytes { &note, sizeof(note) }));
+
+    // names and desc are 4-bytes aligned.
+    Array<u8, 3> zero_padding {};
+    TRY(builder.append(name));
+    TRY(builder.append_bytes(zero_padding.span().trim(mod(-name.length(), 4))));
+
+    return {};
+}
+
 ErrorOr<OwnPtr<KBuffer>> Coredump::create_notes_segment_data() const
 {
     auto builder = TRY(KBufferBuilder::try_create());
 
+    TRY(add_elf_note_header(builder));
+    auto header_offset = builder.length();
+
     TRY(create_notes_process_data(builder));
     TRY(create_notes_threads_data(builder));
     TRY(create_notes_regions_data(builder));
@@ -390,6 +413,13 @@ ErrorOr<OwnPtr<KBuffer>> Coredump::create_notes_segment_data() const
     null_entry.type = ELF::Core::NotesEntryHeader::Type::Null;
     TRY(builder.append(ReadonlyBytes { &null_entry, sizeof(null_entry) }));
 
+    auto desc_size = builder.length() - header_offset;
+    auto note_header = reinterpret_cast<Elf_Note*>(builder.bytes().data());
+    note_header->descsz = desc_size;
+
+    Array<u8, 3> padding_bytes {};
+    TRY(builder.append_bytes(padding_bytes.span().trim(mod(-desc_size, 4))));
+
     return builder.build();
 }
 

Userland/Libraries/LibCoredump/Reader.cpp

@@ -18,6 +18,11 @@
 #include <string.h>
 #include <unistd.h>
 
+template<>
+struct AK::Traits<Elf64_Note> : public AK::DefaultTraits<Elf64_Note> {
+    static constexpr bool is_trivially_serializable() { return true; }
+};
+
 namespace Coredump {
 
 OwnPtr<Reader> Reader::create(StringView path)
@@ -133,6 +138,23 @@ bool Reader::NotesEntryIterator::at_end() const
     return type() == ELF::Core::NotesEntryHeader::Type::Null;
 }
 
+ReadonlyBytes Reader::get_serenity_note_section() const
+{
+    auto const& program_header = m_coredump_image.program_header(m_notes_segment_index);
+    auto note_section_bytes = ReadonlyBytes { program_header.raw_data(), program_header.size_in_memory() };
+    FixedMemoryStream section_stream { note_section_bytes };
+
+    auto note = section_stream.read_value<Elf_Note>().release_value_but_fixme_should_propagate_errors();
+    auto name = section_stream.read_in_place<u8 const>(note.namesz);
+
+    VERIFY(note.type == 0);
+    VERIFY(name.release_value_but_fixme_should_propagate_errors() == "SerenityOS\0"sv.bytes());
+
+    section_stream.discard(mod(-note.namesz, 4)).release_value_but_fixme_should_propagate_errors();
+
+    return section_stream.read_in_place<u8 const>(note.descsz).release_value_but_fixme_should_propagate_errors();
+}
+
 Optional<FlatPtr> Reader::peek_memory(FlatPtr address) const
 {
     auto region = region_containing(address);
@@ -149,7 +171,7 @@ Optional<FlatPtr> Reader::peek_memory(FlatPtr address) const
 JsonObject const Reader::process_info() const
 {
     const ELF::Core::ProcessInfo* process_info_notes_entry = nullptr;
-    NotesEntryIterator it(bit_cast<u8 const*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    NotesEntryIterator it(get_serenity_note_section().data());
     for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::ProcessInfo)
             continue;
@@ -248,7 +270,7 @@ Vector<ByteString> Reader::process_environment() const
 HashMap<ByteString, ByteString> Reader::metadata() const
 {
     const ELF::Core::Metadata* metadata_notes_entry = nullptr;
-    NotesEntryIterator it(bit_cast<u8 const*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    NotesEntryIterator it(get_serenity_note_section().data());
     for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::Metadata)
             continue;

Userland/Libraries/LibCoredump/Reader.h

@@ -102,6 +102,8 @@ private:
         u8 const* start { nullptr };
     };
 
+    ReadonlyBytes get_serenity_note_section() const;
+
     // Private as we don't need anyone poking around in this JsonObject
     // manually - we know very well what should be included and expose that
     // as getters with the appropriate (non-JsonValue) types.
@@ -122,7 +124,7 @@ private:
 template<typename Func>
 void Reader::for_each_memory_region_info(Func func) const
 {
-    NotesEntryIterator it(bit_cast<u8 const*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    NotesEntryIterator it(get_serenity_note_section().data());
     for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo)
             continue;
@@ -150,7 +152,7 @@ void Reader::for_each_memory_region_info(Func func) const
 template<typename Func>
 void Reader::for_each_thread_info(Func func) const
 {
-    NotesEntryIterator it(bit_cast<u8 const*>(m_coredump_image.program_header(m_notes_segment_index).raw_data()));
+    NotesEntryIterator it(get_serenity_note_section().data());
     for (; !it.at_end(); it.next()) {
         if (it.type() != ELF::Core::NotesEntryHeader::Type::ThreadInfo)
             continue;