Open ELF core dumps with more than 64K sections

[EugeneBi]

Summary

Problem:

There are three filelds in the ELF header - e_phnum, e_shnum, and e_shstrndx -
that could be bigger than 64K and therefore do not fit in 16 bits reserved for
them in the header. If this happens, pretty often there is a special section at
index 0 which contains their real values for these fields in the section header
in the fields sh_info, sh_size, and sh_link respectively.

Fix:

• Rename original fields in the header declaration. We want to have them around

just in case.

• Reintroduce these fields as 32-bit members at the end of the header. By default

they are initialized from the header in Parse() method.

• In Parse(), detect the situation when the header might have been extended into

section info #0 and try to read it from the same data source.

• ObjectFileELF::GetModuleSpecifications accesses some of these fields but the

original parse uses too small data source. Re-parse the header if necessary
using bigger data source.

• ProcessElfCore::CreateInstance uses header with potentially sentinel values,

but it does not access these fields, so a comment here is enough.

[labath]

The changes themselves look fine. However:

• this diff seems to be based on some very old snapshot, as it still uses the old 4-char indentation and everything. Please rebase the change on current master.
• have given any thoughts on how to test this? It sounds like testing the whole pipeline would be quite hard. But we could at least add a unit test for the ELFHeader::Parse function. It seems quite doable - just create some elf-looking data in memory and pass it to this function. Currently there is no unittests/ObjectFile/ELF folder, so please create one and put the test there.

source/Plugins/ObjectFile/ELF/ELFHeader.h

56–57

I am wondering whether these is any use in keeping these old values. It sounds like it's a recipe for someone getting confused and using the wrong ones. What do you think about just deleting these?

[EugeneBi]

1. Yes, this is based on release_39 branch. OK, I'll rebase it on current master.
2. Sorry, I am not familiar with LLDB testing and unfortunately I do not have time to do it. I am testing it on the core dumps I have. If this is not acceptable, I'll keep using my private copy and wait until somebody with more resources to spend on LLDB picks it up.
3. I thought about keeping vs. discarding values from the original header. I slightly inclined to keep them because:

• They are not necessary if everything works, but if something goes wrong it could be useful to see in the debugger what was the original header
• They are used in HasHeaderExtension method which is quite simple right now, I do not want it get complicated.
• To avoid confusion, I added "_hdr" suffix to the original values. Well, we can invent better name - maybe "_orig" or what?

[EugeneBi]

Rebased to recent master

[EugeneBi]

Sorry, messed up.
Will upload correct change soon

[EugeneBi]

Hmm... This fix does not work with the recent sources :(
I'll debug some more tomorrow.

source/Plugins/ObjectFile/ELF/ELFHeader.h

67

Accidentally deleted this line

[EugeneBi]

This compiles, builds, and run.

Unfortunately, unlike release_39 branch, I cannot open my core dump, but the problem seems unrelated.
So I think I want to push it through and continue investigation what is not working this time.

[EugeneBi]

source/Plugins/ObjectFile/ELF/ELFHeader.h

56–57

I will leave it to you - just tell me what you prefer. I see both pros and cons of my current code.

[labath]

Add a unittest for the header parse function

[labath]

I've added a quick test to demonstrate what I had in mind.

Unfortunately, unlike release_39 branch, I cannot open my core dump, but the problem seems unrelated. So I think I want to push it through and continue investigation what is not working this time.

That's kinda the reason I wanted to add tests. :)
This test would not have actually caught your new problem, but it could add at least some protection for the future, as I doubt many people will run into real files with this many sections.

I think this should be ok to put in if there are no objections from anyone else.

source/Plugins/ObjectFile/ELF/ELFHeader.h

56–57

Ok, let's leave it this way then.

[davidb]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

108

Would this make more sense to compare against a named constant ELF::PN_XNUM?

110

And this with SHN_UNDEF?

112

and this with ELF::SHN_XINDEX.

[EugeneBi]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

108

Would be nice, but - where is it defined? I tried ELF::PN_XNUM, elf::PN_XNUM, PN_XNUM - compiler barks anyway.

[EugeneBi]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

108

OK, I see the other two are defined in Support/ELF.h, I'll put PN_XNUM there then.

[EugeneBi]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

108

Oh, this ELF.h is a part of a different git repo - it lives in llvm. I am not sure what to do about it.

[EugeneBi]

Used named constants for SHN_UNDEF and SHN_XINDEX sentinels.
Unfortunately ELF.h lacks definition of PN_XNUM, so left it as a comment.

[EugeneBi]

So, what's now? Can somebody commit it, please?
I do not see any option to do it myself.

[labath]

Sorry, I have been a bit busy.

I've committed this in now. Thank you for the patch.

source/Plugins/ObjectFile/ELF/ELFHeader.h

 typedef int64_t elf_sxword;
 //@}
 
 //------------------------------------------------------------------------------
 /// @class ELFHeader
 /// @brief Generic representation of an ELF file header.
 ///
 /// This object is used to identify the general attributes on an ELF file and to
 /// locate additional sections within the file.
 struct ELFHeader {
   unsigned char e_ident[llvm::ELF::EI_NIDENT]; ///< ELF file identification.
   elf_addr e_entry;     ///< Virtual address program entry point.
   elf_off e_phoff;      ///< File offset of program header table.
   elf_off e_shoff;      ///< File offset of section header table.
   elf_word e_flags;     ///< Processor specific flags.
   elf_word e_version;   ///< Version of object file (always 1).
   elf_half e_type;      ///< Object file type.
   elf_half e_machine;   ///< Target architecture.
   elf_half e_ehsize;    ///< Byte size of the ELF header.
   elf_half e_phentsize; ///< Size of a program header table entry.
-  elf_half e_phnum;     ///< Number of program header entries.
+  elf_half e_phnum_hdr; ///< Number of program header entries.
   elf_half e_shentsize; ///< Size of a section header table entry.
-  elf_half e_shnum;     ///< Number of section header entries.
-  elf_half e_shstrndx;  ///< String table section index.
+  elf_half e_shnum_hdr; ///< Number of section header entries.
+  elf_half e_shstrndx_hdr; ///< String table section index.
+
+  // In some cases these numbers do not fit in 16 bits and they are
+  // stored outside of the header in section #0. Here are the actual
+  // values.
+  elf_word e_phnum;     ///< Number of program header entries.
+  elf_word e_shnum;     ///< Number of section header entries.
+  elf_word e_shstrndx;  ///< String table section index.
 
   ELFHeader();
 
   //--------------------------------------------------------------------------
   /// Returns true if this is a 32 bit ELF file header.
   ///
   /// @return
   ///    True if this is a 32 bit ELF file header.
struct ELFHeader {
   ///    The byte order of this ELF file as described by the header.
   lldb::ByteOrder GetByteOrder() const;
 
   //--------------------------------------------------------------------------
   /// The jump slot relocation type of this ELF.
   unsigned GetRelocationJumpSlotType() const;
 
   //--------------------------------------------------------------------------
+  /// Check if there should be header extension in section header #0
+  ///
+  /// @return
+  ///    True if parsing the ELFHeader requires reading header extension
+  ///    and false otherwise.
+  bool HasHeaderExtension() const;
+
+  //--------------------------------------------------------------------------
   /// Parse an ELFHeader entry starting at position \p offset and
   /// update the data extractor with the address size and byte order
   /// attributes as defined by the header.
   ///
   /// @param[in,out] data
   ///    The DataExtractor to read from.  Updated with the address size and
   ///    byte order attributes appropriate to this header.
   ///
struct ELFHeader {
   /// determines the address size of the underlying ELF file.  This function
   /// should only be called on an pointer for which MagicBytesMatch returns
   /// true.
   ///
   /// @return
   ///    The number of bytes forming an address in the ELF file (either 4 or
   ///    8), else zero if the address size could not be determined.
   static unsigned AddressSizeInBytes(const uint8_t *magic);
+
+private:
+
+  //--------------------------------------------------------------------------
+  /// Parse an ELFHeader header extension entry.  This method is called
+  /// by Parse().
+  ///
+  /// @param[in] data
+  ///    The DataExtractor to read from.
+  void ParseHeaderExtension(lldb_private::DataExtractor &data);
 };
 
 //------------------------------------------------------------------------------
 /// @class ELFSectionHeader
 /// @brief Generic representation of an ELF section header.
 struct ELFSectionHeader {
   elf_word sh_name;       ///< Section name string index.
   elf_word sh_type;       ///< Section type.

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

 ByteOrder ELFHeader::GetByteOrder() const {
   if (e_ident[EI_DATA] == ELFDATA2MSB)
     return eByteOrderBig;
   if (e_ident[EI_DATA] == ELFDATA2LSB)
     return eByteOrderLittle;
   return eByteOrderInvalid;
 }
 
+bool ELFHeader::HasHeaderExtension() const {
+  bool result = false;
+
+  // Check if any of these values looks like sentinel.
+  result |= e_phnum_hdr == 0xFFFF;
+  result |= e_shnum_hdr == 0;
+  result |= e_shstrndx_hdr == 0xFFFF;
+
+  // If header extension is present, the section offset cannot be null.
+  result &= e_shoff != 0;
+
+  // Done.
+  return result;
+}
+
+void ELFHeader::ParseHeaderExtension(lldb_private::DataExtractor &data) {
+  // Extract section #0 header.
+  ELFSectionHeader section_zero;
+  lldb::offset_t offset = 0;
+  lldb_private::DataExtractor sh_data(data, e_shoff, e_shentsize);
+  bool ok = section_zero.Parse(sh_data, &offset);
+
+  // If we succeeded, fix the header.
+  if (ok) {
+    if (e_phnum_hdr == 0xFFFF)
+      e_phnum = section_zero.sh_info;
+    if (e_shnum_hdr == 0)
+      e_shnum = section_zero.sh_size;
+    if (e_shstrndx_hdr == 0xFFFF)
+      e_shstrndx = section_zero.sh_link;
+  }
+}
+
 bool ELFHeader::Parse(lldb_private::DataExtractor &data,
                       lldb::offset_t *offset) {
   // Read e_ident.  This provides byte order and address size info.
   if (data.GetU8(offset, &e_ident, EI_NIDENT) == NULL)
     return false;
 
   const unsigned byte_size = Is32Bit() ? 4 : 8;
   data.SetByteOrder(GetByteOrder());
bool ELFHeader::Parse(lldb_private::DataExtractor &data,
   if (data.GetU32(offset, &e_flags, 1) == NULL)
     return false;
 
   // Read e_ehsize, e_phentsize, e_phnum, e_shentsize, e_shnum and
   // e_shstrndx.
   if (data.GetU16(offset, &e_ehsize, 6) == NULL)
     return false;
 
+  // Initialize e_phnum, e_shnum, and e_shstrndx with the values
+  // read from the header.
+  e_phnum = e_phnum_hdr;
+  e_shnum = e_shnum_hdr;
+  e_shstrndx = e_shstrndx_hdr;
+
+  // See if we have extended header in section #0.
+  if (HasHeaderExtension())
+    ParseHeaderExtension(data);
+
   return true;
 }
 
 bool ELFHeader::MagicBytesMatch(const uint8_t *magic) {
   return memcmp(magic, ElfMagic, strlen(ElfMagic)) == 0;
 }
 
 unsigned ELFHeader::AddressSizeInBytes(const uint8_t *magic) {

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

size_t ObjectFileELF::GetModuleSpecifications(
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_MODULES));
 
   const size_t initial_count = specs.GetSize();
 
   if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize())) {
     DataExtractor data;
     data.SetData(data_sp);
     elf::ELFHeader header;
-    if (header.Parse(data, &data_offset)) {
+    lldb::offset_t header_offset = data_offset;
+    if (header.Parse(data, &header_offset)) {
       if (data_sp) {
         ModuleSpec spec(file);
 
         const uint32_t sub_type = subTypeFromElfHeader(header);
         spec.GetArchitecture().SetArchitecture(
             eArchTypeELF, header.e_machine, sub_type, header.e_ident[EI_OSABI]);
 
         if (spec.GetArchitecture().IsValid()) {
if (header.Parse(data, &header_offset)) {
           assert(spec_ostype == ostype);
           if (spec_ostype != llvm::Triple::OSType::UnknownOS) {
             if (log)
               log->Printf("ObjectFileELF::%s file '%s' set ELF module OS type "
                           "from ELF header OSABI.",
                           __FUNCTION__, file.GetPath().c_str());
           }
 
+          // In case there is header extension in the section #0, the header
+          // we parsed above could have sentinel values for e_phnum, e_shnum,
+          // and e_shstrndx.  In this case we need to reparse the header
+          // with a bigger data source to get the actual values.
+          size_t section_header_end = header.e_shoff + header.e_shentsize;
+          if (header.HasHeaderExtension() &&
+            section_header_end > data_sp->GetByteSize()) {
+            data_sp = file.MemoryMapFileContentsIfLocal (file_offset,
+                                                         section_header_end);
+            data.SetData(data_sp);
+            lldb::offset_t header_offset = data_offset;
+            header.Parse(data, &header_offset);
+          }
+
           // Try to get the UUID from the section list. Usually that's at the
           // end, so
           // map the file in if we don't have it already.
-          size_t section_header_end =
+          section_header_end =
               header.e_shoff + header.e_shnum * header.e_shentsize;
           if (section_header_end > data_sp->GetByteSize()) {
             data_sp = file.MemoryMapFileContentsIfLocal(file_offset,
                                                         section_header_end);
             data.SetData(data_sp);
           }
 
           uint32_t gnu_debuglink_crc = 0;
void ObjectFileELF::DumpELFHeader(Stream *s, const ELFHeader &header) {
   s->Printf("\ne_machine   = 0x%4.4x\n", header.e_machine);
   s->Printf("e_version   = 0x%8.8x\n", header.e_version);
   s->Printf("e_entry     = 0x%8.8" PRIx64 "\n", header.e_entry);
   s->Printf("e_phoff     = 0x%8.8" PRIx64 "\n", header.e_phoff);
   s->Printf("e_shoff     = 0x%8.8" PRIx64 "\n", header.e_shoff);
   s->Printf("e_flags     = 0x%8.8x\n", header.e_flags);
   s->Printf("e_ehsize    = 0x%4.4x\n", header.e_ehsize);
   s->Printf("e_phentsize = 0x%4.4x\n", header.e_phentsize);
-  s->Printf("e_phnum     = 0x%4.4x\n", header.e_phnum);
+  s->Printf("e_phnum     = 0x%8.8x\n", header.e_phnum);
   s->Printf("e_shentsize = 0x%4.4x\n", header.e_shentsize);
-  s->Printf("e_shnum     = 0x%4.4x\n", header.e_shnum);
-  s->Printf("e_shstrndx  = 0x%4.4x\n", header.e_shstrndx);
+  s->Printf("e_shnum     = 0x%8.8x\n", header.e_shnum);
+  s->Printf("e_shstrndx  = 0x%8.8x\n", header.e_shstrndx);
 }
 
 //----------------------------------------------------------------------
 // DumpELFHeader_e_type
 //
 // Dump an token value for the ELF header member e_type
 //----------------------------------------------------------------------
 void ObjectFileELF::DumpELFHeader_e_type(Stream *s, elf_half e_type) {

source/Plugins/Process/elf-core/ProcessElfCore.cpp

 }
 
 lldb::ProcessSP ProcessElfCore::CreateInstance(lldb::TargetSP target_sp,
                                                lldb::ListenerSP listener_sp,
                                                const FileSpec *crash_file) {
   lldb::ProcessSP process_sp;
   if (crash_file) {
     // Read enough data for a ELF32 header or ELF64 header
+    // Note: Here we care about e_type field only, so it is safe
+    // to ignore possible presence of the header extension.
     const size_t header_size = sizeof(llvm::ELF::Elf64_Ehdr);
 
     lldb::DataBufferSP data_sp(crash_file->ReadFileContents(0, header_size));
     if (data_sp && data_sp->GetByteSize() == header_size &&
         elf::ELFHeader::MagicBytesMatch(data_sp->GetBytes())) {
       elf::ELFHeader elf_header;
       DataExtractor data(data_sp, lldb::eByteOrderLittle, 4);
       lldb::offset_t data_offset = 0;