On 1/27/24 13:47, Xin Li wrote:

On 1/26/2024 8:41 PM, Jinghao Jia wrote:

quoted

Both INTs (INT n, INT1, INT3, INTO) and UDs (UD0, UD1, UD2) serve
special purposes in the kernel, e.g., INT3 is used by KGDB and UD2 is
involved in LLVM-KCFI instrumentation. At the same time, attaching
kprobes on these instructions (particularly UDs) will pollute the stack
trace dumped in the kernel ring buffer, since the exception is triggered
in the copy buffer rather than the original location.

Check for INTs and UDs in can_probe and reject any kprobes trying to
attach to these instructions.

Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Signed-off-by: Jinghao Jia <redacted>
---
  arch/x86/kernel/kprobes/core.c | 33 ++++++++++++++++++++++++++-------
  1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index e8babebad7b8..792b38d22126 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c

@@ -252,6 +252,22 @@ unsigned long recover_probed_instruction(kprobe_opcode_t *buf, unsigned long add

      return __recover_probed_insn(buf, addr);
  }
  +static inline int is_exception_insn(struct insn *insn)

s/int/bool

Oh yes, the return type should be bool. Thanks for pointing out!

--Jinghao

quoted

+{
+    if (insn->opcode.bytes[0] == 0x0f) {
+        /* UD0 / UD1 / UD2 */
+        return insn->opcode.bytes[1] == 0xff ||
+               insn->opcode.bytes[1] == 0xb9 ||
+               insn->opcode.bytes[1] == 0x0b;
+    } else {
+        /* INT3 / INT n / INTO / INT1 */
+        return insn->opcode.bytes[0] == 0xcc ||
+               insn->opcode.bytes[0] == 0xcd ||
+               insn->opcode.bytes[0] == 0xce ||
+               insn->opcode.bytes[0] == 0xf1;
+    }
+}
+
  /* Check if paddr is at an instruction boundary */
  static int can_probe(unsigned long paddr)
  {

@@ -294,6 +310,16 @@ static int can_probe(unsigned long paddr)

  #endif
          addr += insn.length;
      }
+    __addr = recover_probed_instruction(buf, addr);
+    if (!__addr)
+        return 0;
+
+    if (insn_decode_kernel(&insn, (void *)__addr) < 0)
+        return 0;
+
+    if (is_exception_insn(&insn))
+        return 0;
+
      if (IS_ENABLED(CONFIG_CFI_CLANG)) {
          /*
           * The compiler generates the following instruction sequence

@@ -308,13 +334,6 @@ static int can_probe(unsigned long paddr)

           * 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)