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

71

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

58–59

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

71

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

71

Ok, let's leave it this way then.

[davidb]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

240

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

242

And this with SHN_UNDEF?

244

and this with ELF::SHN_XINDEX.

[EugeneBi]

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

240

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

240

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

240

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
     GetByteOrder() const;
 
     //--------------------------------------------------------------------------
     /// The jump slot relocation type of this ELF.
     unsigned
     GetRelocationJumpSlotType() const;
 
     //--------------------------------------------------------------------------
+    /// Check if there should be header extension in section header #0
+    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
     /// 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.

source/Plugins/ObjectFile/ELF/ELFHeader.cpp

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());
ELFHeader::Parse(lldb_private::DataExtractor &data, lldb::offset_t *offset)
     // Read e_entry, e_phoff and e_shoff.
     if (GetMaxU64(data, offset, &e_entry, byte_size, 3) == false)
         return false;
 
     // Read e_flags.
     if (data.GetU32(offset, &e_flags, 1) == NULL)
         return false;
 
-    // Read e_ehsize, e_phentsize, e_phnum, e_shentsize, e_shnum and
-    // e_shstrndx.
+    // Read e_ehsize, e_phentsize, e_phnum_hdr, e_shentsize,
+    // e_shnum_hdr and e_shstrndx_hdr.
     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;
 }
ELFHeader::GetRelocationJumpSlotType() const
     case EM_386:
     case EM_IAMCU: // FIXME: is this correct?
         slot = R_386_JUMP_SLOT;
         break;
     case EM_X86_64:
         slot = R_X86_64_JUMP_SLOT;
         break;
     case EM_ARM:
         slot = R_ARM_JUMP_SLOT;
         break;
     case EM_HEXAGON:
         slot = R_HEX_JMP_SLOT;
         break;
     case EM_AARCH64:
         slot = R_AARCH64_JUMP_SLOT;
         break;
     case EM_MIPS:
         slot = R_MIPS_JUMP_SLOT;
         break;
     case EM_S390:
         slot = R_390_JMP_SLOT;

source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

ObjectFileELF::GetModuleSpecifications (const lldb_private::FileSpec& file,
 
     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,
if (ObjectFileELF::MagicBytesMatch(data_sp, 0, data_sp->GetByteSize()))
                     GetOsFromOSABI (header.e_ident[EI_OSABI], ostype);
                     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 = header.e_shoff + header.e_shnum * header.e_shentsize;
+                    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;
                     std::string gnu_debuglink_file;
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

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);