Conversation
- need to test Signed-off-by: sdashmiz <shadi.dashmiz@amd.com>
There was a problem hiding this comment.
Pull request overview
This PR updates HIP fat binary parsing to avoid out-of-bounds reads when identifying compressed/uncompressed/ELF code objects by adding explicit image-size bounds checks to the magic/header comparisons.
Changes:
- Extend code-object type detection helpers to accept an
image_sizeargument and guardmemcmpcalls with size checks. - Add an ELF magic (0x7F 'E' 'L' 'F') pre-check before reading ELF header fields.
- Derive an
image_sizefor file-mapped images and pass it through the detection logic.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Determine image size for bounds checking in magic string comparisons. | ||
| // For file-mapped images use the known file size; for direct-pointer images | ||
| // (compiler-embedded or user-provided) the size is not tracked, so use SIZE_MAX | ||
| // to allow the checks to proceed (caller is responsible for valid data). | ||
| const size_t image_size = (image_mapped_ && ufd_) | ||
| ? (ufd_->fsize_ - foffset_) | ||
| : SIZE_MAX; |
There was a problem hiding this comment.
image_size falls back to SIZE_MAX when the buffer size is unknown, but this file doesn’t include a header that guarantees SIZE_MAX is defined on all toolchains. Also, other code in this repo typically uses std::numeric_limits<size_t>::max() (e.g. projects/clr/hipamd/src/hip_memory.cpp) instead of SIZE_MAX. Consider switching to std::numeric_limits<size_t>::max() (and including <limits>), or explicitly including the header that defines SIZE_MAX.
1 participant
Motivation
Technical Details
JIRA ID
Test Plan
Test Result
Submission Checklist