From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Prohibit probing on the compiler generated CFI typeid checking code
because it is used for decoding typeid when CFI error happens.
The compiler generates the following instruction sequence for indirect
call checks on x86;
movl -<id>, %r10d ; 6 bytes
addl -4(%reg), %r10d ; 4 bytes
je .Ltmp1 ; 2 bytes
ud2 ; <- regs->ip
And handle_cfi_failure() decodes these instructions (movl and addl)
for the typeid and the target address. Thus if we put a kprobe on
those instructions, the decode will fail and report a wrong typeid
and target address.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/kprobes/core.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index f7f6042eb7e6..fa8c2b41cbaf 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -54,6 +54,7 @@
#include <asm/insn.h>
#include <asm/debugreg.h>
#include <asm/ibt.h>
+#include <asm/cfi.h>
#include "common.h"
@@ -293,7 +294,40 @@ static int can_probe(unsigned long paddr)
#endif
addr += insn.length;
}
+ if (IS_ENABLED(CONFIG_CFI_CLANG)) {
+ /*
+ * The compiler generates the following instruction sequence
+ * for indirect call checks and cfi.c decodes this;
+ *
+ * movl -<id>, %r10d ; 6 bytes
+ * addl -4(%reg), %r10d ; 4 bytes
+ * je .Ltmp1 ; 2 bytes
+ * ud2 ; <- regs->ip
+ * .Ltmp1:
+ *
+ * Also, these movl and addl are used for showing expected
+ * type. So those must not be touched.
+ */
+ __addr = recover_probed_instruction(buf, addr);
+ if (!__addr)
+ return 0;
+
+ if (insn_decode_kernel(&insn, (void *)__addr) < 0)
+ return 0;
+
+ if (insn.opcode.value == 0xBA)
+ offset = 12;
+ else if (insn.opcode.value == 0x3)
+ offset = 6;
+ else
+ goto out;
+
+ /* This movl/addl is used for decoding CFI. */
+ if (is_cfi_trap(addr + offset))
+ return 0;
+ }
+out:
return (addr == paddr);
}
On Tue, 11 Jul 2023 10:50:58 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Prohibit probing on the compiler generated CFI typeid checking code
> because it is used for decoding typeid when CFI error happens.
>
> The compiler generates the following instruction sequence for indirect
> call checks on x86;
>
> movl -<id>, %r10d ; 6 bytes
> addl -4(%reg), %r10d ; 4 bytes
> je .Ltmp1 ; 2 bytes
> ud2 ; <- regs->ip
>
> And handle_cfi_failure() decodes these instructions (movl and addl)
> for the typeid and the target address. Thus if we put a kprobe on
> those instructions, the decode will fail and report a wrong typeid
> and target address.
>
>
Hi Peter,
Can I pick this to probes/fixes branch ?
Thank you,
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/kernel/kprobes/core.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index f7f6042eb7e6..fa8c2b41cbaf 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -54,6 +54,7 @@
> #include <asm/insn.h>
> #include <asm/debugreg.h>
> #include <asm/ibt.h>
> +#include <asm/cfi.h>
>
> #include "common.h"
>
> @@ -293,7 +294,40 @@ static int can_probe(unsigned long paddr)
> #endif
> addr += insn.length;
> }
> + if (IS_ENABLED(CONFIG_CFI_CLANG)) {
> + /*
> + * The compiler generates the following instruction sequence
> + * for indirect call checks and cfi.c decodes this;
> + *
> + * movl -<id>, %r10d ; 6 bytes
> + * addl -4(%reg), %r10d ; 4 bytes
> + * je .Ltmp1 ; 2 bytes
> + * ud2 ; <- regs->ip
> + * .Ltmp1:
> + *
> + * Also, these movl and addl are used for showing expected
> + * type. So those must not be touched.
> + */
> + __addr = recover_probed_instruction(buf, addr);
> + if (!__addr)
> + return 0;
> +
> + if (insn_decode_kernel(&insn, (void *)__addr) < 0)
> + return 0;
> +
> + if (insn.opcode.value == 0xBA)
> + offset = 12;
> + else if (insn.opcode.value == 0x3)
> + offset = 6;
> + else
> + goto out;
> +
> + /* This movl/addl is used for decoding CFI. */
> + if (is_cfi_trap(addr + offset))
> + return 0;
> + }
>
> +out:
> return (addr == paddr);
> }
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
On Wed, Jul 26, 2023 at 12:23:17PM +0900, Masami Hiramatsu wrote: > On Tue, 11 Jul 2023 10:50:58 +0900 > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > Prohibit probing on the compiler generated CFI typeid checking code > > because it is used for decoding typeid when CFI error happens. > > > > The compiler generates the following instruction sequence for indirect > > call checks on x86; > > > > movl -<id>, %r10d ; 6 bytes > > addl -4(%reg), %r10d ; 4 bytes > > je .Ltmp1 ; 2 bytes > > ud2 ; <- regs->ip > > > > And handle_cfi_failure() decodes these instructions (movl and addl) > > for the typeid and the target address. Thus if we put a kprobe on > > those instructions, the decode will fail and report a wrong typeid > > and target address. > > > > > > Hi Peter, > > Can I pick this to probes/fixes branch ? I'll stick them in tip/x86/core, that ok?
On Wed, 26 Jul 2023 11:29:17 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Wed, Jul 26, 2023 at 12:23:17PM +0900, Masami Hiramatsu wrote: > > On Tue, 11 Jul 2023 10:50:58 +0900 > > "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote: > > > > > From: Masami Hiramatsu (Google) <mhiramat@kernel.org> > > > > > > Prohibit probing on the compiler generated CFI typeid checking code > > > because it is used for decoding typeid when CFI error happens. > > > > > > The compiler generates the following instruction sequence for indirect > > > call checks on x86; > > > > > > movl -<id>, %r10d ; 6 bytes > > > addl -4(%reg), %r10d ; 4 bytes > > > je .Ltmp1 ; 2 bytes > > > ud2 ; <- regs->ip > > > > > > And handle_cfi_failure() decodes these instructions (movl and addl) > > > for the typeid and the target address. Thus if we put a kprobe on > > > those instructions, the decode will fail and report a wrong typeid > > > and target address. > > > > > > > > > > Hi Peter, > > > > Can I pick this to probes/fixes branch ? > > I'll stick them in tip/x86/core, that ok? Yes, since it is for CFI change. Thank you, -- Masami Hiramatsu (Google) <mhiramat@kernel.org>
The following commit has been merged into the x86/core branch of tip:
Commit-ID: b65413768abd27a55af74945aec58127a52b30a8
Gitweb: https://git.kernel.org/tip/b65413768abd27a55af74945aec58127a52b30a8
Author: Masami Hiramatsu <mhiramat@kernel.org>
AuthorDate: Tue, 11 Jul 2023 10:50:58 +09:00
Committer: Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Aug 2023 16:27:07 +02:00
x86/kprobes: Prohibit probing on compiler generated CFI checking code
Prohibit probing on the compiler generated CFI typeid checking code
because it is used for decoding typeid when CFI error happens.
The compiler generates the following instruction sequence for indirect
call checks on x86;
movl -<id>, %r10d ; 6 bytes
addl -4(%reg), %r10d ; 4 bytes
je .Ltmp1 ; 2 bytes
ud2 ; <- regs->ip
And handle_cfi_failure() decodes these instructions (movl and addl)
for the typeid and the target address. Thus if we put a kprobe on
those instructions, the decode will fail and report a wrong typeid
and target address.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/r/168904025785.116016.12766408611437534723.stgit@devnote2
---
arch/x86/kernel/kprobes/core.c | 34 +++++++++++++++++++++++++++++++++-
include/linux/cfi.h | 4 +++-
2 files changed, 37 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index f7f6042..e8babeb 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -45,6 +45,7 @@
#include <linux/vmalloc.h>
#include <linux/pgtable.h>
#include <linux/set_memory.h>
+#include <linux/cfi.h>
#include <asm/text-patching.h>
#include <asm/cacheflush.h>
@@ -293,7 +294,40 @@ static int can_probe(unsigned long paddr)
#endif
addr += insn.length;
}
+ if (IS_ENABLED(CONFIG_CFI_CLANG)) {
+ /*
+ * The compiler generates the following instruction sequence
+ * for indirect call checks and cfi.c decodes this;
+ *
+ * movl -<id>, %r10d ; 6 bytes
+ * addl -4(%reg), %r10d ; 4 bytes
+ * je .Ltmp1 ; 2 bytes
+ * ud2 ; <- regs->ip
+ * .Ltmp1:
+ *
+ * Also, these movl and addl are used for showing expected
+ * type. So those must not be touched.
+ */
+ __addr = recover_probed_instruction(buf, addr);
+ if (!__addr)
+ return 0;
+
+ if (insn_decode_kernel(&insn, (void *)__addr) < 0)
+ return 0;
+
+ if (insn.opcode.value == 0xBA)
+ offset = 12;
+ else if (insn.opcode.value == 0x3)
+ offset = 6;
+ else
+ goto out;
+
+ /* This movl/addl is used for decoding CFI. */
+ if (is_cfi_trap(addr + offset))
+ return 0;
+ }
+out:
return (addr == paddr);
}
diff --git a/include/linux/cfi.h b/include/linux/cfi.h
index 5e134f4..3552ec8 100644
--- a/include/linux/cfi.h
+++ b/include/linux/cfi.h
@@ -19,11 +19,13 @@ static inline enum bug_trap_type report_cfi_failure_noaddr(struct pt_regs *regs,
{
return report_cfi_failure(regs, addr, NULL, 0);
}
+#endif /* CONFIG_CFI_CLANG */
#ifdef CONFIG_ARCH_USES_CFI_TRAPS
bool is_cfi_trap(unsigned long addr);
+#else
+static inline bool is_cfi_trap(unsigned long addr) { return false; }
#endif
-#endif /* CONFIG_CFI_CLANG */
#ifdef CONFIG_MODULES
#ifdef CONFIG_ARCH_USES_CFI_TRAPS© 2016 - 2026 Red Hat, Inc.