Skip to content

[llvm-objdump][ELF]Fix crash when reading strings from .dynstr #125679

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Mar 10, 2025
Merged

[llvm-objdump][ELF]Fix crash when reading strings from .dynstr #125679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2cc5751
[llvm-objdump][ELF]Fix crash when reading strings from .dynstr(#86612)
cabbaken Jan 26, 2025
e8414fc
[llvm-objdump][ELF] Fix test file format
cabbaken Feb 6, 2025
1acfb61
Update llvm/test/tools/llvm-objdump/ELF/dynamic-section.test
cabbaken Feb 10, 2025
61a78c9
Merge branch 'main' into main
cabbaken Feb 11, 2025
21560c7
Fix .dynstr string size check and modify code style.
cabbaken Feb 12, 2025
f1e32bc
Optimize warning report format.
cabbaken Feb 12, 2025
4181db5
Add getDynamicStrTab String size check
cabbaken Feb 20, 2025
a8107b3
modify pointer initialize format.
cabbaken Feb 24, 2025
6eeb6ee
Optimize dynamic-section.test and code style.
cabbaken Feb 25, 2025
7e569fb
Fix check of dynamic-section.test
cabbaken Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions llvm/test/tools/llvm-objdump/ELF/dynamic-section.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,3 +470,46 @@ Sections:
Value: 0x1
- Tag: DT_NULL
Value: 0x0

# RUN: yaml2obj --docnum=5 %s -o %t5
# RUN: llvm-objdump -p %t5 2>&1 | FileCheck %s --strict-whitespace -DFILE=%t5 --check-prefix=WARN

# WARN: Dynamic Section:
# WARN-NEXT: warning: '[[FILE]]': invalid string table offset
# WARN-NEXT: NEEDED 0x0000000000000010
# WARN-NEXT: NEEDED {{$}}
# WARN-NEXT: STRTAB 0x0000000000001000
# WARN-NEXT: STRSZ 0x0000000000000010

--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Machine: EM_X86_64
Sections:
- Name: .dynstr
Type: SHT_STRTAB
Address: 0x1000
Size: 0x10
- Name: .dynamic
Type: SHT_DYNAMIC
Entries:
- Tag: DT_NEEDED
Value: 0x10
- Tag: DT_NEEDED
Value: 0x0F
- Tag: DT_STRTAB
Value: 0x1000
- Tag: DT_STRSZ
Value: 0x10
- Tag: DT_NULL
Value: 0x0
ProgramHeaders:
- Type: PT_LOAD
VAddr: 0x1000
FirstSec: .dynstr
LastSec: .dynamic
- Type: PT_DYNAMIC
FirstSec: .dynamic
LastSec: .dynamic
21 changes: 19 additions & 2 deletions llvm/tools/llvm-objdump/ELFDump.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,24 @@ static Expected<StringRef> getDynamicStrTab(const ELFFile<ELFT> &Elf) {
if (!DynamicEntriesOrError)
return DynamicEntriesOrError.takeError();

typename ELFT::Xword StringTableSize{0};
Copy link
Member

@MaskRay MaskRay Feb 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

llvm style prefers = 0 to {0}. and = nullptr below

Copy link
Contributor Author

@cabbaken cabbaken Feb 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the constructor is marked as explicit. Try this:

typename ELFT::Xword StringTableSize{0};

This worked locally for me with a Visual Studio build at least.
#125679 (comment)

I followed this. Do you have any suggestions on which style is the correct one to use here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typename ELFT::Xword StringTableSize = 0;

Copy link
Contributor Author

@cabbaken cabbaken Feb 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typename ELFT::Xword StringTableSize = 0;

I got error with this.

No viable conversion from 'int' to 'typename ELFType<llvm::endianness::little, false>::Xword' (aka 'packed_endian_specific_integral<unsigned long, (llvm::endianness)1, 1>')

I have discussed this with @jh7370 here, I initially use

typename ELFT::Xword StringTableSize = (typename ELFT::Xword)0;

which is unnecessarily redundant, then I modified to

auto StringTableSize = (typename ELFT::Xword)0;

but @jh7370 recommended

typename ELFT::Xword StringTableSize{0};

const uint8_t *MappedAddr = nullptr;
for (const typename ELFT::Dyn &Dyn : *DynamicEntriesOrError) {
if (Dyn.d_tag == ELF::DT_STRTAB) {
auto MappedAddrOrError = Elf.toMappedAddr(Dyn.getPtr());
if (!MappedAddrOrError)
return MappedAddrOrError.takeError();
return StringRef(reinterpret_cast<const char *>(*MappedAddrOrError));
MappedAddr = *MappedAddrOrError;
}
if (Dyn.d_tag == ELF::DT_STRSZ)
StringTableSize = Dyn.getVal();
}
if (MappedAddr && StringTableSize)
return StringRef(reinterpret_cast<const char *>(MappedAddr),
StringTableSize);

// If the dynamic segment is not present, we fall back on the sections.
// If the dynamic segment is not present, or is missing the important tags, we
// fall back on the sections.
auto SectionsOrError = Elf.sections();
if (!SectionsOrError)
return SectionsOrError.takeError();
Expand Down Expand Up @@ -221,6 +229,7 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
std::string TagFmt = " %-" + std::to_string(MaxLen) + "s ";

outs() << "\nDynamic Section:\n";

for (const typename ELFT::Dyn &Dyn : DynamicEntries) {
if (Dyn.d_tag == ELF::DT_NULL)
continue;
Expand All @@ -235,6 +244,14 @@ template <class ELFT> void ELFDumper<ELFT>::printDynamicSection() {
Expected<StringRef> StrTabOrErr = getDynamicStrTab(Elf);
if (StrTabOrErr) {
const char *Data = StrTabOrErr->data();
if (Dyn.getVal() >= StrTabOrErr->size()) {
reportWarning("invalid string table offset, string table size: 0x" +
Twine::utohexstr(StrTabOrErr->size()),
Obj.getFileName());
outs() << format(TagFmt.c_str(), Str.c_str())
<< format(Fmt, (uint64_t)Dyn.getVal());
continue;
}
outs() << format(TagFmt.c_str(), Str.c_str()) << Data + Dyn.getVal()
<< "\n";
continue;
Expand Down
Loading