Skip to content

Commit 488fd8a

Browse files
committed
fixup! fixup! Fix ARM64 stack corruption in ghost_stack by skipping internal frames
1 parent 542a807 commit 488fd8a

File tree

1 file changed

+22
-7
lines changed

1 file changed

+22
-7
lines changed

src/memray/_memray/ghost_stack/src/ghost_stack.cpp

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ extern "C" void ghost_ret_trampoline();
5252
// Logging (minimal, stderr only)
5353
// ============================================================================
5454

55-
// #define DEBUG
55+
#define DEBUG 1
5656
#ifdef DEBUG
5757
#define LOG_DEBUG(...) do { fprintf(stderr, "[GS] " __VA_ARGS__); fflush(stderr); } while(0)
5858
#else
@@ -302,42 +302,57 @@ class GhostStackImpl {
302302
#ifdef __linux__
303303
unw_save_loc_t loc;
304304
int save_loc_ret = unw_get_save_loc(&cursor, GS_RA_REGISTER, &loc);
305+
LOG_DEBUG("cap_inst[%zu]: ip=0x%lx sp(fp)=0x%lx actual_sp=0x%lx\n",
306+
frame_idx, (unsigned long)ip, (unsigned long)sp, (unsigned long)actual_sp);
307+
LOG_DEBUG("cap_inst[%zu]: unw_get_save_loc ret=%d type=%d addr=0x%lx\n",
308+
frame_idx, save_loc_ret, (int)loc.type, (unsigned long)loc.u.addr);
305309

306310
if (save_loc_ret == 0 && loc.type == UNW_SLT_MEMORY && loc.u.addr != 0) {
307311
ret_loc = reinterpret_cast<uintptr_t*>(loc.u.addr);
308312
}
309313
#else
310314
// macOS: return address is at fp + sizeof(void*)
315+
LOG_DEBUG("cap_inst[%zu]: ip=0x%lx sp(fp)=0x%lx actual_sp=0x%lx\n",
316+
frame_idx, (unsigned long)ip, (unsigned long)sp, (unsigned long)actual_sp);
311317
ret_loc = reinterpret_cast<uintptr_t*>(sp + sizeof(void*));
318+
LOG_DEBUG("cap_inst[%zu]: ret_loc=fp+8=%p\n", frame_idx, (void*)ret_loc);
312319
#endif
313320

314321
if (!ret_loc) {
322+
LOG_DEBUG("cap_inst[%zu]: no ret_loc, breaking\n", frame_idx);
315323
break;
316324
}
317325

318326
uintptr_t ret_addr = *ret_loc;
327+
LOG_DEBUG("cap_inst[%zu]: ret_addr=0x%lx (at %p)\n",
328+
frame_idx, (unsigned long)ret_addr, (void*)ret_loc);
319329

320330
// Strip PAC (Pointer Authentication Code) if present.
321331
// On ARM64 with PAC, return addresses have authentication bits
322332
// that must be stripped before comparison or storage.
323333
uintptr_t stripped_ret_addr = ptrauth_strip(ret_addr);
334+
LOG_DEBUG("cap_inst[%zu]: stripped_ret_addr=0x%lx trampoline=0x%lx\n",
335+
frame_idx, (unsigned long)stripped_ret_addr,
336+
(unsigned long)reinterpret_cast<uintptr_t>(ghost_ret_trampoline));
324337

325338
// Check if already patched (cache hit)
326339
// Compare against stripped address since trampoline address doesn't have PAC
327340
if (stripped_ret_addr == reinterpret_cast<uintptr_t>(ghost_ret_trampoline)) {
328341
found_existing = true;
342+
LOG_DEBUG("cap_inst[%zu]: found existing trampoline\n", frame_idx);
329343
break;
330344
}
331345

332346
// Store the stack pointer that the trampoline will pass.
333347
// This allows longjmp detection by comparing against the stored value.
334348
//
335-
// On x86_64: RET pops return address, so trampoline sees ret_loc + 8
336-
// On ARM64: RET doesn't touch SP. The trampoline receives the actual SP
337-
// at the moment of return (after the function's epilogue ran).
338-
// This is the value from UNW_REG_SP, not the FP (UNW_AARCH64_X29).
339-
#ifdef GS_ARCH_AARCH64
340-
uintptr_t expected_sp = actual_sp; // Actual SP at this frame
349+
// The trampoline passes different SP values depending on platform:
350+
// - x86_64: RET pops return address, so trampoline sees ret_loc + 8
351+
// - Linux ARM64: Trampoline passes SP after saving registers, which
352+
// corresponds to actual_sp from libunwind
353+
// - macOS ARM64: Trampoline passes ret_loc + 8 (similar to x86_64)
354+
#if defined(GS_ARCH_AARCH64) && defined(__linux__)
355+
uintptr_t expected_sp = actual_sp; // Linux ARM64: use actual SP
341356
#else
342357
uintptr_t expected_sp = reinterpret_cast<uintptr_t>(ret_loc) + sizeof(void*);
343358
#endif

0 commit comments

Comments
 (0)