From 0d61dc6959c7e450b08471d9e0cc21ada3b350af Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Wed, 22 Jul 2026 12:37:34 -0400 Subject: [PATCH 1/2] fix(rocr): Bounds-check ELF loader section and symbol indices Harden the ROCr ELF code-object loader against out-of-bounds heap reads driven by attacker-controlled section/symbol indices in untrusted code objects. 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. - 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/GElfRelocationSection::pullData(): fail (rejecting the code object) when the referenced string/symbol/target sections are invalid. - executable.cpp: null-check GetSection()/section()/symbol() results in LoadDefinitionSymbol, SymbolAddress, ApplyStaticRelocation and ApplyDynamicRelocation, returning HSA_STATUS_ERROR_INVALID_CODE_OBJECT. JIRA ID : SWSPLAT-45972 Co-authored-by: Cursor --- .../libamdhsacode/amd_elf_image.cpp | 52 +++++++++++++++---- .../runtime/hsa-runtime/loader/executable.cpp | 32 +++++++++--- 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp b/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp index 87f8f403ff3b..22867be5bd9e 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp @@ -742,14 +742,16 @@ namespace elf { } GElfSymbolTable* getSymtab(uint16_t index) override { - if (section(index)->type() == SHT_SYMTAB) - return static_cast(section(index)); + GElfSection* s = section(index); + if (s && s->type() == SHT_SYMTAB) + return static_cast(s); return nullptr; } GElfSymbolTable* getDynsym(uint16_t index) override { - if (section(index)->type() == SHT_DYNSYM) - return static_cast(section(index)); + GElfSection* s = section(index); + if (s && s->type() == SHT_DYNSYM) + return static_cast(s); return nullptr; } @@ -772,7 +774,12 @@ namespace elf { GElfSegment* segment(size_t i) override { return segments[i].get(); } Segment* segmentByVAddr(uint64_t vaddr) override; size_t sectionCount() override { return sections.size(); } - GElfSection* section(size_t i) override { return sections[i].get(); } + // Bounds-check attacker-controlled section indices (sh_link, sh_info, + // st_shndx). Out-of-range indices return nullptr instead of reading heap + // memory past the vector's internal array. + GElfSection* section(size_t i) override { + return i < sections.size() ? sections[i].get() : nullptr; + } Section* sectionByVAddr(uint64_t vaddr) override; uint16_t machine() const; uint16_t etype() const; @@ -1132,10 +1139,15 @@ namespace elf { Section* GElfSymbol::section() { - if (Sym()->st_shndx != SHN_UNDEF) { - return symtab->elf->section(Sym()->st_shndx); + uint16_t shndx = Sym()->st_shndx; + // Reserved indices (SHN_LORESERVE..SHN_HIRESERVE, e.g. SHN_ABS, + // SHN_COMMON) do not refer to a real section, and any index >= the + // section count is out of bounds. section() itself bounds-checks, but + // guard here so a crafted st_shndx does not bypass the SHN_UNDEF check. + if (shndx == SHN_UNDEF || shndx >= SHN_LORESERVE) { + return 0; } - return 0; + return symtab->elf->section(shndx); } bool GElfSymbol::push(const std::string& name, uint64_t value, uint64_t size, unsigned char type, unsigned char binding, uint16_t shndx, unsigned char other) @@ -1170,7 +1182,11 @@ namespace elf { bool GElfSymbolTable::pullData() { + // sh_link identifies this symbol table's string table; a crafted value + // out of section-table range yields nullptr. Reject the code object + // rather than storing a garbage string-table pointer. strtab = elf->getStringTable(hdr.sh_link); + if (!strtab) { return false; } for (size_t i = 0; i < data0.size() / sizeof(GElf_Sym); ++i) { symbols.push_back(std::unique_ptr(new GElfSymbol(this, data0, i * sizeof(GElf_Sym)))); } @@ -1204,7 +1220,10 @@ namespace elf { Symbol* GElfSymbolTable::symbol(size_t i) { - return symbols[i].get(); + // i derives from the attacker-controlled relocation r_info symbol index + // (GELF_R_SYM); reject out-of-range values instead of reading past the + // symbols vector. + return i < symbols.size() ? symbols[i].get() : nullptr; } GElfNoteSection::GElfNoteSection(GElfImage* elf) @@ -1319,8 +1338,20 @@ namespace elf { bool GElfRelocationSection::pullData() { + // sh_info/sh_link identify the target section and referenced symbol + // table and are attacker-controlled. sh_info == 0 (the SHN_UNDEF/null + // section) is legitimate: it marks a dynamic relocation section, and + // section() then returns nullptr, which the loader uses to route dynamic + // relocations. Only an out-of-range sh_info is malformed, so validate the + // index rather than the returned pointer. + if (hdr.sh_info >= elf->sectionCount()) { return false; } section = elf->section(hdr.sh_info); + // sh_link must reference the associated symbol table; + // getReferencedSymbolTable() returns nullptr for an out-of-range index + // (section() is bounds-checked). Reject rather than storing a garbage + // pointer that later gets virtual-dispatched during relocation. symtab = elf->getReferencedSymbolTable(hdr.sh_link); + if (!symtab) { return false; } Elf_Scn *lScn = elf_getscn(elf->e, ndxscn); assert(lScn); Elf_Data *lData = elf_getdata(lScn, nullptr); @@ -1651,6 +1682,9 @@ namespace elf { GElfStringTable* GElfImage::getStringTable(uint16_t index) { + // index derives from the attacker-controlled sh_link field; reject + // out-of-range values instead of reading past the sections vector. + if (index >= sections.size()) { return nullptr; } return static_cast(sections[index].get()); } diff --git a/projects/rocr-runtime/runtime/hsa-runtime/loader/executable.cpp b/projects/rocr-runtime/runtime/hsa-runtime/loader/executable.cpp index a9794bcb2765..0fbb4a038f49 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/loader/executable.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/loader/executable.cpp @@ -1707,6 +1707,11 @@ hsa_status_t ExecutableImpl::LoadDefinitionSymbol(hsa_agent_t agent, code::Symbol* sym, uint32_t majorVersion) { + // A definition symbol must reside in a real section. GetSection() returns + // nullptr for a crafted/reserved st_shndx; reject the code object here rather + // than dereferencing a null section pointer in the accessors below + // (IsAgent(), Allocation(), Alignment(), getData(), ...). + if (!sym->GetSection()) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } bool isAgent = sym->IsAgent(); if (majorVersion >= 2) { isAgent = agent.handle != 0; @@ -1875,6 +1880,9 @@ Segment* ExecutableImpl::VirtualAddressSegment(uint64_t vaddr) uint64_t ExecutableImpl::SymbolAddress(hsa_agent_t agent, code::Symbol* sym) { code::Section* sec = sym->GetSection(); + // GetSection() returns nullptr for a crafted/reserved st_shndx; avoid + // dereferencing it in SectionSegment()/VAddr(). + if (!sec) { return 0; } Segment* seg = SectionSegment(agent, sec); return nullptr == seg ? 0 : (uint64_t) (uintptr_t) seg->Address(sym->VAddr()); } @@ -1954,6 +1962,9 @@ hsa_status_t ExecutableImpl::ApplyStaticRelocation(hsa_agent_t agent, amd::hsa:: { hsa_status_t status = HSA_STATUS_SUCCESS; amd::elf::Symbol* sym = rel->symbol(); + // symbol() returns nullptr for a crafted r_info symbol index that is out of + // the symbol table's range; reject rather than dereferencing it below. + if (!sym) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } code::RelocationSection* rsec = rel->section(); code::Section* sec = rsec->targetSection(); Segment* rseg = SectionSegment(agent, sec); @@ -2016,13 +2027,16 @@ hsa_status_t ExecutableImpl::ApplyStaticRelocation(hsa_agent_t agent, amd::hsa:: case R_AMDGPU_V1_INIT_SAMPLER: { + // section() returns nullptr for a crafted/reserved st_shndx. + elf::Section* ssec = sym->section(); + if (!ssec) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } if (STT_AMDGPU_HSA_METADATA != sym->type() || - SHT_PROGBITS != sym->section()->type() || - !(sym->section()->flags() & SHF_MERGE)) { + SHT_PROGBITS != ssec->type() || + !(ssec->flags() & SHF_MERGE)) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } amdgpu_hsa_sampler_descriptor_t desc; - if (!sym->section()->getData(sym->value(), &desc, sizeof(desc))) { + if (!ssec->getData(sym->value(), &desc, sizeof(desc))) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } if (AMDGPU_HSA_METADATA_KIND_INIT_SAMP != desc.kind) { @@ -2047,14 +2061,17 @@ hsa_status_t ExecutableImpl::ApplyStaticRelocation(hsa_agent_t agent, amd::hsa:: case R_AMDGPU_V1_INIT_IMAGE: { + // section() returns nullptr for a crafted/reserved st_shndx. + elf::Section* ssec = sym->section(); + if (!ssec) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } if (STT_AMDGPU_HSA_METADATA != sym->type() || - SHT_PROGBITS != sym->section()->type() || - !(sym->section()->flags() & SHF_MERGE)) { + SHT_PROGBITS != ssec->type() || + !(ssec->flags() & SHF_MERGE)) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } amdgpu_hsa_image_descriptor_t desc; - if (!sym->section()->getData(sym->value(), &desc, sizeof(desc))) { + if (!ssec->getData(sym->value(), &desc, sizeof(desc))) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } if (AMDGPU_HSA_METADATA_KIND_INIT_ROIMG != desc.kind && @@ -2131,6 +2148,9 @@ hsa_status_t ExecutableImpl::ApplyDynamicRelocation(hsa_agent_t agent, amd::hsa: // VirtualAddressSegment() returns nullptr when no loaded segment covers the // attacker-controlled r_offset; reject rather than dereferencing it. if (!relSeg) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } + // symbol() returns nullptr for a crafted r_info symbol index that is out of + // the symbol table's range; reject rather than dereferencing it below. + if (!rel->symbol()) { return HSA_STATUS_ERROR_INVALID_CODE_OBJECT; } uint64_t symAddr = 0; switch (rel->symbol()->type()) { case STT_OBJECT: From 51768055c5dc1368f35a36c1a287e7f9eb6cb07d Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Mon, 27 Jul 2026 19:02:27 -0400 Subject: [PATCH 2/2] fix(rocr): Reject wrong-type sections in ELF string/symbol table lookups Address review feedback on the ELF loader hardening: getStringTable() and getReferencedSymbolTable() bounds-checked the section index but still static_cast an in-range section of the wrong type, leaving a residual type-confusion. Verify the referenced section is actually SHT_STRTAB (string table) or SHT_SYMTAB/SHT_DYNSYM (symbol table) respectively, and return nullptr otherwise; callers already handle nullptr. JIRA ID : SWSPLAT-45972 Co-authored-by: Cursor --- .../libamdhsacode/amd_elf_image.cpp | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp b/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp index 22867be5bd9e..e1bf894ac2b7 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/libamdhsacode/amd_elf_image.cpp @@ -738,7 +738,15 @@ namespace elf { GElfStringTable* strtab() override; GElfSymbolTable* getReferencedSymbolTable(uint16_t index) { - return static_cast(section(index)); + // index derives from an attacker-controlled sh_link. section() already + // bounds-checks and returns nullptr for an out-of-range index, but a + // crafted value may reference an in-range wrong-type section. Verify it + // is actually a symbol table so the static_cast cannot produce a + // mis-typed pointer later virtual-dispatched during relocation. + GElfSection* s = section(index); + if (s && (s->type() == SHT_SYMTAB || s->type() == SHT_DYNSYM)) + return static_cast(s); + return nullptr; } GElfSymbolTable* getSymtab(uint16_t index) override { @@ -1347,9 +1355,10 @@ namespace elf { if (hdr.sh_info >= elf->sectionCount()) { return false; } section = elf->section(hdr.sh_info); // sh_link must reference the associated symbol table; - // getReferencedSymbolTable() returns nullptr for an out-of-range index - // (section() is bounds-checked). Reject rather than storing a garbage - // pointer that later gets virtual-dispatched during relocation. + // getReferencedSymbolTable() returns nullptr for an out-of-range index or + // an in-range section that is not a symbol table. Reject rather than + // storing a garbage/mis-typed pointer that later gets virtual-dispatched + // during relocation. symtab = elf->getReferencedSymbolTable(hdr.sh_link); if (!symtab) { return false; } Elf_Scn *lScn = elf_getscn(elf->e, ndxscn); @@ -1685,7 +1694,13 @@ namespace elf { // index derives from the attacker-controlled sh_link field; reject // out-of-range values instead of reading past the sections vector. if (index >= sections.size()) { return nullptr; } - return static_cast(sections[index].get()); + // A crafted sh_link may reference an in-range but wrong-type section + // (e.g. SHT_PROGBITS). Reject it so the static_cast below cannot produce + // a mis-typed pointer later used as a string table (type confusion). + // Note: section index 0 (SHT_NULL) is stored as a null unique_ptr. + GElfSection* s = sections[index].get(); + if (!s || s->type() != SHT_STRTAB) { return nullptr; } + return static_cast(s); } GElfSymbolTable* GElfImage::addSymbolTable(const std::string& name, StringTable* stab)