Messages in this thread | | | Date | Thu, 16 Feb 2023 08:06:19 -0800 | From | Josh Poimboeuf <> | Subject | Re: [PATCH 2/2] x86/entry: Fix unwinding from kprobe on PUSH/POP instruction |
| |
On Thu, Feb 16, 2023 at 12:58:58PM +0100, Peter Zijlstra wrote: > On Fri, Feb 10, 2023 at 02:42:02PM -0800, Josh Poimboeuf wrote: > > > The problem is that #BP saves the pointer to the instruction immediately > > *after* the INT3, rather than to the INT3 itself. The instruction > > replaced by the INT3 hasn't actually run, but ORC assumes otherwise and > > expects the wrong stack layout. > > > > Fix it by annotating the #BP exception as a non-signal stack frame, > > which tells the ORC unwinder to decrement the instruction pointer before > > looking up the corresponding ORC entry. > > > > Reported-by: Chen Zhongjin <chenzhongjin@huawei.com> > > Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org> > > --- > > arch/x86/entry/entry_64.S | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > > index 15739a2c0983..8d21881adf86 100644 > > --- a/arch/x86/entry/entry_64.S > > +++ b/arch/x86/entry/entry_64.S > > @@ -385,7 +385,14 @@ SYM_CODE_END(xen_error_entry) > > */ > > .macro idtentry vector asmsym cfunc has_error_code:req > > SYM_CODE_START(\asmsym) > > - UNWIND_HINT_IRET_REGS offset=\has_error_code*8 > > + > > + .if \vector == X86_TRAP_BP > > + /* #BP advances %rip to the next instruction */ > > + UNWIND_HINT_IRET_REGS offset=\has_error_code*8 signal=0 > > So the fact that INT3 is trap like is not the problem, the problem is > that we use INT3 to overwrite stack modifying instruction and we should > not assume those instructions have completed when in the #BP handler. > > Now, the reason all this actually works is because INT3 itself does not > modify the stack so rewinding on non-overwrite INT3 instructions is > invariant wrt stack state.
Right, that's what my patch description attempting to say.
That's also why I was asking about INT1, which is a trap. Do we care about INT1?
-- Josh
| |