[lld] r307948 - [COFF] Bounds check relocations

[Reid Kleckner via llvm-commits]

Author: rnk
Date: Thu Jul 13 13:29:59 2017
New Revision: 307948

URL: http://llvm.org/viewvc/llvm-project?rev=307948&view=rev
Log:
[COFF] Bounds check relocations

Summary:
This would have caught the invalid object file I used in my test case in
r307726. The OOB was only caught by ASan later, which is slow and
doesn't work on some platforms. LLD should do some basic input
validation itself. This check isn't perfect, so relocations can reach
OOB by up to seven bytes, but it's better than what we had and probably
cheap.

Reviewers: ruiu

Subscribers: llvm-commits

Differential Revision: https://reviews.llvm.org/D35371

Added:
    lld/trunk/test/COFF/reloc-oob.yaml
Modified:
    lld/trunk/COFF/Chunks.cpp
    lld/trunk/COFF/Chunks.h

Modified: lld/trunk/COFF/Chunks.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.cpp?rev=307948&r1=307947&r2=307948&view=diff
==============================================================================
--- lld/trunk/COFF/Chunks.cpp (original)
+++ lld/trunk/COFF/Chunks.cpp Thu Jul 13 13:29:59 2017
@@ -210,7 +210,15 @@ void SectionChunk::writeTo(uint8_t *Buf)
   memcpy(Buf + OutputSectionOff, A.data(), A.size());
 
   // Apply relocations.
+  size_t InputSize = getSize();
   for (const coff_relocation &Rel : Relocs) {
+    // Check for an invalid relocation offset. This check isn't perfect, because
+    // we don't have the relocation size, which is only known after checking the
+    // machine and relocation type. As a result, a relocation may overwrite the
+    // beginning of the following input section.
+    if (Rel.VirtualAddress >= InputSize)
+      fatal("relocation points beyond the end of its parent section");
+
     uint8_t *Off = Buf + OutputSectionOff + Rel.VirtualAddress;
 
     // Get the output section of the symbol for this relocation.  The output

Modified: lld/trunk/COFF/Chunks.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Chunks.h?rev=307948&r1=307947&r2=307948&view=diff
==============================================================================
--- lld/trunk/COFF/Chunks.h (original)
+++ lld/trunk/COFF/Chunks.h Thu Jul 13 13:29:59 2017
@@ -112,7 +112,7 @@ protected:
 };
 
 // A chunk corresponding a section of an input file.
-class SectionChunk : public Chunk {
+class SectionChunk final : public Chunk {
   // Identical COMDAT Folding feature accesses section internal data.
   friend class ICF;
 

Added: lld/trunk/test/COFF/reloc-oob.yaml
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/reloc-oob.yaml?rev=307948&view=auto
==============================================================================
--- lld/trunk/test/COFF/reloc-oob.yaml (added)
+++ lld/trunk/test/COFF/reloc-oob.yaml Thu Jul 13 13:29:59 2017
@@ -0,0 +1,62 @@
+# Make sure LLD does some light relocation bounds checking.
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: not lld-link %t.obj -entry:main -nodefaultlib -out:%t.exe 2>&1 | FileCheck %s
+
+# CHECK: error: relocation points beyond the end of its parent section
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_I386
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     5589E550C745FC00000000A10000000083C4045DC3
+    Relocations:
+      - VirtualAddress:  24
+        SymbolName:      _g
+        Type:            IMAGE_REL_I386_DIR32
+  - Name:            .data
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+    Alignment:       4
+    SectionData:     2A000000
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          21
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        662775349
+      Number:          1
+  - Name:            .data
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          4
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        3482275674
+      Number:          2
+  - Name:            _main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            _g
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...