Release xrtdeps.sh to xrt/bin - #135
Merged
Merged
Conversation
|
Build Failed! :( |
stsoe
added a commit
to stsoe/XRT
that referenced
this pull request
Aug 5, 2026
#### Problem solved by the commit Four CWE-125 heap OOB reads in copyBufferUpdateMetadata() where aie_resources_bin mpo_* fields (mpo_name, mpo_version, m_start_column, m_num_columns) were used as raw pointer offsets without bounds checking when computing sDefault fallback strings, bypassing the bounded_mpo_cstr() guard added in bb1791f. Two additional CWE-125 OOB reads where m_image_offset and m_image_size were used without bounds checking in both copyBufferUpdateMetadata() and writeObjImage() before reading image data. #### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered Jira tickets: - AIESW-41124 - AIESW-41125 - AIESW-41126 - AIESW-41127 - AIESW-41128 - AIESW-41129 CodeQL alerts Xilinx#135-Xilinx#140 (amd-psirt/xclbin-parser-oob, HIGH severity). The sDefault reads were missed by bb1791f (SWSPLAT-30717), which protected the TRACE() and writeMetadata() paths but not the fallback default value paths in copyBufferUpdateMetadata(). The image offset/size checks were never added to writeObjImage(). #### How problem was solved, alternative solutions (if any) and why they were rejected Replaced the four raw sDefault pointer calculations with bounded_mpo_cstr(pHdr, pHdr->field, _origSectionSize). This also fixes a pre-existing double-offset bug in the original code: the sDefault expressions incorrectly added sizeof(aie_resources_bin) on top of the mpo_* field value, but mpo_* fields already store absolute offsets from pHdr (sizeof is baked in at write time), so the fallback pointer was pointing past the actual string. Added explicit uint64_t overflow-safe bounds checks for m_image_offset + m_image_size against section size in both copyBufferUpdateMetadata() and writeObjImage(). #### Risks (if any) associated the changes in the commit Low. The sDefault path is only taken when the JSON metadata stream omits a key; the double-offset bug meant the fallback was producing wrong values anyway. The image bounds checks only reject malformed xclbins. #### What has been tested and how, request additional testing if necessary Built xclbinutil successfully. Recommend testing xclbinutil --input with a crafted xclbin containing OOB mpo offsets and verifying it throws rather than reading past the buffer. #### Documentation impact (if any) None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com>
stsoe
added a commit
that referenced
this pull request
Aug 6, 2026
* CodeQL Fix OOB reads in SectionAIEResourcesBin #### Problem solved by the commit Four CWE-125 heap OOB reads in copyBufferUpdateMetadata() where aie_resources_bin mpo_* fields (mpo_name, mpo_version, m_start_column, m_num_columns) were used as raw pointer offsets without bounds checking when computing sDefault fallback strings, bypassing the bounded_mpo_cstr() guard added in bb1791f. Two additional CWE-125 OOB reads where m_image_offset and m_image_size were used without bounds checking in both copyBufferUpdateMetadata() and writeObjImage() before reading image data. #### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered Jira tickets: - AIESW-41124 - AIESW-41125 - AIESW-41126 - AIESW-41127 - AIESW-41128 - AIESW-41129 CodeQL alerts #135-#140 (amd-psirt/xclbin-parser-oob, HIGH severity). The sDefault reads were missed by bb1791f (SWSPLAT-30717), which protected the TRACE() and writeMetadata() paths but not the fallback default value paths in copyBufferUpdateMetadata(). The image offset/size checks were never added to writeObjImage(). #### How problem was solved, alternative solutions (if any) and why they were rejected Replaced the four raw sDefault pointer calculations with bounded_mpo_cstr(pHdr, pHdr->field, _origSectionSize). This also fixes a pre-existing double-offset bug in the original code: the sDefault expressions incorrectly added sizeof(aie_resources_bin) on top of the mpo_* field value, but mpo_* fields already store absolute offsets from pHdr (sizeof is baked in at write time), so the fallback pointer was pointing past the actual string. Added explicit uint64_t overflow-safe bounds checks for m_image_offset + m_image_size against section size in both copyBufferUpdateMetadata() and writeObjImage(). #### Risks (if any) associated the changes in the commit Low. The sDefault path is only taken when the JSON metadata stream omits a key; the double-offset bug meant the fallback was producing wrong values anyway. The image bounds checks only reject malformed xclbins. #### What has been tested and how, request additional testing if necessary Built xclbinutil successfully. Recommend testing xclbinutil --input with a crafted xclbin containing OOB mpo offsets and verifying it throws rather than reading past the buffer. #### Documentation impact (if any) None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com> * Fix uint64_t overflow in SectionBMC bounds check #### Problem solved by the commit copyBufferUpdateMetadata() computed `pHdr->m_offset + pHdr->m_size` as a uint64_t addition before comparing against _origSectionSize. If both fields are near UINT64_MAX the addition wraps silently, causing the bounds check to pass on a malformed section. #### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered Found during triage of AIESW-41122/41123 (false positives). The overflow was latent in the original check predating bb1791f. #### How problem was solved, alternative solutions (if any) and why they were rejected Replaced the addition with an overflow-safe two-part check: m_size > _origSectionSize || m_offset > _origSectionSize - m_size #### Risks (if any) associated the changes in the commit Low. Semantically equivalent for all non-overflow inputs; only rejects crafted inputs that previously slipped past the check. #### What has been tested and how, request additional testing if necessary Built xclbinutil successfully. #### Documentation impact (if any) None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com> * CodeQL Fix OOB reads in SectionFlash #### Problem solved by the commit Five CWE-125 heap OOB reads in SectionFlash.cxx where attacker- controlled flash mpo_* and image offset/size fields were used without bounds checking: - AIESW-41121/41120/41119: mpo_name, mpo_version, mpo_md5_value used as raw pointer offsets in the TRACE block of copyBufferUpdateMetadata() without going through bounded_mpo_cstr(), unlike the equivalent paths in SectionAIEResourcesBin which were fixed in bb1791f. - AIESW-41117/41118: m_image_offset and m_image_size used to read image data at line 281 with no bounds check against _origSectionSize. #### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered Jira: - AIESW-41117 - AIESW-41118 - AIESW-41119 - AIESW-41120 - AIESW-41121 CodeQL alerts #143-#147 (amd-psirt/xclbin-parser-oob, HIGH severity). Missed by bb1791f (SWSPLAT-30717) which fixed other mpo_* reads but left the TRACE paths and image copy in SectionFlash unguarded. #### How problem was solved, alternative solutions (if any) and why they were rejected TRACE paths: replaced raw `pHdr + mpo_field` with bounded_mpo_cstr(pHdr, mpo_field, size) in both copyBufferUpdateMetadata() and writeMetadata(). sDefault fallback strings: replaced raw `pHdr + sizeof(flash) + field` with bounded_mpo_cstr(pHdr, field, _origSectionSize), also fixing the same double-offset bug as found in SectionAIEResourcesBin where sizeof(flash) was incorrectly added on top of already-absolute mpo offsets. Image copy: added overflow-safe uint64_t bounds check before the _buffer.write() in copyBufferUpdateMetadata() and writeObjImage(). #### Risks (if any) associated the changes in the commit Low. Only rejects malformed xclbins; behavior for well-formed inputs is unchanged. #### What has been tested and how, request additional testing if necessary Built xclbinutil successfully. #### Documentation impact (if any) None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com> * CodeQL Fix OOB reads in SectionSoftKernel Six CWE-125 heap OOB reads in SectionSoftKernel.cxx where attacker- controlled soft_kernel mpo_* and image offset/size fields were used without bounds checking: - AIESW-41116/41115/41114/41113: mpo_name, mpo_version, mpo_md5_value, mpo_symbol_name used as raw pointer offsets in the TRACE block of copyBufferUpdateMetadata() without going through bounded_mpo_cstr(), unlike writeMetadata() which already used it correctly. - AIESW-41111/41112: m_image_offset and m_image_size used to read image data at line 273 with no bounds check against _origSectionSize. Jira: - AIESW-41111 - AIESW-41112 - AIESW-41113 - AIESW-41114 - AIESW-41115 - AIESW-41116 CodeQL alerts #148-#153 (amd-psirt/xclbin-parser-oob, HIGH severity). Missed by bb1791f (SWSPLAT-30717) which fixed writeMetadata() but left the TRACE paths and image copy in copyBufferUpdateMetadata() unguarded, and writeObjImage() without image bounds checking. TRACE paths: replaced raw `pHdr + mpo_field` arithmetic with bounded_mpo_cstr(pHdr, mpo_field, _origSectionSize). sDefault fallback strings: replaced raw `pHdr + sizeof(soft_kernel) + field` with bounded_mpo_cstr(pHdr, field, _origSectionSize), also fixing the same double-offset bug as found in SectionAIEResourcesBin and SectionFlash where sizeof(soft_kernel) was incorrectly added on top of already-absolute mpo offsets. Image copy: added overflow-safe uint64_t bounds check before the _buffer.write() in copyBufferUpdateMetadata() and writeObjImage(). Low. Only rejects malformed xclbins; behavior for well-formed inputs is unchanged. Built xclbinutil successfully. None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com> * AIESW-41108 AIESW-41109 AIESW-41110 Fix OOB reads in SectionVenderMetadata #### Problem solved by the commit Three CWE-125 heap OOB reads in SectionVenderMetadata.cxx where attacker-controlled vender_metadata mpo_name and image offset/size fields were used without bounds checking: - AIESW-41110: mpo_name used as raw pointer offset in the TRACE block of copyBufferUpdateMetadata() without going through bounded_mpo_cstr(), unlike writeMetadata() which already used it correctly. - AIESW-41108/41109: m_image_offset and m_image_size used to read image data at line 140 with no bounds check against _origSectionSize. #### Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered CodeQL alerts #154-#156 (amd-psirt/xclbin-parser-oob, HIGH severity). Missed by bb1791f (SWSPLAT-30717) which fixed writeMetadata() but left the TRACE path and image copy in copyBufferUpdateMetadata() unguarded, and writeObjImage() without image bounds checking. #### How problem was solved, alternative solutions (if any) and why they were rejected TRACE path: replaced raw `pHdr + mpo_name` with bounded_mpo_cstr(pHdr, pHdr->mpo_name, _origSectionSize). sDefault fallback string: replaced raw `pHdr + sizeof(vender_metadata) + mpo_name` with bounded_mpo_cstr(pHdr, pHdr->mpo_name, _origSectionSize), also fixing the same double-offset bug found in all other Section* files where sizeof(struct) was incorrectly added on top of already-absolute mpo offsets. Image copy: added overflow-safe uint64_t bounds check before the _buffer.write() in copyBufferUpdateMetadata() and writeObjImage(). #### Risks (if any) associated the changes in the commit Low. Only rejects malformed xclbins; behavior for well-formed inputs is unchanged. #### What has been tested and how, request additional testing if necessary Built xclbinutil successfully. #### Documentation impact (if any) None Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-Authored-By: Claude <noreply@anthropic.com> --------- Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.