kprobes/x86: Fix to clear TF bit in fault-on-single-stepping

Fix kprobe_fault_handler to clear TF (trap flag) bit of flags register in the case of fault fixup on single-stepping.

If we put a kprobe on the instruction which can cause a page fault (e.g. actual mov instructions in copy_user_*), that fault happens on a single-stepping buffer. In this case, kprobes resets running instance so that the CPU can retry execution on the original ip address. However, current code forgets reset TF bit. Since this fault happens with TF bit set for enabling single-stepping, when it retries, it causes a debug exception and kprobes can not handle it because it already reset itself.

On the most of x86-64 platform, it can be easily reproduced by using kprobe tracer. E.g.

  # cd /sys/kernel/debug/tracing
  # echo p copy_user_enhanced_fast_string+5 > kprobe_events
  # echo 1 > events/kprobes/enable

And you'll see a kernel panic on do_debug(), since the debug trap is not handled by kprobes.

To fix this problem, we just need to clear the TF bit when resetting running kprobe.

Signed-off-by Masami Hiramatsu <[email protected]>

Fix kprobe_fault_handler to clear TF (trap flag) bit of flags register in the case of fault fixup on single-stepping.

If we put a kprobe on the instruction which can cause a page fault (e.g. actual mov instructions in copy_user_*), that fault happens on a single-stepping buffer. In this case, kprobes resets running instance so that the CPU can retry execution on the original ip address. However, current code forgets reset TF bit. Since this fault happens with TF bit set for enabling single-stepping, when it retries, it causes a debug exception and kprobes can not handle it because it already reset itself.

On the most of x86-64 platform, it can be easily reproduced by using kprobe tracer. E.g.

  # cd /sys/kernel/debug/tracing
  # echo p copy_user_enhanced_fast_string+5 > kprobe_events
  # echo 1 > events/kprobes/enable

And you'll see a kernel panic on do_debug(), since the debug trap is not handled by kprobes.

To fix this problem, we just need to clear the TF bit when resetting running kprobe.

Signed-off-by Masami Hiramatsu <[email protected]>

---
 arch/x86/kernel/kprobes/core.c |    7 +++++++
 1 file changed, 7 insertions(+)

arch/x86/kernel/kprobes/core.c +7 -0

index 38cf7a7..856df81 100644
@@ -961,6 +961,13 @@ int kprobe_fault_handler(struct pt_regs *regs, int trapnr)
 		 * normal page fault.
 		 */
 		regs->ip = (unsigned long)cur->addr;
+		/*
+		 * Trap flag has been set here because this fault happened
+		 * where the single stepping will be done. So clear it with
+		 * resetting current kprobe.
+		 */
+		regs->flags &= ~X86_EFLAGS_TF;
+		/* If the TF was set before the kprobe hit, don't touch it */
 		regs->flags |= kcb->kprobe_old_flags;
 		if (kcb->kprobe_status == KPROBE_REENTER)
 			restore_previous_kprobe(kcb);

Quoted 10 lines

  # cd /sys/kernel/debug/tracing
  # echo p copy_user_enhanced_fast_string+5 > kprobe_events
  # echo 1 > events/kprobes/enable

And you'll see a kernel panic on do_debug(), since the debug trap is not handled by kprobes.

To fix this problem, we just need to clear the TF bit when resetting running kprobe.

This should definitely be marked for stable, and I bisected it all the way down to this commit: f4cb1cc18f364d "x86-64, copy_user: Remove zero byte check before copy user buffer."

I reverted that commit and sure enough, this bug goes away. I'm not saying the revert should be done. I'm just doing an FYI, and showing how changes that appear to be a nice clean up can have subtle effects. I'm not even sure how that change caused this to be a problem with kprobes.

Quoted 7 lines · On Mon, 13 Jun 2016 19:13:45 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 7 lines

  # cd /sys/kernel/debug/tracing
  # echo p copy_user_enhanced_fast_string+5 > kprobe_events
  # echo 1 > events/kprobes/enable

And you'll see a kernel panic on do_debug(), since the debug trap is not handled by kprobes.

Quoted 7 lines

Quoted 3 lines

To fix this problem, we just need to clear the TF bit when resetting running kprobe.

This should definitely be marked for stable, and I bisected it all the way down to this commit: f4cb1cc18f364d "x86-64, copy_user: Remove zero byte check before copy user buffer."

Quoted 5 lines

I reverted that commit and sure enough, this bug goes away. I'm not saying the revert should be done. I'm just doing an FYI, and showing how changes that appear to be a nice clean up can have subtle effects. I'm not even sure how that change caused this to be a problem with kprobes.

Nevermind, reverting that commit only moved the location of the "rep movsb" that you were placing the kprobe on. When I do:

echo "p copy_user_enhanced_fast_string+9" > kprobe_events

I get the same result.

That means we need to make that stable tag even earlier.

Quoted 11 lines · On Mon, 13 Jun 2016 19:13:45 -0400 Steven Rostedt <[email protected]> wrote:

Quoted 10 lines

  # cd /sys/kernel/debug/tracing
  # echo p copy_user_enhanced_fast_string+5 > kprobe_events
  # echo 1 > events/kprobes/enable

And you'll see a kernel panic on do_debug(), since the debug trap is not handled by kprobes.

To fix this problem, we just need to clear the TF bit when resetting running kprobe.

Quoted 9 lines

This should definitely be marked for stable, and I bisected it all the way down to this commit: f4cb1cc18f364d "x86-64, copy_user: Remove zero byte check before copy user buffer."

I reverted that commit and sure enough, this bug goes away. I'm not saying the revert should be done. I'm just doing an FYI, and showing how changes that appear to be a nice clean up can have subtle effects. I'm not even sure how that change caused this to be a problem with kprobes.

Nevermind, reverting that commit only moved the location of the "rep movsb" that you were placing the kprobe on. When I do:

echo "p copy_user_enhanced_fast_string+9" > kprobe_events

I get the same result.

That means we need to make that stable tag even earlier.