On Fri, Jan 11, 2019 at 7:15 AM Josh Poimboeuf <jpoim...@redhat.com> wrote: > > > > > Now, in the int3 handler can you take the faulting RIP and search for it in > > the “static-calls” table, writing the RIP+5 (offset) into R10 (return > > address) and the target into R11. You make the int3 handler to divert the > > code execution by changing pt_regs->rip to point to a new function that > > does: > > > > push R10 > > jmp __x86_indirect_thunk_r11 > > > > And then you are done. No? > > IIUC, that sounds pretty much like what Steven proposed: > > https://lkml.kernel.org/r/20181129122000.7fb4f...@gandalf.local.home > > I liked the idea, BUT, how would it work for callee-saved PV ops? In > that case there's only one clobbered register to work with (rax).
Actually, there's a much simpler model now that I think about it. The BP fixup just fixes up %rip to to point to "bp_int3_handler". And that's just a random text address set up by "text_poke_bp()". So how about the static call rewriting simply do this: - for each static call: 1) create a fixup code stub that does push $returnaddressforTHIScall jmp targetforTHIScall 2) do on_each_cpu(do_sync_core, NULL, 1); to make sure all CPU's see this generated code 3) do text_poke_bp(addr, newcode, newlen, generatedcode); Ta-daa! Done. In fact, it turns out that even the extra "do_sync_core()" in #2 is unnecessary, because taking the BP will be serializing on the CPU that takes it, so we can skip it. End result: the text_poke_bp() function will do the two do_sync_core IPI's that guarantee that by the time it returns, no other CPU is using the generated code any more, so it can be re-used for the next static call fixup. Notice? No odd emulation, no need to adjust the stack in the BP handler, just the regular "return to a different IP". Now, there is a nasty special case with that stub, though. So nasty thing with the whole "generate a stub for each call" case: because it's dynamic and because of the re-use of the stub, you could be in the situation where: CPU1 CPU2 ---- ---- generate a stub on_each_cpu(do_sync_core..) text_poke_bp() ... rewrite to BP trigger the BP return to the stub fun the first instruction of the stub *INTERRUPT causes rescheduling* on_each_cpu(do_sync_core..) rewrite to good instruction on_each_cpu(do_sync_core..) free or re-generate the stub !! The stub is still in use !! So that simple "just generate the stub dynamically" isn't so simple after all. But it turns out that that is really simple to handle too. How do we do that? We do that by giving the BP handler *two* code sequences, and we make the BP handler pick one depending on whether it is returning to a "interrupts disabled" or "interrupts enabled" case. So the BP handler does this: - if we're returning with interrupts disabled, pick the simple stub - if we're returning with interrupts enabled, clkear IF in the return %rflags, and pick a *slightly* more complex stub: push $returnaddressforTHIScall sti jmp targetforTHIScall and now the STI shadow will mean that this sequence is uninterruptible. So we'd not do complex emulation of the call instruction at BP time, but we'd do that *trivial* change at BP time. This seems simple, doesn't need any temporary registers at all, and doesn't need any extra stack magic. It literally needs just a trivial sequence in poke_int3_handler(). The we'd change the end of poke_int3_handler() to do something like this instead: void *newip = bp_int3_handler; .. if (new == magic_static_call_bp_int3_handler) { if (regs->flags &X86_FLAGS_IF) { newip = magic_static_call_bp_int3_handler_sti; regs->flags &= ~X86_FLAGS_IF; } regs->ip = (unsigned long) newip; return 1; AAND now we're *really* done. Does anybody see any issues in this? Linus