Skip to content

Fix stack-buffer-overflow sanitizer issue in AUIPC_JALR_TO_XQCI.test#922

Open
parth-07 wants to merge 1 commit intoqualcomm:release/22.xfrom
parth-07:XQCJSanIssue
Open

Fix stack-buffer-overflow sanitizer issue in AUIPC_JALR_TO_XQCI.test#922
parth-07 wants to merge 1 commit intoqualcomm:release/22.xfrom
parth-07:XQCJSanIssue

Conversation

@parth-07
Copy link
Contributor

@parth-07 parth-07 commented Mar 6, 2026

This commit fixes stack-buffer-overflow sanitizer issue in AUIPIC_JALR_TO_XQCI.test. The root cause was that the QC_E_J relaxation patch 6 bytes instruction but the 'Instr' parameter of 'RegionFragmentEx::replaceInstruction' that stores the updated instruction is only of 4 bytes.

This commit fixes stack-buffer-overflow sanitizer issue in
AUIPIC_JALR_TO_XQCI.test. The root cause was that the QC_E_J relaxation
patch 6 bytes instruction but the 'Instr' parameter of
'RegionFragmentEx::replaceInstruction' that stores the updated
instruction is only of 4 bytes.

Signed-off-by: Parth Arora <partaror@qti.qualcomm.com>
@partaror partaror requested review from lenary and quic-seaswara March 6, 2026 14:03
static bool classof(const RegionFragmentEx *) { return true; }

bool replaceInstruction(uint32_t Offset, Relocation *Reloc, uint32_t Instr,
bool replaceInstruction(uint32_t Offset, Relocation *Reloc, uint64_t Instr,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

templatized function might be better

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is just doing a memcpy, maybe we can just expose Instr as const char * to avoid templating or issues with sizing, and pass &Instr everywhere we're calling it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@quic-seaswara I like @lenary's suggestion. Please let me know your thoughts on it.

Copy link
Contributor

@quic-seaswara quic-seaswara Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good!, const uint8_t *

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically a const uint8_t * isn't the same as a const char * in terms of strict aliasing, BUT you use the former so much in the codebase I'm not going to complain if you keep using it in this interface too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we should be consistent, for now const char * is ok, but we should clean it up when dealing with ELF data as const uint8_t * as much as possible

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants