[LLDB] Add FreeBSD kernel coredump matching check.

[aokblast]

This patch add check for the UUID match between kernel and coredump to prevent developer falsely load kernel with unmatched coredump.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (aokblast)

Changes

This patch add check for the UUID match between kernel and coredump to prevent developer falsely load kernel with unmatched coredump.

---

Full diff: https://github.com/llvm/llvm-project/pull/80785.diff

4 Files Affected:

• (modified) lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp (+25-4)
• (modified) lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h (+2-1)
• (modified) lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (+29)
• (modified) lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h (+3)

diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
index e504e6cbf6921..5e5e2dd2cbf75 100644
--- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
+++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
@@ -31,6 +31,8 @@
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 
 #include "DynamicLoaderFreeBSDKernel.h"
+#include "lldb/Utility/UUID.h"
+#include "llvm/BinaryFormat/ELF.h"
 #include <memory>
 #include <mutex>
 
@@ -153,9 +155,10 @@ addr_t DynamicLoaderFreeBSDKernel::FindKernelAtLoadAddress(
 }
 
 // Read ELF header from memry and return
+template <typename Elf_Ehdr>
 bool DynamicLoaderFreeBSDKernel::ReadELFHeader(Process *process,
                                                lldb::addr_t addr,
-                                               llvm::ELF::Elf32_Ehdr &header,
+                                               Elf_Ehdr &header,
                                                bool *read_error) {
   Status error;
   if (read_error)
@@ -200,8 +203,19 @@ lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress(
   if (header.e_type != llvm::ELF::ET_EXEC)
     return UUID();
 
-  ModuleSP memory_module_sp =
-      process->ReadModuleFromMemory(FileSpec("temp_freebsd_kernel"), addr);
+  ArchSpec kernel_arch(llvm::ELF::convertEMachineToArchName(header.e_machine));
+
+  ModuleSP memory_module_sp = process->ReadModuleFromMemory(
+      FileSpec("temp_freebsd_kernel"), addr, header.e_shoff);
+  if (header.e_ident[llvm::ELF::EI_CLASS] == llvm::ELF::ELFCLASS64) {
+    llvm::ELF::Elf64_Ehdr header;
+    if (!ReadELFHeader(process, addr, header)) {
+      *read_error = true;
+      return UUID();
+    }
+    memory_module_sp = process->ReadModuleFromMemory(
+        FileSpec("temp_freebsd_kernel"), addr, header.e_shoff);
+  }
 
   if (!memory_module_sp.get()) {
     *read_error = true;
@@ -221,7 +235,14 @@ lldb_private::UUID DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress(
   // In here, I should check is_kernel for memory_module_sp
   // However, the ReadModuleFromMemory reads wrong section so that this check
   // will failed
-  ArchSpec kernel_arch(llvm::ELF::convertEMachineToArchName(header.e_machine));
+  memory_module_sp->GetObjectFile()->SetType(ObjectFile::eTypeCoreFile);
+
+  if (memory_module_sp->GetUUID() !=
+      process->GetTarget().GetExecutableModule()->GetUUID()) {
+    LLDB_LOGF(log, "DynamicLoaderFreeBSDKernel::CheckForKernelImageAtAddress "
+                   "Cannot match coredump with binary by build-id");
+    return UUID();
+  }
 
   if (!process->GetTarget().GetArchitecture().IsCompatibleMatch(kernel_arch))
     process->GetTarget().SetArchitecture(kernel_arch);
diff --git a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h
index d8656e9c49dfe..bce0837aa4df4 100644
--- a/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h
+++ b/lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.h
@@ -148,8 +148,9 @@ class DynamicLoaderFreeBSDKernel : public lldb_private::DynamicLoader {
 
   static lldb::addr_t FindKernelAtLoadAddress(lldb_private::Process *process);
 
+  template <typename Elf_Ehdr>
   static bool ReadELFHeader(lldb_private::Process *process,
-                            lldb::addr_t address, llvm::ELF::Elf32_Ehdr &header,
+                            lldb::addr_t address, Elf_Ehdr &header,
                             bool *read_error = nullptr);
 
   lldb_private::Process *m_process;
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 0d95a1c12bde3..3b047e3d5c759 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -795,6 +795,32 @@ bool ObjectFileELF::ParseHeader() {
   return m_header.Parse(m_data, &offset);
 }
 
+UUID ObjectFileELF::GetUUIDByNoteSection() {
+  if (!ParseProgramHeaders())
+    return UUID();
+  for (const ELFProgramHeader &H : m_program_headers) {
+    if (H.p_type == llvm::ELF::PT_NOTE) {
+      const elf_off ph_offset = H.p_offset;
+      const size_t ph_size = H.p_filesz;
+
+      DataExtractor segment_data;
+      if (segment_data.SetData(m_data, ph_offset, ph_size) != ph_size) {
+        // The ELF program header contained incorrect data, probably corefile
+        // is incomplete or corrupted.
+        break;
+      }
+
+      UUID uuid;
+      ArchSpec arch;
+
+      if (RefineModuleDetailsFromNote(segment_data, arch, uuid).Success())
+        return uuid;
+    }
+  }
+
+  return UUID();
+}
+
 UUID ObjectFileELF::GetUUID() {
   // Need to parse the section list to get the UUIDs, so make sure that's been
   // done.
@@ -809,6 +835,9 @@ UUID ObjectFileELF::GetUUID() {
       if (!ParseProgramHeaders())
         return UUID();
 
+      if ((m_uuid = GetUUIDByNoteSection()).IsValid())
+        return m_uuid;
+
       core_notes_crc =
           CalculateELFNotesSegmentsCRC32(m_program_headers, m_data);
 
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
index bc8e34981a9d8..6954bd96b753a 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h
@@ -272,6 +272,9 @@ class ObjectFileELF : public lldb_private::ObjectFile {
                                      uint32_t &gnu_debuglink_crc,
                                      lldb_private::ArchSpec &arch_spec);
 
+  // Use Note Section to get uuid.
+  lldb_private::UUID GetUUIDByNoteSection();
+
   /// Scans the dynamic section and locates all dependent modules (shared
   /// libraries) populating m_filespec_up.  This method will compute the
   /// dependent module list only once.  Returns the number of dependent

[github-actions[bot]]

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

[aokblast]

All of the problems are fixed.

However, FreeBSD doesn't load PT_NOTE segment originally and I add it yesterday in this patch with this LLDB patch at the same time. Currently, FreeBSD phabricator has no response yet. I think we should land the patch in FreeBSD first or the LLDB will failed in the UUID check.

Maybe I can tag some guys in FreeBSD like @emaste to check if the patch in FreeBSD make senses?

[DavidSpickett]

Thanks, the logic is clearer now.

Should this check be limited to only when a uuid is actually found? This would allow debugging of older FreeBSDs with a newer lldb.

[aokblast]

Oh, yes, you are right. I am just thinking about the same thing. XD

[DavidSpickett]

Seems like adding a test for this would be difficult but if you can think of a way that would be great. Otherwise this looks ok to me, @emaste should approve as the code owner for FreeBSD.

[aokblast]

Sorry but please wait, there is still some problems. FreeBSD kernel has several types of dump. Currently, this check works on /dev/mem and full memory dump. However, for minimal dump, the build-id section won't loaded because minimal dump only dump the used memory by kernel. I'm still figuring out for this problem or the minimal dump cannot use the dynamic loader plugin for FreeBSD kernel. That's why I delete the previous comment.

[emaste]

However, for minimal dump, the build-id section won't loaded because minimal dump only dump the used memory by kernel.

If we fall back to no-UUID-found in that case and don't require that it matches this is still an improvement, yeah?

[aokblast]

I extend this feature to all kernel module in here. Then I will write the corresponding code for LLDB later. Since kernel is also in linker_files structure. So I convinced that it can fix the problem.