On Tue, May 5, 2020 at 7:16 AM Thomas Gleixner <t...@linutronix.de> wrote:
>
> For code simplicity split up the int3 handler into a kernel and user part
> which makes the code flow simpler to understand.
>
> Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> ---
> arch/x86/kernel/traps.c | 67
> +++++++++++++++++++++++++++---------------------
> 1 file changed, 39 insertions(+), 28 deletions(-)
>
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -564,6 +564,35 @@ DEFINE_IDTENTRY_ERRORCODE(exc_general_pr
> cond_local_irq_disable(regs);
> }
>
> +static bool do_int3(struct pt_regs *regs)
> +{
> + int res;
> +
> +#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
> + if (kgdb_ll_trap(DIE_INT3, "int3", regs, 0, X86_TRAP_BP,
> + SIGTRAP) == NOTIFY_STOP)
> + return true;
> +#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
> +
> +#ifdef CONFIG_KPROBES
> + if (kprobe_int3_handler(regs))
> + return true;
> +#endif
> + res = notify_die(DIE_INT3, "int3", regs, 0, X86_TRAP_BP, SIGTRAP);
> +
> + return res == NOTIFY_STOP;
> +}
> +
> +static void do_int3_user(struct pt_regs *regs)
> +{
> + if (do_int3(regs))
> + return;
> +
> + cond_local_irq_enable(regs);
> + do_trap(X86_TRAP_BP, SIGTRAP, "int3", regs, 0, 0, NULL);
> + cond_local_irq_disable(regs);
> +}
> +
> DEFINE_IDTENTRY_RAW(exc_int3)
> {
> /*
> @@ -581,37 +610,19 @@ DEFINE_IDTENTRY_RAW(exc_int3)
> * because the INT3 could have been hit in any context including
> * NMI.
> */
> - if (user_mode(regs))
> + if (user_mode(regs)) {
> idtentry_enter(regs);
> - else
> - nmi_enter();
> -
> - instr_begin();
> -#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
> - if (kgdb_ll_trap(DIE_INT3, "int3", regs, 0, X86_TRAP_BP,
> - SIGTRAP) == NOTIFY_STOP)
> - goto exit;
> -#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
> -
> -#ifdef CONFIG_KPROBES
> - if (kprobe_int3_handler(regs))
> - goto exit;
> -#endif
> -
> - if (notify_die(DIE_INT3, "int3", regs, 0, X86_TRAP_BP,
> - SIGTRAP) == NOTIFY_STOP)
> - goto exit;
> -
> - cond_local_irq_enable(regs);
> - do_trap(X86_TRAP_BP, SIGTRAP, "int3", regs, 0, 0, NULL);
> - cond_local_irq_disable(regs);
> -
> -exit:
> - instr_end();
> - if (user_mode(regs))
> + instr_begin();
> + do_int3_user(regs);
> + instr_end();
> idtentry_exit(regs);
> - else
> + } else {
> + nmi_enter();
> + instr_begin();
> + do_int3(regs);
I think you should be checking the return value here. Presumably this
should die() if it's not handled, since otherwise it will just
infinite loop.
> + instr_end();
> nmi_exit();
> + }
> }
>
> #ifdef CONFIG_X86_64
>
