Avoid UBSan wrap in ElfSection vaddr arithmetic - #795
Conversation
p_vaddr is an unrelated virtual address, not an offset within _base; subtracting it via pointer arithmetic can wrap through a null representation, which UBSan flags. Do the subtraction in integer space and reinterpret, matching this file's existing integer-space validation.
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
CI Test ResultsRun: #35252608321 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-17 17:43:15 UTC |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d19ff7ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 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. |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
What does this PR do?:
Avoids undefined behavior (UBSan flagged) in
ElfSection's vaddr handling insymbols_linux.cpp:p_vaddris an unrelated virtual address, not an offset within the_baseallocation; subtracting it via pointer arithmetic can wrap through a null representation. The subtraction now happens in integer space before reinterpreting, matching the file's existing integer-space validation pattern.Motivation:
UBSan flags this arithmetic; it must be fixed before sanitizer CI can pass for the reference-chains work (PROF-15341). Kept as a tiny standalone PR at the bottom of the stack so reviewers see it in seconds.
Additional Notes:
Bottom of a stacked series for PROF-15341 (reference chains). No functional change.
How to test the change?:
Existing
symbols-related tests; optionally run an UBSan build over the ELF parsing paths.For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.