[dayatsin-amd]

Summary

Hardens the ROCr ELF code-object loader against out-of-bounds heap reads driven by attacker-controlled section/symbol indices when parsing untrusted code objects (reported in SWSPLAT-45972).

GElfImage::section() indexed the sections vector with unvalidated sh_link, sh_info, and st_shndx fields, and GElfSymbolTable::symbol() indexed the symbols vector with the unvalidated relocation r_info symbol index. A crafted .hsaco could supply an out-of-range index, causing std::vector::operator[] to read past the vector's backing array and later dereference a garbage pointer (deterministic SIGSEGV / DoS, with conditional info-disclosure).

This complements the already-merged loader hardening in #6994 (segment loading, note parsing, relocation offsets), which did not cover these index-based vectors.

Changes in libamdhsacode/amd_elf_image.cpp:

• section(): return nullptr for out-of-range indices instead of reading past the vector.
• getStringTable(): reject out-of-range sh_link.
• getSymtab() / getDynsym(): null-check the section() result before ->type().
• GElfSymbol::section(): reject reserved indices (>= SHN_LORESERVE) so a crafted st_shndx cannot bypass the SHN_UNDEF check.
• GElfSymbolTable::symbol(): bounds-check the r_info symbol index.
• GElfSymbolTable::pullData() / GElfRelocationSection::pullData(): reject the code object when the referenced string/symbol/target sections are out of range. Note sh_info == 0 (the null section) is preserved as legitimate for dynamic relocation sections, so the index is range-checked rather than the returned pointer.

Changes in loader/executable.cpp:

• Null-check GetSection() / section() / symbol() results in LoadDefinitionSymbol, SymbolAddress, ApplyStaticRelocation, and ApplyDynamicRelocation, returning HSA_STATUS_ERROR_INVALID_CODE_OBJECT for malformed input.

JIRA ID

JIRA ID : SWSPLAT-45972

Test plan

Verified on a gfx1100 (Radeon RX 7900) system, building libhsa-runtime64 against a current TheRock toolchain (ROCm 7.14.0a20260612, amdclang++). Compared this branch ("FIXED") against the unmodified develop build ("BASELINE").

• Build cleanly against the toolchain — no errors, no new warnings on the changed files.
• Load well-formed code objects and run existing kernels; confirm no regression.
  • HIP vadd (1,048,576 elements) launched via the loader: correct results.
  • hsa_executable_load_agent_code_object on valid gfx1100 code objects (with and without dynamic relocations): HSA_STATUS_SUCCESS (identical to baseline). This caught and fixed a regression where a dynamic-relocation section (sh_info == 0) was initially over-rejected.
• Load crafted code objects with out-of-range sh_link / sh_info / st_shndx / r_info symbol index; confirm clean rejection instead of a crash.

Crafted code object	BASELINE (develop)	FIXED (this PR)
valid object (control)	HSA_STATUS_SUCCESS	HSA_STATUS_SUCCESS
sh_link OOB in .symtab	SIGSEGV	INVALID_CODE_OBJECT
sh_link OOB in .dynsym	SIGSEGV	INVALID_CODE_OBJECT
sh_info OOB in .rela.dyn	SIGSEGV	INVALID_CODE_OBJECT
sh_link OOB in .rela.dyn	SIGSEGV	INVALID_CODE_OBJECT
st_shndx OOB in symbol entry	SIGSEGV	INVALID_CODE_OBJECT
r_info symbol index OOB in relocation	SIGSEGV	INVALID_CODE_OBJECT

All six attacker-controlled index vectors crash the unpatched loader and are cleanly rejected with HSA_STATUS_ERROR_INVALID_CODE_OBJECT after this change.

• HIP Catch2 module / code-object suite (gfx1100) — no regression vs. baseline libhsa-runtime64.
  • unit/module/ModuleTest: 60 passed / 2 failed / 5 skipped — identical on FIXED and BASELINE. The 2 failures (hipModuleGetGlobal.cc:38, "invalid argument") reproduce byte-for-byte on the unmodified develop build, so they are pre-existing/environmental (single-GPU box) and not introduced by this change.
  • unit/module/module: all 8 test cases pass (1,000,397 assertions) on both FIXED and BASELINE.

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp, projects/rocr-runtime/runtime/hsa-runtime/loader/executable.cpp; no test file found
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[therock-pr-bot]

🚫 Please fix the failed policies before requesting reviews.

The following policy checks failed:

• ❌ Unit Test

The Not ready to Review label has been added to this PR.
Once all policies pass, the label will be removed automatically.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

Reviewed the ELF-loader bounds-checking hardening. executable.cpp null guards (LoadDefinitionSymbol, SymbolAddress, ApplyStaticRelocation, ApplyDynamicRelocation) and the section()/symbol()/getStringTable() bounds checks in amd_elf_image.cpp look correct and fail closed. A few non-blocking notes inline.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author update

All three prior findings are resolved in commit 3df30b6:

• [Question] section(0) — confirmed via pullElf() that an SHT_NULL section is stored as an empty unique_ptr, so section(0) genuinely returns nullptr; the routing rationale holds. Concern dropped.
• [Optional] getStringTable() type confusion — fixed: now rejects an in-range wrong-type section (!s || s->type() != SHT_STRTAB).
• [Optional] audit remaining section() callers — getReferencedSymbolTable() had the same raw static_cast and is now hardened to require SHT_SYMTAB/SHT_DYNSYM; other callers were already null-guarded.

The follow-up commit adds real type-confusion hardening beyond the original scope. One non-blocking, out-of-scope note for a possible separate follow-up: the amd_hsa_code.cpp symbol accessors (Symbol::IsAgent/IsSegment/GetAlignment/IsConst) still dereference elfsym->section() without a null check — a crafted out-of-range st_shndx would now yield a deterministic null-deref (DoS only, not the prior exploitable OOB + garbage virtual dispatch), so this PR is a strict improvement there. Looks good to me.

[amd-justchen]

Will merge given code owner approval.
The TheRock CI failures are only known infra issues, unrelated to the rocr ELF loader bounds-check change:

• Windows (gfx1151) / Build Windows Packages: known missing win_flex/winflexbison toolchain in amd-mesa (meson), fixed forward by ci(windows): install winflexbison3 for the amd-mesa flex/bison dependency #10951
• Linux hip-tests (gfx94X) shard 2 of 4: 1 of 1254 tests, the known-flaky Unit_HRR_ZeroInitRoundtrip roundtrip test, unrelated to a loader bounds-check
• Linux rocgdb-gpu (gfx94X): expected-fail (xfail) lane
• Linux MI455 Test sanity: container sanity check cancelled, zero signal

[systems-assistant]

TheRock Submodule Bump Activity

Newest first

• 2026-09-01 — Included in TheRock via ROCm/TheRock#7793.