| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
Sorry, something went wrong.
CI Test ResultsRun: #35862201798 | Commit: c41b6a8 | Duration: 51m 11s (longest job)
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-23 13:37:11 UTC |
Sorry, something went wrong.
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: a5e34de3ac
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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".
Sorry, something went wrong.
There was a problem hiding this comment.
A damaged secondary PT_LOAD address can overflow before the new bounds check runs. This fault can still crash the profiler.
🤖 Datadog Autotest · Commit a5e34de · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Sorry, something went wrong.
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fb47b3445
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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".
Sorry, something went wrong.
| uint32_t* chain_end = &buckets[nbuckets]; | ||
| uint64_t chain_offset_bytes = (uint64_t)symoffset * sizeof(uint32_t); | ||
| if (chain_offset_bytes > (uintptr_t)chain_end) { | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
Reject GNU hash buckets below symoffset
When a malformed GNU hash has a nonzero bucket smaller than symoffset (for example, bucket 1 and symoffset 4), this calculation is still accepted. The subsequent chain[nsyms] then reads bloom or bucket words before the actual chain; an odd bucket value can terminate that scan and inflate nsyms, causing loadSymbolTable() to interpret unrelated mapped bytes as dynamic symbols. Reject any nonzero bucket value below symoffset before deriving the chain pointer.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
| // entries, one inLiveImage() (O(e_phnum)) call at a time, while | ||
| // Symbols::_parse_lock is held. | ||
| bool terminated = false; | ||
| for (uint32_t scanned = 0; scanned < MAX_GNU_HASH_CHAIN_SCAN; scanned++) { |
There was a problem hiding this comment.
Limit GNU hash scans to a small bounded range
With a corrupted GNU hash whose bucket is in range but whose chain lacks a terminator, this loop performs 1,048,576 inLiveImage() calls; each call rescans program headers while Symbols::_parse_lock is held. That lets a malformed DSO encountered during profiler startup create a substantial global stall. Use a small bounded scan or validate the chain range once instead of retaining the one-million-entry cap.
AGENTS.md reference: AGENTS.md:L385-L386
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
What does this PR do?:
Hardens ElfParser's parsing of the PT_DYNAMIC section and related virtual-address-relative structures (DT_HASH, DT_GNU_HASH, DT_SYMTAB/DT_STRTAB, .rela.plt/.rela.dyn relocation tables, the SFrame/eh_frame_hdr unwind sections) against corrupted or malformed ELF metadata found in a live-mapped shared library.
Motivation:
Production crash since tracer v1.56.1:
The DT_HASH case in parseDynamicSection() dereferenced a dyn_ptr()-derived pointer with no bounds check — a single malformed DT_HASH entry crashed the process. This was the one path in symbols_linux.cpp not routed through any bounds check (everything else already used inImage() for file-offset-relative data). Rather than special-casing just DT_HASH, this fixes the underlying gap: every virtual-address-relative pointer computed via at()/dyn_ptr() in this file is now validated the same way, so the same class of bug can't resurface at DT_GNU_HASH, the relocation tables, or the SFrame/eh_frame_hdr sections.
Additional Notes:
How to test the change?:
ddprof-lib/src/test/cpp/elfparser_ut.cpp adds extensive coverage, including:
Run on Linux: ./gradlew :ddprof-lib:gtestDebug_elfparser_ut (or gtestRelease_elfparser_ut).
For Datadog employees:
credentials of any kind, I've requested a security review (run the dd:platform-security-review
skill, or file a request via the PSEC review form).
bewaire also runs automatically on every PR.
Unsure? Have a question? Request a review!