[PATCH 5.10] kprobes/x86: Fix kprobe debug exception handling logic

From: Li Huafei
Date: Thu Jun 29 2023 - 22:09:48 EST


We get the following crash caused by a null pointer access:

BUG: kernel NULL pointer dereference, address: 0000000000000000
...
RIP: 0010:resume_execution+0x35/0x190
...
Call Trace:
<#DB>
kprobe_debug_handler+0x41/0xd0
exc_debug+0xe5/0x1b0
asm_exc_debug+0x19/0x30
RIP: 0010:copy_from_kernel_nofault.part.0+0x55/0xc0
...
</#DB>
process_fetch_insn+0xfb/0x720
kprobe_trace_func+0x199/0x2c0
? kernel_clone+0x5/0x2f0
kprobe_dispatcher+0x3d/0x60
aggr_pre_handler+0x40/0x80
? kernel_clone+0x1/0x2f0
kprobe_ftrace_handler+0x82/0xf0
? __se_sys_clone+0x65/0x90
ftrace_ops_assist_func+0x86/0x110
? rcu_nocb_try_bypass+0x1f3/0x370
0xffffffffc07e60c8
? kernel_clone+0x1/0x2f0
kernel_clone+0x5/0x2f0

The analysis reveals that kprobe and hardware breakpoints conflict in
the use of debug exceptions.

If we set a hardware breakpoint on a memory address and also have a
kprobe event to fetch the memory at this address. Then when kprobe
triggers, it goes to read the memory and triggers hardware breakpoint
monitoring. This time, since kprobe handles debug exceptions earlier
than hardware breakpoints, it will cause kprobe to incorrectly assume
that the exception is a kprobe trigger.

Notice that after the mainline commit 6256e668b7af ("x86/kprobes: Use
int3 instead of debug trap for single-step"), kprobe no longer uses
debug trap, avoiding the conflict with hardware breakpoints here. This
commit is to remove the IRET that returns to kernel, not to fix the
problem we have here. Also there are a bunch of merge conflicts when
trying to apply this commit to older kernels, so fixing it directly in
older kernels is probably a better option.

If the debug exception is triggered by kprobe, then regs->ip should be
located in the kprobe instruction slot. Add this check to
kprobe_debug_handler() to properly determine if a debug exception should
be handled by kprobe.

The stable kernels affected are 5.10, 5.4, 4.19, and 4.14. I made the
fix in 5.10, and we should probably apply this fix to other stable
kernels.

Signed-off-by: Li Huafei <lihuafei1@xxxxxxxxxx>
---
arch/x86/kernel/kprobes/core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 5de757099186..fd8d7d128807 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -900,7 +900,14 @@ int kprobe_debug_handler(struct pt_regs *regs)
struct kprobe *cur = kprobe_running();
struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();

- if (!cur)
+ if (!cur || !cur->ainsn.insn)
+ return 0;
+
+ /* regs->ip should be the address of next instruction to
+ * cur->ainsn.insn.
+ */
+ if (regs->ip < (unsigned long)cur->ainsn.insn ||
+ regs->ip - (unsigned long)cur->ainsn.insn > MAX_INSN_SIZE)
return 0;

resume_execution(cur, regs, kcb);
--
2.17.1