From 8e791f7eba4c7711f56616ae163ee3cbc00b1bf4 Mon Sep 17 00:00:00 2001 From: "Masami Hiramatsu (Google)" Date: Wed, 30 Nov 2022 22:33:09 +0900 Subject: x86/kprobes: Drop removed INT3 handling code Drop removed INT3 handling code from kprobe_int3_handler() because this case (get_kprobe() doesn't return corresponding kprobe AND the INT3 is removed) must not happen with the kprobe managed INT3, but can happen with the non-kprobe INT3, which should be handled by other callbacks. For the kprobe managed INT3, it is already safe. The commit 5c02ece81848d ("x86/kprobes: Fix ordering while text-patching") introduced text_poke_sync() to the arch_disarm_kprobe() right after removing INT3. Since this text_poke_sync() uses IPI to call sync_core() on all online cpus, that ensures that all running INT3 exception handlers have done. And, the unregister_kprobe() will remove the kprobe from the hash table after arch_disarm_kprobe(). Thus, when the kprobe managed INT3 hits, kprobe_int3_handler() should be able to find corresponding kprobe always by get_kprobe(). If it can not find any kprobe, this means that is NOT a kprobe managed INT3. Signed-off-by: Masami Hiramatsu (Google) Signed-off-by: Ingo Molnar Reviewed-by: Steven Rostedt (Google) Link: https://lore.kernel.org/r/166981518895.1131462.4693062055762912734.stgit@devnote3 --- arch/x86/kernel/kprobes/core.c | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'arch/x86/kernel/kprobes/core.c') diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index 66299682b6b7..33390ed4dcf3 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -986,20 +986,6 @@ int kprobe_int3_handler(struct pt_regs *regs) kprobe_post_process(p, regs, kcb); return 1; } - } - - if (*addr != INT3_INSN_OPCODE) { - /* - * The breakpoint instruction was removed right - * after we hit it. Another cpu has removed - * either a probepoint or a debugger breakpoint - * at this address. In either case, no further - * handling of this interrupt is appropriate. - * Back up over the (now missing) int3 and run - * the original instruction. - */ - regs->ip = (unsigned long)addr; - return 1; } /* else: not a kprobe fault; let the kernel handle it */ return 0; -- cgit v1.2.3 From 9fcad995c6c52cc9791f7ee9f1386a5684055f9c Mon Sep 17 00:00:00 2001 From: Chuang Wang Date: Tue, 29 Nov 2022 16:39:51 +0800 Subject: x86/kprobes: Use switch-case for 0xFF opcodes in prepare_emulation For the `FF /digit` opcodes in prepare_emulation, use switch-case instead of hand-written code to make the logic easier to understand. Signed-off-by: Chuang Wang Signed-off-by: Ingo Molnar Acked-by: Masami Hiramatsu (Google) Link: https://lore.kernel.org/r/20221129084022.718355-1-nashuiliang@gmail.com --- arch/x86/kernel/kprobes/core.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) (limited to 'arch/x86/kernel/kprobes/core.c') diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index 33390ed4dcf3..fd0420a3e72e 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -655,17 +655,19 @@ static int prepare_emulation(struct kprobe *p, struct insn *insn) * is determined by the MOD/RM byte. */ opcode = insn->modrm.bytes[0]; - if ((opcode & 0x30) == 0x10) { - if ((opcode & 0x8) == 0x8) - return -EOPNOTSUPP; /* far call */ - /* call absolute, indirect */ + switch (X86_MODRM_REG(opcode)) { + case 0b010: /* FF /2, call near, absolute indirect */ p->ainsn.emulate_op = kprobe_emulate_call_indirect; - } else if ((opcode & 0x30) == 0x20) { - if ((opcode & 0x8) == 0x8) - return -EOPNOTSUPP; /* far jmp */ - /* jmp near absolute indirect */ + break; + case 0b100: /* FF /4, jmp near, absolute indirect */ p->ainsn.emulate_op = kprobe_emulate_jmp_indirect; - } else + break; + case 0b011: /* FF /3, call far, absolute indirect */ + case 0b101: /* FF /5, jmp far, absolute indirect */ + return -EOPNOTSUPP; + } + + if (!p->ainsn.emulate_op) break; if (insn->addr_bytes != sizeof(unsigned long)) -- cgit v1.2.3 From db7adcfd1cec4e95155e37bc066fddab302c6340 Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Mon, 23 Jan 2023 21:59:16 +0100 Subject: x86/alternatives: Introduce int3_emulate_jcc() Move the kprobe Jcc emulation into int3_emulate_jcc() so it can be used by more code -- specifically static_call() will need this. Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Ingo Molnar Reviewed-by: Masami Hiramatsu (Google) Link: https://lore.kernel.org/r/20230123210607.057678245@infradead.org --- arch/x86/include/asm/text-patching.h | 31 +++++++++++++++++++++++++++++ arch/x86/kernel/kprobes/core.c | 38 ++++++++---------------------------- 2 files changed, 39 insertions(+), 30 deletions(-) (limited to 'arch/x86/kernel/kprobes/core.c') diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h index f4b87f08f5c5..29832c338cdc 100644 --- a/arch/x86/include/asm/text-patching.h +++ b/arch/x86/include/asm/text-patching.h @@ -184,6 +184,37 @@ void int3_emulate_ret(struct pt_regs *regs) unsigned long ip = int3_emulate_pop(regs); int3_emulate_jmp(regs, ip); } + +static __always_inline +void int3_emulate_jcc(struct pt_regs *regs, u8 cc, unsigned long ip, unsigned long disp) +{ + static const unsigned long jcc_mask[6] = { + [0] = X86_EFLAGS_OF, + [1] = X86_EFLAGS_CF, + [2] = X86_EFLAGS_ZF, + [3] = X86_EFLAGS_CF | X86_EFLAGS_ZF, + [4] = X86_EFLAGS_SF, + [5] = X86_EFLAGS_PF, + }; + + bool invert = cc & 1; + bool match; + + if (cc < 0xc) { + match = regs->flags & jcc_mask[cc >> 1]; + } else { + match = ((regs->flags & X86_EFLAGS_SF) >> X86_EFLAGS_SF_BIT) ^ + ((regs->flags & X86_EFLAGS_OF) >> X86_EFLAGS_OF_BIT); + if (cc >= 0xe) + match = match || (regs->flags & X86_EFLAGS_ZF); + } + + if ((match && !invert) || (!match && invert)) + ip += disp; + + int3_emulate_jmp(regs, ip); +} + #endif /* !CONFIG_UML_X86 */ #endif /* _ASM_X86_TEXT_PATCHING_H */ diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index 66299682b6b7..d48638cc71eb 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -460,50 +460,26 @@ static void kprobe_emulate_call(struct kprobe *p, struct pt_regs *regs) } NOKPROBE_SYMBOL(kprobe_emulate_call); -static nokprobe_inline -void __kprobe_emulate_jmp(struct kprobe *p, struct pt_regs *regs, bool cond) +static void kprobe_emulate_jmp(struct kprobe *p, struct pt_regs *regs) { unsigned long ip = regs->ip - INT3_INSN_SIZE + p->ainsn.size; - if (cond) - ip += p->ainsn.rel32; + ip += p->ainsn.rel32; int3_emulate_jmp(regs, ip); } - -static void kprobe_emulate_jmp(struct kprobe *p, struct pt_regs *regs) -{ - __kprobe_emulate_jmp(p, regs, true); -} NOKPROBE_SYMBOL(kprobe_emulate_jmp); -static const unsigned long jcc_mask[6] = { - [0] = X86_EFLAGS_OF, - [1] = X86_EFLAGS_CF, - [2] = X86_EFLAGS_ZF, - [3] = X86_EFLAGS_CF | X86_EFLAGS_ZF, - [4] = X86_EFLAGS_SF, - [5] = X86_EFLAGS_PF, -}; - static void kprobe_emulate_jcc(struct kprobe *p, struct pt_regs *regs) { - bool invert = p->ainsn.jcc.type & 1; - bool match; + unsigned long ip = regs->ip - INT3_INSN_SIZE + p->ainsn.size; - if (p->ainsn.jcc.type < 0xc) { - match = regs->flags & jcc_mask[p->ainsn.jcc.type >> 1]; - } else { - match = ((regs->flags & X86_EFLAGS_SF) >> X86_EFLAGS_SF_BIT) ^ - ((regs->flags & X86_EFLAGS_OF) >> X86_EFLAGS_OF_BIT); - if (p->ainsn.jcc.type >= 0xe) - match = match || (regs->flags & X86_EFLAGS_ZF); - } - __kprobe_emulate_jmp(p, regs, (match && !invert) || (!match && invert)); + int3_emulate_jcc(regs, p->ainsn.jcc.type, ip, p->ainsn.rel32); } NOKPROBE_SYMBOL(kprobe_emulate_jcc); static void kprobe_emulate_loop(struct kprobe *p, struct pt_regs *regs) { + unsigned long ip = regs->ip - INT3_INSN_SIZE + p->ainsn.size; bool match; if (p->ainsn.loop.type != 3) { /* LOOP* */ @@ -531,7 +507,9 @@ static void kprobe_emulate_loop(struct kprobe *p, struct pt_regs *regs) else if (p->ainsn.loop.type == 1) /* LOOPE */ match = match && (regs->flags & X86_EFLAGS_ZF); - __kprobe_emulate_jmp(p, regs, match); + if (match) + ip += p->ainsn.rel32; + int3_emulate_jmp(regs, ip); } NOKPROBE_SYMBOL(kprobe_emulate_loop); -- cgit v1.2.3 From ae052e3ae09572194d7e574906db7272041577d3 Mon Sep 17 00:00:00 2001 From: Nadav Amit Date: Wed, 8 Feb 2023 07:17:08 +0000 Subject: x86/kprobes: Fix 1 byte conditional jump target Commit 3bc753c06dd0 ("kbuild: treat char as always unsigned") broke kprobes. Setting a probe-point on 1 byte conditional jump can cause the kernel to crash when the (signed) relative jump offset gets treated as unsigned. Fix by replacing the unsigned 'immediate.bytes' (plus a cast) with the signed 'immediate.value' when assigning to the relative jump offset. [ dhansen: clarified changelog ] Fixes: 3bc753c06dd0 ("kbuild: treat char as always unsigned") Suggested-by: Masami Hiramatsu (Google) Suggested-by: Dave Hansen Signed-off-by: Nadav Amit Signed-off-by: Dave Hansen Acked-by: Peter Zijlstra (Intel) Link: https://lore.kernel.org/all/20230208071708.4048-1-namit%40vmware.com --- arch/x86/kernel/kprobes/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch/x86/kernel/kprobes/core.c') diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c index b36f3c367cb2..695873c0f50b 100644 --- a/arch/x86/kernel/kprobes/core.c +++ b/arch/x86/kernel/kprobes/core.c @@ -625,7 +625,7 @@ static int prepare_emulation(struct kprobe *p, struct insn *insn) /* 1 byte conditional jump */ p->ainsn.emulate_op = kprobe_emulate_jcc; p->ainsn.jcc.type = opcode & 0xf; - p->ainsn.rel32 = *(char *)insn->immediate.bytes; + p->ainsn.rel32 = insn->immediate.value; break; case 0x0f: opcode = insn->opcode.bytes[1]; -- cgit v1.2.3