diff --git a/src/Monkeypatcher.cc b/src/Monkeypatcher.cc index 23115cc823a..1efa5e233b3 100644 --- a/src/Monkeypatcher.cc +++ b/src/Monkeypatcher.cc @@ -500,8 +500,8 @@ remote_code_ptr Monkeypatcher::get_jump_stub_exit_breakpoint(remote_code_ptr ip, --it; patched_syscall *ps = &syscall_stub_list[it->second]; auto bp = it->first + ps->size - ps->safe_suffix; - if (pp == bp - 4 || pp == bp - 8) { - return remote_code_ptr((it->first + ps->size - 4).as_int()); + if (pp == bp - 4 || pp == bp - 8 || pp == bp - 12) { + return remote_code_ptr((it->first + ps->size - 12).as_int()); } return nullptr; } @@ -717,13 +717,13 @@ bool patch_syscall_with_hook_arch(Monkeypatcher& patcher, 2 * 4, /** * safe_suffix: - * We've returned from syscallbuf and continue execution - * won't hit syscallbuf breakpoint - * (this also include the 8 bytes that stores the return address) - * Note that stack restore instruction also belongs to the syscallbuf return path - * However, since it is still using the scratch memory, - * it doesn't belong to the safe area. - * The caller needs to have special handling for that instruction. + * The safe suffix are all instructions that are no longer using syscallbuf + * private stack memory. On aarch64, that is the bail path svc instruction + * and the final jump instruction (including the 8 byte return address). + * See the detailed extended jump patch assembly above for details. + * Note that the stack restore instructions also occurr on the syscallbuf + * return path, but are not considered part of the safe suffix, since they + * still rely on the syscallbuf stack memory to function properly. */ 2 * 4 + 8 }); diff --git a/src/preload/syscall_hook.S b/src/preload/syscall_hook.S index 9191f5f9a6d..a07a40cbaf0 100644 --- a/src/preload/syscall_hook.S +++ b/src/preload/syscall_hook.S @@ -891,7 +891,9 @@ retq _syscallbuf_code_start: _syscall_hook_trampoline: - // stack frame: + // parent frame: + // 0 (688): lr from the extended jump patch [this gets rewritten here in the bail path] + // this stack frame: // 208-688: q2 - q31 // 128-200: x10 - x18 // 112-128: x7, x9 @@ -952,6 +954,7 @@ _syscall_hook_trampoline: cbnz x0, 1f // If the function requested the bail path, rewrite the return address + // N.B.: This modifies the stack address saved in the parent frame. ldr x0, [sp, 688] add x0, x0, 8 str x0, [sp, 688] diff --git a/src/record_signal.cc b/src/record_signal.cc index c9a24a81e9f..c6a2d7015ab 100644 --- a/src/record_signal.cc +++ b/src/record_signal.cc @@ -298,7 +298,7 @@ bool handle_syscallbuf_breakpoint(RecordTask* t) { LOG(debug) << "Reached syscallstub exit instruction, singlestepping to " "enable signal dispatch"; ASSERT(t, t->arch() == aarch64 && t->syscallstub_exit_breakpoint); - auto retaddr_addr = t->syscallstub_exit_breakpoint.to_data_ptr() + 3 * 4; + auto retaddr_addr = t->syscallstub_exit_breakpoint.to_data_ptr() + 4; uint64_t retaddr; t->read_bytes_helper(retaddr_addr, sizeof(retaddr), &retaddr); Registers r = t->regs();