[llvm] [llvm][ELF]Add Shdr check for getBuildID (PR #126537)
Ruoyu Qiu via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 21 19:36:55 PDT 2025
cabbaken wrote:
After reading the unit test I wrote, I noticed something curious. In [`ELF.h`](https://github.com/cabbaken/llvm-project/blob/126418/llvm/include/llvm/Object/ELF.h#L410):
```c++
Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const {
assert(Phdr.p_type == ELF::PT_NOTE && "Phdr is not of type PT_NOTE");
ErrorAsOutParameter ErrAsOutParam(Err);
if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) {
Err =
createError("invalid offset (0x" + Twine::utohexstr(Phdr.p_offset) +
") or size (0x" + Twine::utohexstr(Phdr.p_filesz) + ")");
return Elf_Note_Iterator(Err);
}
```
The `p_filesz` in the unit test is set to `0x0xffffffffffffffff`. This cause `Phdr.p_offset + Phdr.p_filesz` to overflow, so `return Elf_Note_Iterator(Err);` is skipped. The unit test then "succeed" based on the overflow, but it may cause an invalid memory access if the elf has no buildID. For example:
```yaml
--- !ELF
FileHeader:
Class: ELFCLASS64
Data: ELFDATA2LSB
Type: ET_EXEC
Machine: EM_X86_64
ProgramHeaders:
- Type: PT_NOTE
FileSize: 0xffffffffffffffff
FirstSec: .note.gnu.build-id
LastSec: .note.gnu.build-id
Sections:
- Name: .note.gnu.build-id
Type: SHT_NOBITS
AddressAlign: 0x04
```
Given this, I think we should avoid iterating over `notes` if the ELF has an invalid `p_filesz`, so that we don’t run into boundary issues here. The large `p_filesz` value in the test is only meant to demonstrate that we detect the section header before the program header.
I'm sorry I was misunderstanding about this:
>That the BuildID can be looked up from a section header, if there is no program header.
That the code handles a malformed program header that points at data outside the file.
That the code handles a malformed section header that points at data outside the file.
https://github.com/llvm/llvm-project/pull/126537
More information about the llvm-commits
mailing list