On Sat, Jul 01, 2023 at 04:43:46PM +0800, Li Huafei wrote:
>
>
> On 2023/6/30 13:21, Greg KH wrote:
> > On Fri, Jun 30, 2023 at 10:08:45AM +0800, Li Huafei wrote:
> >> 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.
> >
> > What is the list of commits that it would take to resolve this in these
> > kernels? We would almost always prefer to do that instead of taking
> > changes that are not upstream.
>
> I have sorted out that for 5.10 there are 9 patches that need to be
> backported:
>
> #9 8924779df820 ("x86/kprobes: Fix JNG/JNLE emulation")
> #8 dec8784c9088 ("x86/kprobes: Update kcb status flag after singlestepping")
> #7 2304d14db659 ("x86/kprobes: Move 'inline' to the beginning of the kprobe_is_ss() declaration")
> #6 2f706e0e5e26 ("x86/kprobes: Fix to identify indirect jmp and others using range case")
> #5 6256e668b7af ("x86/kprobes: Use int3 instead of debug trap for single-step")
> #4 a194acd316f9 ("x86/kprobes: Identify far indirect JMP correctly")
> #3 d60ad3d46f1d ("x86/kprobes: Retrieve correct opcode for group instruction")
> #2 abd82e533d88 ("x86/kprobes: Do not decode opcode in resume_execution()")
> #1 e689b300c99c ("kprobes/x86: Fix fall-through warnings for Clang e689b300c99c")
>
> The main one we need to backport is patch 5, patche 1-6 are pre-patches,
> and patche 6-9 are fix patches for patch 5. The major modifications are
> patch 2 and patch 4. Patch 2 optimizes resume_execution() to avoid
> repeated instruction decoding, and patch 5 uses int3 instead of debug
> trap, and as Masami said in the commit message this patch will change
> some behavior of kprobe, but it has almost no effect on the actual
> usage.
>
> I'm not sure backport these patches are acceptable, do I need to send
> them out for review?
Yes, please make up the patch series for these, that's not all that bad,
and looks like it is more "correct" than just your one-off patch.
thanks,
greg k-h
