[PATCH] D35371: [COFF] Bounds check relocations

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.

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.


https://reviews.llvm.org/D35371

Files:
  lld/COFF/Chunks.cpp
  lld/test/COFF/reloc-oob.yaml


Index: lld/test/COFF/reloc-oob.yaml
===================================================================
--- /dev/null
+++ lld/test/COFF/reloc-oob.yaml
@@ -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
+...
Index: lld/COFF/Chunks.cpp
===================================================================
--- lld/COFF/Chunks.cpp
+++ lld/COFF/Chunks.cpp
@@ -211,6 +211,13 @@
 
   // Apply relocations.
   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 >= getSize())
+      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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35371.106467.patch
Type: text/x-patch
Size: 3047 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170713/2cbbc8c5/attachment.bin>