[llvm] 2584e1e - [llvm-readobj] - Don't crash when relocation table goes past the EOF.
Georgii Rymar via llvm-commits
llvm-commits at lists.llvm.org
Sun Nov 22 23:40:07 PST 2020
Author: Georgii Rymar
Date: 2020-11-23T10:31:04+03:00
New Revision: 2584e1e324c97eeeacc1e421e5f3191a708c3d2d
URL: https://github.com/llvm/llvm-project/commit/2584e1e324c97eeeacc1e421e5f3191a708c3d2d
DIFF: https://github.com/llvm/llvm-project/commit/2584e1e324c97eeeacc1e421e5f3191a708c3d2d.diff
LOG: [llvm-readobj] - Don't crash when relocation table goes past the EOF.
It is possible to trigger reading past the EOF by breaking fields like
DT_PLTRELSZ, DT_RELSZ or DT_RELASZ
This patch adds a validation in `DynRegionInfo` helper class.
Differential revision: https://reviews.llvm.org/D91787
Added:
Modified:
llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
llvm/tools/llvm-readobj/ELFDumper.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
index bd7916a91fc1..741eaec90d13 100644
--- a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
+++ b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
@@ -49,11 +49,16 @@ ProgramHeaders:
LastSec: .dynamic
## Show we print a warning for an invalid relocation table size stored in a DT_RELASZ entry.
-# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0xFF -DTAG2=DT_RELAENT %s -o %t2
-# RUN: llvm-readobj --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ
-# RUN: llvm-readelf --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ
-# INVALID-DT-RELASZ: warning: '[[FILE]]': invalid DT_RELASZ value (0xff) or DT_RELAENT value (0x18)
+## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning
+
+# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0xFF -DTAG2=DT_RELAENT %s -o %t2a
+# RUN: llvm-readobj --dyn-relocations %t2a 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t2a --check-prefix=INVALID-DT-RELASZ1 --implicit-check-not=warning:
+# RUN: llvm-readelf --dyn-relocations %t2a 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t2a --check-prefix=INVALID-DT-RELASZ1 --implicit-check-not=warning:
+
+# INVALID-DT-RELASZ1: warning: '[[FILE]]': invalid DT_RELASZ value (0xff) or DT_RELAENT value (0x18)
--- !ELF
FileHeader:
@@ -80,19 +85,42 @@ ProgramHeaders:
FirstSec: .relx.dyn
LastSec: .dynamic
+## Case B: the DT_RELASZ has value of 0x251, what is too large, because the relocation table goes past the EOF.
+
+# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG1VAL=0x251 -DTAG2=DT_RELAENT %s -o %t2b
+# RUN: llvm-readobj --dyn-relocations %t2b 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t2b --check-prefix=INVALID-DT-RELASZ2 --implicit-check-not=warning:
+# RUN: llvm-readelf --dyn-relocations %t2b 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t2b --check-prefix=INVALID-DT-RELASZ2 --implicit-check-not=warning:
+
+# INVALID-DT-RELASZ2: warning: '[[FILE]]': unable to read data at 0x78 of size 0x251 (DT_RELASZ value): it goes past the end of the file of size 0x2c8
+
## Show we print a warning for an invalid relocation table entry size stored in a DT_RELAENT entry.
# RUN: yaml2obj --docnum=2 -DRELTYPE=RELA -DTAG1=DT_RELASZ -DTAG2=DT_RELAENT -DTAG2VAL=0xFF %s -o %t3
-# RUN: llvm-readobj --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT
-# RUN: llvm-readelf --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT
+# RUN: llvm-readobj --dyn-relocations %t3 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT --implicit-check-not=warning:
+# RUN: llvm-readelf --dyn-relocations %t3 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT --implicit-check-not=warning:
## INVALID-DT-RELAENT: warning: '[[FILE]]': invalid DT_RELASZ value (0x18) or DT_RELAENT value (0xff)
## Show we print a warning for an invalid relocation table size stored in a DT_RELSZ entry.
-# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0xFF -DTAG2=DT_RELENT %s -o %t4
-# RUN: llvm-readobj --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ
-# RUN: llvm-readelf --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ
-## INVALID-DT-RELSZ: warning: '[[FILE]]': invalid DT_RELSZ value (0xff) or DT_RELENT value (0x18)
+## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning.
+
+# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0xFF -DTAG2=DT_RELENT %s -o %t4a
+# RUN: llvm-readobj --dyn-relocations %t4a 2>&1 | FileCheck %s -DFILE=%t4a --check-prefix=INVALID-DT-RELSZ1
+# RUN: llvm-readelf --dyn-relocations %t4a 2>&1 | FileCheck %s -DFILE=%t4a --check-prefix=INVALID-DT-RELSZ1
+
+## INVALID-DT-RELSZ1: warning: '[[FILE]]': invalid DT_RELSZ value (0xff) or DT_RELENT value (0x18)
+
+## Case B: the DT_RELSZ has value of 0x251, what is too large, because the relocation table goes past the EOF.
+
+# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG1VAL=0x251 -DTAG2=DT_RELENT %s -o %t4b
+# RUN: llvm-readobj --dyn-relocations %t4b 2>&1 | FileCheck %s -DFILE=%t4b --check-prefix=INVALID-DT-RELSZ2
+# RUN: llvm-readelf --dyn-relocations %t4b 2>&1 | FileCheck %s -DFILE=%t4b --check-prefix=INVALID-DT-RELSZ2
+
+# INVALID-DT-RELSZ2: warning: '[[FILE]]': unable to read data at 0x78 of size 0x251 (DT_RELSZ value): it goes past the end of the file of size 0x2c8
## Show we print a warning for an invalid relocation table entry size stored in a DT_RELENT entry.
# RUN: yaml2obj --docnum=2 -DRELTYPE=REL -DTAG1=DT_RELSZ -DTAG2=DT_RELENT -DTAG2VAL=0xFF %s -o %t5
@@ -126,11 +154,16 @@ ProgramHeaders:
## Show we print a warning for an invalid value of DT_PLTRELSZ, which describes the total size
## of the relocation entries associated with the procedure linkage table.
-# RUN: yaml2obj --docnum=3 %s -o %t10
-# RUN: llvm-readobj --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ
-# RUN: llvm-readelf --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ
-# INVALID-DT-PLTRELSZ: warning: '[[FILE]]': invalid DT_PLTRELSZ value (0xff){{$}}
+## Case A: the size of a single relocation entry is 0x18. In this case 0xFF % 0x18 != 0 and we report a warning.
+
+# RUN: yaml2obj --docnum=3 -DVAL=0xFF %s -o %t10a
+# RUN: llvm-readobj --dyn-relocations %t10a 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t10a --check-prefix=INVALID-DT-PLTRELSZ1 --implicit-check-not=warning:
+# RUN: llvm-readelf --dyn-relocations %t10a 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t10a --check-prefix=INVALID-DT-PLTRELSZ1 --implicit-check-not=warning:
+
+# INVALID-DT-PLTRELSZ1: warning: '[[FILE]]': invalid DT_PLTRELSZ value (0xff){{$}}
--- !ELF
FileHeader:
@@ -149,7 +182,7 @@ Sections:
- Tag: DT_JMPREL
Value: 0x0
- Tag: DT_PLTRELSZ
- Value: 0xFF ## The valid value would be 0x18.
+ Value: [[VAL]] ## The valid value would be 0x18.
- Tag: DT_PLTREL
Value: 0x7 ## DT_RELA
- Tag: DT_NULL
@@ -160,6 +193,22 @@ ProgramHeaders:
FirstSec: .rela.plt
LastSec: .dynamic
+## Case B: the DT_PLTRELSZ (PLT size) has value of 0x269, what is too large, because PLT goes past the EOF.
+
+# RUN: yaml2obj --docnum=3 -DVAL=0x269 %s -o %t10b
+# RUN: llvm-readobj --dyn-relocations %t10b 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t10b --check-prefix=INVALID-DT-PLTRELSZ2-LLVM --implicit-check-not=warning:
+# RUN: llvm-readelf --dyn-relocations %t10b 2>&1 | \
+# RUN: FileCheck %s -DFILE=%t10b --check-prefix=INVALID-DT-PLTRELSZ2-GNU --implicit-check-not=warning:
+
+# INVALID-DT-PLTRELSZ2-LLVM: Dynamic Relocations {
+# INVALID-DT-PLTRELSZ2-LLVM-NEXT: warning: '[[FILE]]': unable to read data at 0x78 of size 0x269 (DT_PLTRELSZ value): it goes past the end of the file of size 0x2e0
+# INVALID-DT-PLTRELSZ2-LLVM-NEXT: }
+
+# INVALID-DT-PLTRELSZ2-GNU: 'PLT' relocation section at offset 0x78 contains 617 bytes:
+# INVALID-DT-PLTRELSZ2-GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
+# INVALID-DT-PLTRELSZ2-GNU-NEXT: warning: '[[FILE]]': unable to read data at 0x78 of size 0x269 (DT_PLTRELSZ value): it goes past the end of the file of size 0x2e0
+
## Show we print a warning when dumping dynamic relocations if there is no dynamic symbol table.
# RUN: yaml2obj --docnum=4 %s -o %t11
# RUN: llvm-readobj --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=LLVM-NO-DYNSYM
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 0d53ab668f8f..de8d290be779 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -126,9 +126,9 @@ template <class ELFT> struct RelSymbol {
/// the size, entity size and virtual address are
diff erent entries in arbitrary
/// order (DT_REL, DT_RELSZ, DT_RELENT for example).
struct DynRegionInfo {
- DynRegionInfo(StringRef ObjName) : FileName(ObjName) {}
- DynRegionInfo(const uint8_t *A, uint64_t S, uint64_t ES, StringRef ObjName)
- : Addr(A), Size(S), EntSize(ES), FileName(ObjName) {}
+ DynRegionInfo(const Binary &Owner) : Obj(&Owner) {}
+ DynRegionInfo(const Binary &Owner, const uint8_t *A, uint64_t S, uint64_t ES)
+ : Addr(A), Size(S), EntSize(ES), Obj(&Owner) {}
/// Address in current address space.
const uint8_t *Addr = nullptr;
@@ -137,8 +137,8 @@ struct DynRegionInfo {
/// Size of each entity in the region.
uint64_t EntSize = 0;
- /// Name of the file. Used for error reporting.
- StringRef FileName;
+ /// Owner object. Used for error reporting.
+ const Binary *Obj;
/// Error prefix. Used for error reporting to provide more information.
std::string Context;
/// Region size name. Used for error reporting.
@@ -151,6 +151,22 @@ struct DynRegionInfo {
const Type *Start = reinterpret_cast<const Type *>(Addr);
if (!Start)
return {Start, Start};
+
+ const uint64_t Offset =
+ Addr - (const uint8_t *)Obj->getMemoryBufferRef().getBufferStart();
+ const uint64_t ObjSize = Obj->getMemoryBufferRef().getBufferSize();
+
+ if (Size > ObjSize - Offset) {
+ reportWarning(
+ createError("unable to read data at 0x" + Twine::utohexstr(Offset) +
+ " of size 0x" + Twine::utohexstr(Size) + " (" +
+ SizePrintName +
+ "): it goes past the end of the file of size 0x" +
+ Twine::utohexstr(ObjSize)),
+ Obj->getFileName());
+ return {Start, Start};
+ }
+
if (EntSize == sizeof(Type) && (Size % EntSize == 0))
return {Start, Start + (Size / EntSize)};
@@ -165,7 +181,7 @@ struct DynRegionInfo {
(" or " + EntSizePrintName + " (0x" + Twine::utohexstr(EntSize) + ")")
.str();
- reportWarning(createError(Msg.c_str()), FileName);
+ reportWarning(createError(Msg.c_str()), Obj->getFileName());
return {Start, Start};
}
};
@@ -296,8 +312,7 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
") + size (0x" + Twine::utohexstr(Size) +
") is greater than the file size (0x" +
Twine::utohexstr(Obj.getBufSize()) + ")");
- return DynRegionInfo(Obj.base() + Offset, Size, EntSize,
- ObjF.getFileName());
+ return DynRegionInfo(ObjF, Obj.base() + Offset, Size, EntSize);
}
void printAttributes();
@@ -1927,7 +1942,7 @@ void ELFDumper<ELFT>::loadDynamicTable() {
if (!DynamicPhdr && !DynamicSec)
return;
- DynRegionInfo FromPhdr(ObjF.getFileName());
+ DynRegionInfo FromPhdr(ObjF);
bool IsPhdrTableValid = false;
if (DynamicPhdr) {
// Use cantFail(), because p_offset/p_filesz fields of a PT_DYNAMIC are
@@ -1943,7 +1958,7 @@ void ELFDumper<ELFT>::loadDynamicTable() {
// Ignore sh_entsize and use the expected value for entry size explicitly.
// This allows us to dump dynamic sections with a broken sh_entsize
// field.
- DynRegionInfo FromSec(ObjF.getFileName());
+ DynRegionInfo FromSec(ObjF);
bool IsSecTableValid = false;
if (DynamicSec) {
Expected<DynRegionInfo> RegOrErr =
@@ -2005,10 +2020,8 @@ void ELFDumper<ELFT>::loadDynamicTable() {
template <typename ELFT>
ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> &O,
ScopedPrinter &Writer)
- : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()),
- DynRelRegion(O.getFileName()), DynRelaRegion(O.getFileName()),
- DynRelrRegion(O.getFileName()), DynPLTRelRegion(O.getFileName()),
- DynamicTable(O.getFileName()) {
+ : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()), DynRelRegion(O),
+ DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O), DynamicTable(O) {
// Dumper reports all non-critical errors as warnings.
// It does not print the same warning more than once.
WarningHandler = [this](const Twine &Msg) {
@@ -2125,7 +2138,7 @@ void ELFDumper<ELFT>::parseDynamicTable() {
// If we can't map the DT_SYMTAB value to an address (e.g. when there are
// no program headers), we ignore its value.
if (const uint8_t *VA = toMappedAddr(Dyn.getTag(), Dyn.getPtr())) {
- DynSymFromTable.emplace(ObjF.getFileName());
+ DynSymFromTable.emplace(ObjF);
DynSymFromTable->Addr = VA;
DynSymFromTable->EntSize = sizeof(Elf_Sym);
DynSymFromTable->EntSizePrintName = "";
More information about the llvm-commits
mailing list