Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect Behaviour: Arm64 CopyLdrLiteral rewrites Prefetch as Memory Read, which can Trash a Potentially Used Register #306

Open
Sewer56 opened this issue Nov 2, 2023 · 0 comments
Labels
bug Something isn't working

Comments

@Sewer56
Copy link

Sewer56 commented Nov 2, 2023

Describe the bug

This is a sister bug to:

In the following detours code

Detours/src/disasm.cpp

Lines 4226 to 4261 in 4b8c659

BYTE CDetourDis::CopyLdrLiteral(BYTE* pSource, BYTE* pDest, ULONG instruction)
{
LdrLit19& decoded = (LdrLit19&)(instruction);
PULONG pDstInst = (PULONG)(pDest);
BYTE* pTarget = pSource + decoded.Imm();
LONG64 delta = pTarget - pDest;
// output as LDR
if (delta >= -(1 << 21) && delta < (1 << 21))
{
EmitInstruction(pDstInst, LdrLit19::Assemble(decoded.s.Size, decoded.s.FpNeon, decoded.s.Rt, (LONG)delta));
}
// output as move immediate
else if (decoded.s.FpNeon == 0)
{
UINT64 value = 0;
switch (decoded.s.Size)
{
case 0: value = *(ULONG*)pTarget; break;
case 1: value = *(UINT64*)pTarget; break;
case 2: value = *(LONG*)pTarget; break;
}
EmitMovImmediate(pDstInst, decoded.s.Rt, value);
}
// FP/NEON register: compute address in x17 and load from there (BIG assumption that x17 isn't being used for anything!!)
else
{
EmitMovImmediate(pDstInst, 17, (ULONG_PTR)pTarget);
EmitInstruction(pDstInst, LdrFpNeonImm9::Assemble(2 + decoded.s.Size, decoded.s.Rt, 17, 0));
}
return (BYTE)((BYTE*)pDstInst - pDest);
}

which rewrites LDR Literal, the PRFM (Prefetch) instruction is rewritten as a load.

This happens because LDR literal and PRFM use the same opcode, and thus the LDR code path would be used for PRFM

Detours/src/disasm.cpp

Lines 3967 to 3969 in 4b8c659

} else if ((Instruction & 0x3b000000) == 0x18000000) {
CopiedSize = CopyLdrLiteral(pSrc, pDst, Instruction);
} else {

e.g. PRFM PLIL1KEEP, #0 is 0xD8000008. (0xD8000008 & 0x3b000000) == 0x18000000 is true, thus the LDR code path is executed for PRFM.

Expected behavior

Prefetch operation should not be rewritten as a load, as this will trash the existing value in the register, which may still be used by the function.

Additional context

I'm building a cross platform, multi architecture hooking library [big WIP]; and I found detours' source to be pretty invaluable as a reference for code rewriting.

This is just a small thing I noticed while working on code rewriting on my end while referencing detours.

@Sewer56 Sewer56 added the bug Something isn't working label Nov 2, 2023
@Sewer56 Sewer56 changed the title Incorrect Behaviour: Arm64 CopyLdrLiteral rewrites Prefetch as Memory Read Incorrect Behaviour: Arm64 CopyLdrLiteral rewrites Prefetch as Memory Read, which can Trash a Potentially Used Register Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant