Skip to content

Commit 8590b5c

Browse files
author
Georgii Rymar
committed
[libObject, llvm-readobj] - Reimplement ELFFile<ELFT>::getEntry.
Currently, `ELFFile<ELFT>::getEntry` does not check an index of an entry. Because of that the code might read past the end of the symbol table silently. I've added a test to `llvm-readobj\ELF\relocations.test` to demonstrate the possible issue. Also, I've added a unit test for this method. After this change, `getEntry` stops reporting the section index and reuses the `getSectionContentsAsArray` method, which already has all the validation needed. Our related warnings now provide more and better context sometimes. Differential revision: https://reviews.llvm.org/D93209
1 parent 6e913e4 commit 8590b5c

File tree

16 files changed

+152
-101
lines changed

16 files changed

+152
-101
lines changed

lld/test/ELF/invalid/dynamic-section-broken.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
## .dynamic section has invalid sh_entsize, check we report it.
22
# RUN: yaml2obj --docnum=1 %s -o %t.so
33
# RUN: not ld.lld %t.so -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR1
4-
# ERR1: error: {{.*}}.so: section [index 1] has an invalid sh_entsize: 291
4+
# ERR1: error: {{.*}}.so: section [index 1] has invalid sh_entsize: expected 16, but got 291
55

66
--- !ELF
77
FileHeader:

llvm/include/llvm/Object/ELF.h

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ Expected<ArrayRef<T>>
412412
ELFFile<ELFT>::getSectionContentsAsArray(const Elf_Shdr &Sec) const {
413413
if (Sec.sh_entsize != sizeof(T) && sizeof(T) != 1)
414414
return createError("section " + getSecIndexForError(*this, Sec) +
415-
" has an invalid sh_entsize: " + Twine(Sec.sh_entsize));
415+
" has invalid sh_entsize: expected " + Twine(sizeof(T)) +
416+
", but got " + Twine(Sec.sh_entsize));
416417

417418
uintX_t Offset = Sec.sh_offset;
418419
uintX_t Size = Sec.sh_size;
@@ -618,17 +619,17 @@ template <class ELFT>
618619
template <typename T>
619620
Expected<const T *> ELFFile<ELFT>::getEntry(const Elf_Shdr &Section,
620621
uint32_t Entry) const {
621-
if (sizeof(T) != Section.sh_entsize)
622-
return createError("section " + getSecIndexForError(*this, Section) +
623-
" has invalid sh_entsize: expected " + Twine(sizeof(T)) +
624-
", but got " + Twine(Section.sh_entsize));
625-
uint64_t Pos = Section.sh_offset + (uint64_t)Entry * sizeof(T);
626-
if (Pos + sizeof(T) > Buf.size())
627-
return createError("unable to access section " +
628-
getSecIndexForError(*this, Section) + " data at 0x" +
629-
Twine::utohexstr(Pos) +
630-
": offset goes past the end of file");
631-
return reinterpret_cast<const T *>(base() + Pos);
622+
Expected<ArrayRef<T>> EntriesOrErr = getSectionContentsAsArray<T>(Section);
623+
if (!EntriesOrErr)
624+
return EntriesOrErr.takeError();
625+
626+
ArrayRef<T> Arr = *EntriesOrErr;
627+
if (Entry >= Arr.size())
628+
return createError("can't read an entry at 0x" +
629+
Twine::utohexstr(Entry * sizeof(T)) +
630+
": it goes past the end of the section (0x" +
631+
Twine::utohexstr(Section.sh_size) + ")");
632+
return &Arr[Entry];
632633
}
633634

634635
template <class ELFT>

llvm/test/Object/invalid.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ Symbols:
118118
# RUN: yaml2obj %s --docnum=6 -o %t6
119119
# RUN: llvm-readobj --symbols %t6 2>&1 | FileCheck -DFILE=%t6 --check-prefix=INVALID-SYM-SIZE %s
120120

121-
# INVALID-SYM-SIZE: warning: '[[FILE]]': unable to read symbols from the SHT_SYMTAB section: section [index 1] has an invalid sh_entsize: 32
121+
# INVALID-SYM-SIZE: warning: '[[FILE]]': unable to read symbols from the SHT_SYMTAB section: section [index 1] has invalid sh_entsize: expected 24, but got 32
122122

123123
--- !ELF
124124
FileHeader:
@@ -630,7 +630,7 @@ Symbols:
630630
# RUN: yaml2obj %s --docnum=29 -o %t29
631631
# RUN: llvm-readobj -V %t29 2>&1 | FileCheck -DFILE=%t29 --check-prefix=INVALID-VER-SHENTSIZE %s
632632

633-
# INVALID-VER-SHENTSIZE: warning: '[[FILE]]': cannot read content of SHT_GNU_versym section with index 1: section [index 1] has an invalid sh_entsize: 3
633+
# INVALID-VER-SHENTSIZE: warning: '[[FILE]]': cannot read content of SHT_GNU_versym section with index 1: section [index 1] has invalid sh_entsize: expected 2, but got 3
634634

635635
--- !ELF
636636
FileHeader:

llvm/test/tools/llvm-readobj/ELF/call-graph-profile.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ Symbols:
4747
# RUN: llvm-readelf %t2.o --cg-profile | FileCheck %s --check-prefix=GNU
4848

4949
# LLVM-ERR: CGProfile [
50-
# LLVM-ERR-NEXT: warning: '[[FILE]]': unable to dump the SHT_LLVM_CALL_GRAPH_PROFILE section: section [index 1] has an invalid sh_entsize: 15
50+
# LLVM-ERR-NEXT: warning: '[[FILE]]': unable to dump the SHT_LLVM_CALL_GRAPH_PROFILE section: section [index 1] has invalid sh_entsize: expected 16, but got 15
5151
# LLVM-ERR-NEXT: ]
5252

5353
## Check we report a warning when unable to dump a name of a symbol.

llvm/test/tools/llvm-readobj/ELF/reloc-symbol-with-versioning.test

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
# RUN: yaml2obj %s -o %t.o
2-
# RUN: llvm-readobj --demangle -r %t.o | FileCheck %s --check-prefix LLVM
3-
# RUN: llvm-readelf --demangle -r %t.o | FileCheck %s --check-prefix GNU
2+
# RUN: llvm-readobj --demangle -r %t.o 2>&1 | \
3+
# RUN: FileCheck %s --check-prefix=LLVM --implicit-check-not=warning:
4+
# RUN: llvm-readelf --demangle -r %t.o 2>&1 | \
5+
# RUN: FileCheck %s --check-prefix=GNU --implicit-check-not=warning:
46

57
# GNU: Relocation section '.rela.plt' at offset {{.*}} contains 5 entries:
68
# GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
@@ -30,7 +32,7 @@ Sections:
3032
- Name: .gnu.version
3133
Type: SHT_GNU_versym
3234
Flags: [ SHF_ALLOC ]
33-
Entries: [ 0, 2, 3, 4, 2 ]
35+
Entries: [ 0, 2, 3, 4, 2, 1 ]
3436
- Name: .gnu.version_r
3537
Type: SHT_GNU_verneed
3638
Flags: [ SHF_ALLOC ]

llvm/test/tools/llvm-readobj/ELF/relocation-errors.test

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66

77
# LLVM: Relocations [
88
# LLVM-NEXT: Section (3) .rel.text {
9-
# LLVM-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_REL section with index 3: unable to access section [index 6] data at 0x17e7e7e8d0: offset goes past the end of file
10-
# LLVM-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_REL section with index 3: unable to access section [index 6] data at 0x17e7e7e8d0: offset goes past the end of file
9+
# LLVM-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_REL section with index 3: unable to read an entry with index 4278124286 from SHT_SYMTAB section with index 6: can't read an entry at 0x17e7e7e7d0: it goes past the end of the section (0x90)
10+
# LLVM-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_REL section with index 3: unable to read an entry with index 4278124286 from SHT_SYMTAB section with index 6: can't read an entry at 0x17e7e7e7d0: it goes past the end of the section (0x90)
1111
# LLVM-NEXT: 0x2 R_X86_64_NONE -{{$}}
1212
# LLVM-NEXT: 0x3 R_X86_64_NONE .sec.symbol1{{$}}
1313
# LLVM-NEXT: warning: '[[FILE]]': invalid section index: 255
@@ -23,8 +23,8 @@
2323

2424
# GNU: Relocation section '.rel.text' at offset 0x41 contains 7 entries:
2525
# GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name
26-
# GNU-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_REL section with index 3: unable to access section [index 6] data at 0x17e7e7e8d0: offset goes past the end of file
27-
# GNU-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_REL section with index 3: unable to access section [index 6] data at 0x17e7e7e8d0: offset goes past the end of file
26+
# GNU-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_REL section with index 3: unable to read an entry with index 4278124286 from SHT_SYMTAB section with index 6: can't read an entry at 0x17e7e7e7d0: it goes past the end of the section (0x90)
27+
# GNU-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_REL section with index 3: unable to read an entry with index 4278124286 from SHT_SYMTAB section with index 6: can't read an entry at 0x17e7e7e7d0: it goes past the end of the section (0x90)
2828
# GNU-NEXT: 0000000000000002 0000000000000000 R_X86_64_NONE
2929
# GNU-NEXT: 0000000000000003 0000000200000000 R_X86_64_NONE 0000000000000000 .sec.symbol1
3030
# GNU-NEXT: warning: '[[FILE]]': invalid section index: 255

llvm/test/tools/llvm-readobj/ELF/relocations.test

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ Symbols:
207207

208208
# BROKEN-REL-LLVM: Relocations [
209209
# BROKEN-REL-LLVM-NEXT: Section (2) .rel.text {
210-
# BROKEN-REL-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_REL section with index 2: section [index 2] has an invalid sh_entsize: 1
210+
# BROKEN-REL-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_REL section with index 2: section [index 2] has invalid sh_entsize: expected 16, but got 1
211211
# BROKEN-REL-LLVM-NEXT: }
212212
# BROKEN-REL-LLVM-NEXT: Section (3) .rela.text {
213213
# BROKEN-REL-LLVM-NEXT: 0x0 R_X86_64_NONE rela_0 0x0
@@ -220,7 +220,7 @@ Symbols:
220220

221221
# BROKEN-REL-GNU: Relocation section '.rel.text' at offset 0x51 contains 64 entries:
222222
# BROKEN-REL-GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name
223-
# BROKEN-REL-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_REL section with index 2: section [index 2] has an invalid sh_entsize: 1
223+
# BROKEN-REL-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_REL section with index 2: section [index 2] has invalid sh_entsize: expected 16, but got 1
224224
# BROKEN-REL-GNU: Relocation section '.rela.text' at offset 0x91 contains 5 entries:
225225
# BROKEN-REL-GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name
226226
# BROKEN-REL-GNU-NEXT: 0000000000000000 0000000500000000 R_X86_64_NONE 0000000000000000 rela_0 + 0
@@ -245,7 +245,7 @@ Symbols:
245245
# BROKEN-RELA-LLVM-NEXT: 0x9 R_X86_64_64 rel_64{{$}}
246246
# BROKEN-RELA-LLVM-NEXT: }
247247
# BROKEN-RELA-LLVM-NEXT: Section (3) .rela.text {
248-
# BROKEN-RELA-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_RELA section with index 3: section [index 3] has an invalid sh_entsize: 1
248+
# BROKEN-RELA-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_RELA section with index 3: section [index 3] has invalid sh_entsize: expected 24, but got 1
249249
# BROKEN-RELA-LLVM-NEXT: }
250250
# BROKEN-RELA-LLVM-NEXT: ]
251251

@@ -258,7 +258,7 @@ Symbols:
258258
# BROKEN-RELA-GNU-EMPTY:
259259
# BROKEN-RELA-GNU-NEXT: Relocation section '.rela.text' at offset 0x91 contains 120 entries:
260260
# BROKEN-RELA-GNU-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
261-
# BROKEN-RELA-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_RELA section with index 3: section [index 3] has an invalid sh_entsize: 1
261+
# BROKEN-RELA-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from SHT_RELA section with index 3: section [index 3] has invalid sh_entsize: expected 24, but got 1
262262

263263
## Case C: check the case when relocations can't be read from SHT_REL/SHT_RELA sections
264264
## because of broken sh_link fields.
@@ -498,3 +498,41 @@ Symbols:
498498
# GNU-SHNAME-NEXT: 0000000000000005 0000000700000004 R_X86_64_PLT32 0000000000000002 rela_pos + 2
499499
# GNU-SHNAME-NEXT: ffffffffffffffff 0000000800000001 R_X86_64_64 0000000000000003 rela_minneg - 8000000000000000
500500
# GNU-SHNAME-NEXT: 0000000000000009 000000090000000b R_X86_64_32S ffffffffffffffff rela_maxpos + 7fffffffffffffff
501+
502+
## Check that we report a warning when a relocation has a
503+
## symbol index past the end of the symbol table
504+
505+
# RUN: yaml2obj %s --docnum=3 -o %t3
506+
# RUN: llvm-readobj --relocs %t3 2>&1 | \
507+
# RUN: FileCheck %s --implicit-check-not=warning: -DFILE=%t3 --check-prefix=LLVM-SYMNDX
508+
# RUN: llvm-readelf --relocs %t3 2>&1 | \
509+
# RUN: FileCheck %s --implicit-check-not=warning: -DFILE=%t3 --check-prefix=GNU-SYMNDX
510+
511+
# LLVM-SYMNDX: Relocations [
512+
# LLVM-SYMNDX-NEXT: Section (1) .rela.text {
513+
# LLVM-SYMNDX-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_RELA section with index 1: unable to read an entry with index 2 from SHT_SYMTAB section with index 2: can't read an entry at 0x30: it goes past the end of the section (0x30)
514+
# LLVM-SYMNDX-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_RELA section with index 1: unable to read an entry with index 3 from SHT_SYMTAB section with index 2: can't read an entry at 0x48: it goes past the end of the section (0x30)
515+
# LLVM-SYMNDX-NEXT: }
516+
# LLVM-SYMNDX-NEXT: ]
517+
518+
# GNU-SYMNDX: Relocation section '.rela.text' at offset 0x40 contains 2 entries:
519+
# GNU-SYMNDX-NEXT: Offset Info Type Symbol's Value Symbol's Name + Addend
520+
# GNU-SYMNDX-NEXT: warning: '[[FILE]]': unable to print relocation 1 in SHT_RELA section with index 1: unable to read an entry with index 2 from SHT_SYMTAB section with index 2: can't read an entry at 0x30: it goes past the end of the section (0x30)
521+
# GNU-SYMNDX-NEXT: warning: '[[FILE]]': unable to print relocation 2 in SHT_RELA section with index 1: unable to read an entry with index 3 from SHT_SYMTAB section with index 2: can't read an entry at 0x48: it goes past the end of the section (0x30)
522+
523+
--- !ELF
524+
FileHeader:
525+
Class: ELFCLASS64
526+
Data: ELFDATA2LSB
527+
Type: ET_DYN
528+
Machine: EM_X86_64
529+
Sections:
530+
- Name: .rela.text
531+
Type: SHT_RELA
532+
Relocations:
533+
- Type: R_X86_64_NONE
534+
Symbol: 0x2
535+
- Type: R_X86_64_NONE
536+
Symbol: 0x3
537+
Symbols:
538+
- Name: foo

llvm/test/tools/llvm-readobj/ELF/relr-relocs.test

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,14 @@ Sections:
169169

170170
# BROKEN-LLVM: Relocations [
171171
# BROKEN-LLVM-NEXT: Section (1) .relr.dyn {
172-
# BROKEN-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from [[SECNAME]] section with index 1: section [index 1] has an invalid sh_entsize: 1
172+
# BROKEN-LLVM-NEXT: warning: '[[FILE]]': unable to read relocations from [[SECNAME]] section with index 1: section [index 1] has invalid sh_entsize: expected 4, but got 1
173173
# BROKEN-LLVM-NEXT: }
174174
# BROKEN-LLVM-NEXT: ]
175175

176-
# BROKEN-GNU: warning: '[[FILE]]': unable to get the number of relocations in [[SECNAME]] section with index 1: section [index 1] has an invalid sh_entsize: 1
176+
# BROKEN-GNU: warning: '[[FILE]]': unable to get the number of relocations in [[SECNAME]] section with index 1: section [index 1] has invalid sh_entsize: expected 4, but got 1
177177
# BROKEN-GNU: Relocation section '.relr.dyn' at offset 0x34 contains <?> entries:
178178
# BROKEN-GNU-NEXT: Offset Info Type Sym. Value Symbol's Name
179-
# BROKEN-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from [[SECNAME]] section with index 1: section [index 1] has an invalid sh_entsize: 1
179+
# BROKEN-GNU-NEXT: warning: '[[FILE]]': unable to read relocations from [[SECNAME]] section with index 1: section [index 1] has invalid sh_entsize: expected 4, but got 1
180180

181181
## Case B: check the case when relocations can't be read from an SHT_ANDROID_RELR section.
182182
## SHT_ANDROID_RELR = 0x6fffff00.

0 commit comments

Comments
 (0)