From ffb1b4a41016295e298409c9dbcacd55680bd6d4 Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 10 Feb 2023 14:42:01 -0800 Subject: x86/unwind/orc: Add 'signal' field to ORC metadata Add a 'signal' field which allows unwind hints to specify whether the instruction pointer should be taken literally (like for most interrupts and exceptions) rather than decremented (like for call stack return addresses) when used to find the next ORC entry. Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/d2c5ec4d83a45b513d8fd72fab59f1a8cfa46871.1676068346.git.jpoimboe@kernel.org --- arch/x86/include/asm/orc_types.h | 4 +++- arch/x86/include/asm/unwind_hints.h | 10 +++++----- arch/x86/kernel/unwind_orc.c | 5 ++--- 3 files changed, 10 insertions(+), 9 deletions(-) (limited to 'arch/x86') diff --git a/arch/x86/include/asm/orc_types.h b/arch/x86/include/asm/orc_types.h index 5a2baf28a1dc..1343a62106de 100644 --- a/arch/x86/include/asm/orc_types.h +++ b/arch/x86/include/asm/orc_types.h @@ -57,12 +57,14 @@ struct orc_entry { unsigned sp_reg:4; unsigned bp_reg:4; unsigned type:2; + unsigned signal:1; unsigned end:1; #elif defined(__BIG_ENDIAN_BITFIELD) unsigned bp_reg:4; unsigned sp_reg:4; - unsigned unused:5; + unsigned unused:4; unsigned end:1; + unsigned signal:1; unsigned type:2; #endif } __packed; diff --git a/arch/x86/include/asm/unwind_hints.h b/arch/x86/include/asm/unwind_hints.h index f66fbe6537dd..e7c71750b309 100644 --- a/arch/x86/include/asm/unwind_hints.h +++ b/arch/x86/include/asm/unwind_hints.h @@ -15,7 +15,7 @@ UNWIND_HINT type=UNWIND_HINT_TYPE_ENTRY end=1 .endm -.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0 +.macro UNWIND_HINT_REGS base=%rsp offset=0 indirect=0 extra=1 partial=0 signal=1 .if \base == %rsp .if \indirect .set sp_reg, ORC_REG_SP_INDIRECT @@ -45,11 +45,11 @@ .set type, UNWIND_HINT_TYPE_REGS .endif - UNWIND_HINT sp_reg=sp_reg sp_offset=sp_offset type=type + UNWIND_HINT sp_reg=sp_reg sp_offset=sp_offset type=type signal=\signal .endm -.macro UNWIND_HINT_IRET_REGS base=%rsp offset=0 - UNWIND_HINT_REGS base=\base offset=\offset partial=1 +.macro UNWIND_HINT_IRET_REGS base=%rsp offset=0 signal=1 + UNWIND_HINT_REGS base=\base offset=\offset partial=1 signal=\signal .endm .macro UNWIND_HINT_FUNC @@ -67,7 +67,7 @@ #else #define UNWIND_HINT_FUNC \ - UNWIND_HINT(ORC_REG_SP, 8, UNWIND_HINT_TYPE_FUNC, 0) + UNWIND_HINT(ORC_REG_SP, 8, UNWIND_HINT_TYPE_FUNC, 0, 0) #endif /* __ASSEMBLY__ */ diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c index cdf6c6060170..37307b40f8da 100644 --- a/arch/x86/kernel/unwind_orc.c +++ b/arch/x86/kernel/unwind_orc.c @@ -484,6 +484,8 @@ bool unwind_next_frame(struct unwind_state *state) goto the_end; } + state->signal = orc->signal; + /* Find the previous frame's stack: */ switch (orc->sp_reg) { case ORC_REG_SP: @@ -563,7 +565,6 @@ bool unwind_next_frame(struct unwind_state *state) state->sp = sp; state->regs = NULL; state->prev_regs = NULL; - state->signal = false; break; case UNWIND_HINT_TYPE_REGS: @@ -587,7 +588,6 @@ bool unwind_next_frame(struct unwind_state *state) state->regs = (struct pt_regs *)sp; state->prev_regs = NULL; state->full_regs = true; - state->signal = true; break; case UNWIND_HINT_TYPE_REGS_PARTIAL: @@ -604,7 +604,6 @@ bool unwind_next_frame(struct unwind_state *state) state->prev_regs = state->regs; state->regs = (void *)sp - IRET_FRAME_OFFSET; state->full_regs = false; - state->signal = true; break; default: -- cgit v1.2.3 From 37064583f63eca93c98a9cdf2360485ea05f617a Mon Sep 17 00:00:00 2001 From: Josh Poimboeuf Date: Fri, 10 Feb 2023 14:42:02 -0800 Subject: x86/entry: Fix unwinding from kprobe on PUSH/POP instruction If a kprobe (INT3) is set on a stack-modifying single-byte instruction, like a single-byte PUSH/POP or a LEAVE, ORC fails to unwind past it: Call Trace: dump_stack_lvl+0x57/0x90 handler_pre+0x33/0x40 [kprobe_example] aggr_pre_handler+0x49/0x90 kprobe_int3_handler+0xe3/0x180 do_int3+0x3a/0x80 exc_int3+0x7d/0xc0 asm_exc_int3+0x35/0x40 RIP: 0010:kernel_clone+0xe/0x3a0 Code: cc e8 16 b2 bf 00 66 0f 1f 44 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 41 57 41 56 41 55 41 54 cc <53> 48 89 fb 48 83 ec 68 4c 8b 27 65 48 8b 04 25 28 00 00 00 48 89 RSP: 0018:ffffc9000074fda0 EFLAGS: 00000206 RAX: 0000000000808100 RBX: ffff888109de9d80 RCX: 0000000000000000 RDX: 0000000000000011 RSI: ffff888109de9d80 RDI: ffffc9000074fdc8 RBP: ffff8881019543c0 R08: ffffffff81127e30 R09: 00000000e71742a5 R10: ffff888104764a18 R11: 0000000071742a5e R12: ffff888100078800 R13: ffff888100126000 R14: 0000000000000000 R15: ffff888100126005 ? __pfx_call_usermodehelper_exec_async+0x10/0x10 ? kernel_clone+0xe/0x3a0 ? user_mode_thread+0x5b/0x80 ? __pfx_call_usermodehelper_exec_async+0x10/0x10 ? call_usermodehelper_exec_work+0x77/0xb0 ? process_one_work+0x299/0x5f0 ? worker_thread+0x4f/0x3a0 ? __pfx_worker_thread+0x10/0x10 ? kthread+0xf2/0x120 ? __pfx_kthread+0x10/0x10 ? ret_from_fork+0x29/0x50 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 Signed-off-by: Josh Poimboeuf Signed-off-by: Ingo Molnar Link: https://lore.kernel.org/r/baafcd3cc1abb14cb757fe081fa696012a5265ee.1676068346.git.jpoimboe@kernel.org --- arch/x86/entry/entry_64.S | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'arch/x86') 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 + .else + UNWIND_HINT_IRET_REGS offset=\has_error_code*8 + .endif + ENDBR ASM_CLAC cld -- cgit v1.2.3 From 6ea17e848a8ba5138b30e936c4b71877bc972c13 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 8 Feb 2023 18:18:04 +0100 Subject: x86: Fix FILL_RETURN_BUFFER MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With overlapping alternative validation fixed, objtool promptly complains: vmlinux.o: warning: objtool: __switch_to_asm+0x2c: stack layout conflict in alternatives: .altinstr_replacement+0x47 .rela.altinstructions: 000000000000009c 0000000200000002 R_X86_64_PC32 0000000000000000 .text + 16dc 00000000000000a0 0000000600000002 R_X86_64_PC32 0000000000000000 .altinstr_replacement + 3a 00000000000000a8 0000000200000002 R_X86_64_PC32 0000000000000000 .text + 16dc 00000000000000ac 0000000600000002 R_X86_64_PC32 0000000000000000 .altinstr_replacement + 66 .text: 00000000000016b0 <__switch_to_asm>: 16b0: f3 0f 1e fa endbr64 16b4: 55 push %rbp 16b5: 53 push %rbx 16b6: 41 54 push %r12 16b8: 41 55 push %r13 16ba: 41 56 push %r14 16bc: 41 57 push %r15 16be: 48 89 a7 18 0b 00 00 mov %rsp,0xb18(%rdi) 16c5: 48 8b a6 18 0b 00 00 mov 0xb18(%rsi),%rsp 16cc: 48 8b 9e 28 05 00 00 mov 0x528(%rsi),%rbx 16d3: 65 48 89 1c 25 00 00 00 00 mov %rbx,%gs:0x0 16d8: R_X86_64_32S fixed_percpu_data+0x28 16dc: eb 2a jmp 1708 <__switch_to_asm+0x58> 16de: 90 nop 16df: 90 nop 16e0: 90 nop 16e1: 90 nop 16e2: 90 nop 16e3: 90 nop 16e4: 90 nop 16e5: 90 nop 16e6: 90 nop 16e7: 90 nop 16e8: 90 nop 16e9: 90 nop 16ea: 90 nop 16eb: 90 nop 16ec: 90 nop 16ed: 90 nop 16ee: 90 nop 16ef: 90 nop 16f0: 90 nop 16f1: 90 nop 16f2: 90 nop 16f3: 90 nop 16f4: 90 nop 16f5: 90 nop 16f6: 90 nop 16f7: 90 nop 16f8: 90 nop 16f9: 90 nop 16fa: 90 nop 16fb: 90 nop 16fc: 90 nop 16fd: 90 nop 16fe: 90 nop 16ff: 90 nop 1700: 90 nop 1701: 90 nop 1702: 90 nop 1703: 90 nop 1704: 90 nop 1705: 90 nop 1706: 90 nop 1707: 90 nop 1708: 41 5f pop %r15 170a: 41 5e pop %r14 170c: 41 5d pop %r13 170e: 41 5c pop %r12 1710: 5b pop %rbx 1711: 5d pop %rbp 1712: e9 00 00 00 00 jmp 1717 <__switch_to_asm+0x67> 1713: R_X86_64_PLT32 __switch_to-0x4 .altinstr_replacement: 3a: 49 c7 c4 10 00 00 00 mov $0x10,%r12 41: e8 01 00 00 00 call 47 <.altinstr_replacement+0x47> 46: cc int3 47: e8 01 00 00 00 call 4d <.altinstr_replacement+0x4d> 4c: cc int3 4d: 48 83 c4 10 add $0x10,%rsp 51: 49 ff cc dec %r12 54: 75 eb jne 41 <.altinstr_replacement+0x41> 56: 0f ae e8 lfence 59: 65 48 c7 04 25 00 00 00 00 ff ff ff ff movq $0xffffffffffffffff,%gs:0x0 5e: R_X86_64_32S pcpu_hot+0x10 66: e8 01 00 00 00 call 6c <.altinstr_replacement+0x6c> 6b: cc int3 6c: 48 83 c4 08 add $0x8,%rsp 70: 0f ae e8 lfence As can be seen from the two alternatives, when overlaid, the NOP after the shorter (starting at 66) coinsides with the call at 47, leading to conflicting CFI state for that instruction. By offsetting the shorter alternative by 2 bytes, this alignment is undone. Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Ingo Molnar Acked-by: Josh Poimboeuf Tested-by: Nathan Chancellor # build only Tested-by: Thomas Weißschuh # compile and run Link: https://lore.kernel.org/r/20230208172245.783099843@infradead.org --- arch/x86/include/asm/nospec-branch.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86') diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index e04313e89f4f..3ef70e54a858 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -261,7 +261,7 @@ .macro FILL_RETURN_BUFFER reg:req nr:req ftr:req ftr2=ALT_NOT(X86_FEATURE_ALWAYS) ALTERNATIVE_2 "jmp .Lskip_rsb_\@", \ __stringify(__FILL_RETURN_BUFFER(\reg,\nr)), \ftr, \ - __stringify(__FILL_ONE_RETURN), \ftr2 + __stringify(nop;nop;__FILL_ONE_RETURN), \ftr2 .Lskip_rsb_\@: .endm -- cgit v1.2.3