Skip to content

Avoid UBSan wrap in ElfSection vaddr arithmetic #795

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
jbachorik merged 1 commit into from
Sep 17, 2026
+29 −7
Merged

Avoid UBSan wrap in ElfSection vaddr arithmetic #795

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 29 additions & 7 deletions ddprof-lib/src/main/cpp/symbols_linux.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,15 @@ void ElfParser::calcVirtualLoadAddress() {
for (int i = 0; i < _header->e_phnum; i++) {
ElfProgramHeader* pheader = phdrAt(i);
if (pheader != NULL && pheader->p_type == PT_LOAD) {
_vaddr_diff = _base - pheader->p_vaddr;
// p_vaddr is an unrelated virtual address, not an offset within the
// _base allocation - subtracting it via pointer arithmetic can wrap
// to (or through) a null representation, which UBSan flags even
// though the resulting bit pattern is only ever used as an offset
// to add back later (at()/base()/dyn_ptr() above). Do the
// subtraction in integer space and reinterpret, matching this
// file's existing "validate in integer space before forming a
// pointer" pattern (see phdrAt() above).
_vaddr_diff = (const char*)((uintptr_t)_base - (uintptr_t)pheader->p_vaddr);
return;
}
}
Expand All @@ -598,7 +606,9 @@ void ElfParser::parseDynamicSection() {
uint32_t nsyms = 0;

const char* dyn_start = at(dynamic);
const char* dyn_end = dyn_start + dynamic->p_memsz;
// at(dynamic) is NULL when dynamic->p_vaddr == 0 - same null-base
// pointer-arithmetic UB as the other fixes in this file.
const char* dyn_end = (const char*)((uintptr_t)dyn_start + dynamic->p_memsz);
for (ElfDyn* dyn = (ElfDyn*)dyn_start; dyn < (ElfDyn*)dyn_end; dyn++) {
switch (dyn->d_tag) {
case DT_SYMTAB:
Expand Down Expand Up @@ -665,7 +675,11 @@ void ElfParser::parseDynamicSection() {
loadSymbolTable(symtab, syment * nsyms, syment, strtab, strsz);
}

const char* base = this->base();
// base() is NULL for ET_EXEC (non-PIE) images - adding r->r_offset to it
// via pointer arithmetic is UB (base + r->r_offset on a null base), even
// though the intent is just "sym addresses are already absolute". Do the
// addition in integer space, same fix as the .plt case above.
uintptr_t base_addr = (uintptr_t)this->base();
if (jmprel != NULL && pltrelsz != 0) {
// Parse .rela.plt table
for (size_t offs = 0; offs < pltrelsz; offs += relent) {
Expand All @@ -674,7 +688,7 @@ void ElfParser::parseDynamicSection() {
if (sym->st_name != 0) {
const char* sym_name = strAt(strtab, strsz, sym->st_name);
if (sym_name != NULL) {
_cc->addImport((void**)(base + r->r_offset), sym_name);
_cc->addImport((void**)(base_addr + r->r_offset), sym_name);
}
}
}
Expand All @@ -691,7 +705,7 @@ void ElfParser::parseDynamicSection() {
if (sym->st_name != 0) {
const char* sym_name = strAt(strtab, strsz, sym->st_name);
if (sym_name != NULL) {
_cc->addImport((void**)(base + r->r_offset), sym_name);
_cc->addImport((void**)(base_addr + r->r_offset), sym_name);
}
}
}
Expand Down Expand Up @@ -736,7 +750,10 @@ void ElfParser::parseDwarfInfo() {
for (int i = 0; i < _header->e_phnum; i++) {
ElfProgramHeader* ph = phdrAt(i);
if (ph != NULL && ph->p_type == PT_LOAD) {
const char* seg_end = at(ph) + ph->p_memsz;
// at(ph) is NULL when ph->p_vaddr == 0 (a real, if rare, case for
// the first LOAD segment of some binaries) - same null-base
// pointer-arithmetic UB as the other fixes in this file.
Comment on lines +753 to +755

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Correct the null-address rationale for ET_DYN images

For PIE/shared-object (ET_DYN) images whose first PT_LOAD has p_vaddr == 0, at(ph) normally returns _vaddr_diff (the non-null load bias), not NULL; only the ET_EXEC or zero-load-bias cases can produce the claimed null address. This comment therefore misdescribes the common ELF layout and should identify the actual condition being handled rather than attributing it to p_vaddr == 0.

AGENTS.md reference: AGENTS.md:L466-L467

Useful? React with 👍 / 👎.

const char* seg_end = (const char*)((uintptr_t)at(ph) + ph->p_memsz);
if (seg_end > image_end) image_end = seg_end;
}
}
Expand Down Expand Up @@ -792,7 +809,12 @@ void ElfParser::loadSymbols(bool use_debug) {
_cc->setPlt(plt->sh_addr, plt->sh_size);
ElfSection* reltab = findSection(SHT_RELA, ".rela.plt");
if (reltab != NULL || (reltab = findSection(SHT_REL, ".rel.plt")) != NULL) {
addRelocationSymbols(reltab, base() + plt->sh_addr + PLT_HEADER_SIZE);
// base() is NULL for ET_EXEC (non-PIE) images - adding a non-zero
// offset to it via pointer arithmetic is UB even though the intent
// is just "no adjustment needed, sh_addr is already absolute".
// Compute in integer space and cast once, same fix as
// calcVirtualLoadAddress()'s _vaddr_diff computation above.
addRelocationSymbols(reltab, (const char*)((uintptr_t)base() + (uintptr_t)plt->sh_addr + PLT_HEADER_SIZE));
}
}
}
Expand Down
Loading