[Object/ELF] - Do section header sh_offset/sh_size values check once during object loading.

[grimar]

Summary

This helps to keep client code cleaner.
For example we have next code in LLD:

auto *Begin =
    reinterpret_cast<const Elf_Dyn *>(Obj.base() + DynamicSec->sh_offset);
const Elf_Dyn *End = Begin + DynamicSec->sh_size / sizeof(Elf_Dyn);

sh_offset and/or sh_size may be broken here and the same situation can be found for some other sections.
Instead of placing multiple checks in LLD, I suggest this change to check object once during loading.

[• rafael]

I don't think we should do this. Eagerly checking for errors means that a program would be unable to look at the file even if it is not hitting that particular corruption.

For example, at some point llvm-readobj should be able to just report a broken section offset and keep going. This makes it impossible.

Looks like what is needed is a dyns() function that return a Elf_Dyn range. Maybe implement it with a template so that rels() relas() can share the code?

[grimar]

D25327 solves the problem of dynamic section size instead.

include/llvm/Object/ELF.h

public:
   }
   Elf_Sym_Range symbols(const Elf_Shdr *Sec) const {
     return makeArrayRef(symbol_begin(Sec), symbol_end(Sec));
   }
 
   const Elf_Rela *rela_begin(const Elf_Shdr *sec) const {
     if (sec->sh_entsize != sizeof(Elf_Rela))
       report_fatal_error("Invalid relocation entry size");
-    if (sec->sh_offset >= Buf.size())
-      report_fatal_error("Invalid relocation entry offset");
     return reinterpret_cast<const Elf_Rela *>(base() + sec->sh_offset);
   }
 
   const Elf_Rela *rela_end(const Elf_Shdr *sec) const {
     uint64_t Size = sec->sh_size;
     if (Size % sizeof(Elf_Rela))
       report_fatal_error("Invalid relocation table size");
     return rela_begin(sec) + Size / sizeof(Elf_Rela);
   }
 
   Elf_Rela_Range relas(const Elf_Shdr *Sec) const {
     return makeArrayRef(rela_begin(Sec), rela_end(Sec));
   }
 
   const Elf_Rel *rel_begin(const Elf_Shdr *sec) const {
     if (sec->sh_entsize != sizeof(Elf_Rel))
       report_fatal_error("Invalid relocation entry size");
-    if (sec->sh_offset >= Buf.size())
-      report_fatal_error("Invalid relocation entry offset");
     return reinterpret_cast<const Elf_Rel *>(base() + sec->sh_offset);
   }
 
   const Elf_Rel *rel_end(const Elf_Shdr *sec) const {
     uint64_t Size = sec->sh_size;
     if (Size % sizeof(Elf_Rel))
       report_fatal_error("Invalid relocation table size");
     return rel_begin(sec) + Size / sizeof(Elf_Rel);
if ((EC = StrTabSecOrErr.getError()))
       return;
 
     ErrorOr<StringRef> StringTableOrErr = getStringTable(*StrTabSecOrErr);
     if ((EC = StringTableOrErr.getError()))
       return;
     DotShstrtab = *StringTableOrErr;
   }
 
+  for (const Elf_Shdr &Sec : sections()) {
+    if ((Sec.sh_offset > getBufSize()) ||
+      (Sec.sh_offset + Sec.sh_size > getBufSize())) {
+      EC = object_error::parse_failed;
+      return;
+    }
+  }
+
   EC = std::error_code();
 }
 
 template <class ELFT>
 static bool compareAddr(uint64_t VAddr, const Elf_Phdr_Impl<ELFT> *Phdr) {
   return VAddr < Phdr->p_vaddr;
 }
 

test/Object/invalid.test

 RUN: not llvm-readobj -t %p/Inputs/invalid-ext-symtab-index.elf-x86-64 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-EXT-SYMTAB-INDEX %s
 INVALID-EXT-SYMTAB-INDEX: Invalid symbol table index
 
 RUN: not llvm-readobj -r %p/Inputs/invalid-relocation-sec-sh_offset.elf-i386 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-RELOC-SH-OFFSET %s
 RUN: not llvm-readobj -r %p/Inputs/invalid-relocation-sec-sh_offset.elf-x86-64 2>&1 | \
 RUN:   FileCheck --check-prefix=INVALID-RELOC-SH-OFFSET %s
-INVALID-RELOC-SH-OFFSET: Invalid relocation entry offset
+INVALID-RELOC-SH-OFFSET: Invalid data was encountered while parsing the file
+
+RUN: not llvm-readobj -r %p/Inputs/invalid-relocation-sec-sh_size.elf-i386 2>&1 | \
+RUN:   FileCheck --check-prefix=INVALID-RELOC-SH-SIZE %s
+RUN: not llvm-readobj -r %p/Inputs/invalid-relocation-sec-sh_size.elf-x86-64 2>&1 | \
+RUN:   FileCheck --check-prefix=INVALID-RELOC-SH-SIZE %s
+INVALID-RELOC-SH-SIZE: Invalid data was encountered while parsing the file