[ELF] - implement support of extended length field for CIE/FDE records of eh_frame.

[grimar]

Summary

Problem here in the next part of code:

void EHOutputSection<ELFT>::addSectionAux(
    EHInputSection<ELFT> *S,
    iterator_range<const Elf_Rel_Impl<ELFT, IsRela> *> Rels) {
...
    uint32_t Length = read32<E>(D.data());
    Length += 4;
...

Ian Lance Taylor writes: "Read 4 bytes. If they are not 0xffffffff, they are the length of the CIE or FDE record. Otherwise the next 64 bits holds the length, and this is a 64-bit DWARF format. This is like .debug_frame." (http://www.airs.com/blog/archives/460), that also consistent with spec (https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-PDA/LSB-PDA/ehframechpt.html).

When length was 0xffffffff overflow happened and code executed forward without any error here, failing much later.
Patch implements support of described extended length field and also adds few more checks for safety.

[emaste]

Looks reasonable. How about a test for successful Is64Dwarf though?

[ruiu]

ELF/OutputSections.cpp

1001–1002 ↗

(On Diff #42863)

Can you return early if it's 32 bit?

uint64_t Len = read32<E>(D.data());
if (Len < UINT32_MAX) {
  if (Len > D.size())
    error("CIE/FIE ends past the end of the section");
  return Len + 4;
}
if (D.size() < 12)
  error("Truncated CIE/FDE length");
Len = read64<E>(D.data() + 4);
if (Len > D.size())
  error("CIE/FIE ends past the end of the section");
return Len + 12;

ELF/OutputSections.h

304 ↗

(On Diff #42863)

ArrayRef is usually supposed to be passed by value as it's basically a pointer to an array.

[grimar]

• Review commend addressed.
• Added test for valid 64 bit CIE/FDE record size.
• Fixed invalid-cie-length5.s test.

[grimar]

In D15532#311052, @emaste wrote:

Looks reasonable. How about a test for successful Is64Dwarf though?

Added test.

ELF/OutputSections.cpp

1001–1002 ↗

(On Diff #42863)

Done.

ELF/OutputSections.h

304 ↗

(On Diff #42863)

Ok, removed &.

[ruiu]

LGTM

lld/trunk/ELF/OutputSections.h

public:
   void addSectionAux(
       EHInputSection<ELFT> *S,
       llvm::iterator_range<const llvm::object::Elf_Rel_Impl<ELFT, IsRela> *>
           Rels);
 
   void addSection(EHInputSection<ELFT> *S);
 
 private:
+  uintX_t readEntryLength(ArrayRef<uint8_t> D);
+
   std::vector<EHInputSection<ELFT> *> Sections;
   std::vector<Cie<ELFT>> Cies;
 
   // Maps CIE content + personality to a index in Cies.
   llvm::DenseMap<std::pair<StringRef, StringRef>, unsigned> CieMap;
 };
 
 template <class ELFT>

lld/trunk/ELF/OutputSections.cpp

void EHOutputSection<ELFT>::addSectionAux(
   ArrayRef<uint8_t> SecData = S->getSectionData();
   ArrayRef<uint8_t> D = SecData;
   uintX_t Offset = 0;
   auto RelI = Rels.begin();
   auto RelE = Rels.end();
 
   DenseMap<unsigned, unsigned> OffsetToIndex;
   while (!D.empty()) {
-    if (D.size() < 4)
-      error("Truncated CIE/FDE length");
-    uint32_t Length = read32<E>(D.data());
-    Length += 4;
-
     unsigned Index = S->Offsets.size();
     S->Offsets.push_back(std::make_pair(Offset, -1));
 
-    if (Length > D.size())
-      error("CIE/FIE ends past the end of the section");
+    uintX_t Length = readEntryLength(D);
     StringRef Entry((const char *)D.data(), Length);
 
     while (RelI != RelE && RelI->r_offset < Offset)
       ++RelI;
     uintX_t NextOffset = Offset + Length;
     bool HasReloc = RelI != RelE && RelI->r_offset < NextOffset;
 
     uint32_t ID = read32<E>(D.data() + 4);
while (!D.empty()) {
     }
 
     Offset = NextOffset;
     D = D.slice(Length);
   }
 }
 
 template <class ELFT>
+typename EHOutputSection<ELFT>::uintX_t
+EHOutputSection<ELFT>::readEntryLength(ArrayRef<uint8_t> D) {
+  const endianness E = ELFT::TargetEndianness;
+
+  if (D.size() < 4)
+    error("Truncated CIE/FDE length");
+  uint64_t Len = read32<E>(D.data());
+  if (Len < UINT32_MAX) {
+    if (Len > (UINT32_MAX - 4))
+      error("CIE/FIE size is too large");
+    if (Len + 4 > D.size())
+      error("CIE/FIE ends past the end of the section");
+    return Len + 4;
+  }
+
+  if (D.size() < 12)
+    error("Truncated CIE/FDE length");
+  Len = read64<E>(D.data() + 4);
+  if (Len > (UINT64_MAX - 12))
+    error("CIE/FIE size is too large");
+  if (Len + 12 > D.size())
+    error("CIE/FIE ends past the end of the section");
+  return Len + 12;
+}
+
+template <class ELFT>
 void EHOutputSection<ELFT>::addSection(EHInputSection<ELFT> *S) {
   const Elf_Shdr *RelSec = S->RelocSection;
   if (!RelSec)
     return addSectionAux(
         S, make_range((const Elf_Rela *)nullptr, (const Elf_Rela *)nullptr));
   ELFFile<ELFT> &Obj = S->getFile()->getObj();
   if (RelSec->sh_type == SHT_RELA)
     return addSectionAux(S, Obj.relas(RelSec));

lld/trunk/test/ELF/invalid-cie-length3.s

+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
+
+ .section .eh_frame
+ .long 0xFFFFFFFC
+
+// CHECK: CIE/FIE size is too large

lld/trunk/test/ELF/invalid-cie-length4.s

+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
+
+ .section .eh_frame
+ .long 0xFFFFFFFF
+ .byte 0
+
+// CHECK: Truncated CIE/FDE length

lld/trunk/test/ELF/invalid-cie-length5.s

+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
+
+ .section .eh_frame
+ .long 0xFFFFFFFF
+ .quad 0xFFFFFFFFFFFFFFF4
+
+// CHECK: CIE/FIE size is too large

lld/trunk/test/ELF/valid-cie-length-dw64.s

+// REQUIRES: x86
+
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
+
+ .section .eh_frame
+ .long 0xFFFFFFFF
+ .quad 1
+ nop
+
+// CHECK-NOT: Truncated CIE/FDE length
+// CHECK-NOT: CIE/FIE size is too large
+// CHECK-NOT: CIE/FIE ends past the end of the section