On Fri, Mar 01, 2019 at 01:28:01PM +0000, Will Deacon wrote:
> Debug exception handlers may be called for exceptions generated both by
> user and kernel code. In many cases, this is checked explicitly, but
> in other cases things either happen to work by happy accident or they
> go slightly wrong. For example, executing 'brk #4' from userspace will
> enter the kprobes code and be ignored, but the instruction will be
> retried forever in userspace instead of delivering a SIGTRAP.
>
> Fix this issue in the most stable-friendly fashion by simply adding
> explicit checks of the triggering exception level to all of our debug
> exception handlers.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Will Deacon <will.deacon@xxxxxxx>
It might be worth noting in the commit message that this also makes the
functions consistentluy use the DBG_HOOK_* mnemonics, but either way:
Reviewed-by: Mark Rutland <mark.rutland@xxxxxxx>
Mark.
> ---
> arch/arm64/kernel/kgdb.c | 14 ++++++++++----
> arch/arm64/kernel/probes/kprobes.c | 6 ++++++
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index ce46c4cdf368..691854b77c7f 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -244,27 +244,33 @@ int kgdb_arch_handle_exception(int exception_vector, int signo,
>
> static int kgdb_brk_fn(struct pt_regs *regs, unsigned int esr)
> {
> + if (user_mode(regs))
> + return DBG_HOOK_ERROR;
> +
> kgdb_handle_exception(1, SIGTRAP, 0, regs);
> - return 0;
> + return DBG_HOOK_HANDLED;
> }
> NOKPROBE_SYMBOL(kgdb_brk_fn)
>
> static int kgdb_compiled_brk_fn(struct pt_regs *regs, unsigned int esr)
> {
> + if (user_mode(regs))
> + return DBG_HOOK_ERROR;
> +
> compiled_break = 1;
> kgdb_handle_exception(1, SIGTRAP, 0, regs);
>
> - return 0;
> + return DBG_HOOK_HANDLED;
> }
> NOKPROBE_SYMBOL(kgdb_compiled_brk_fn);
>
> static int kgdb_step_brk_fn(struct pt_regs *regs, unsigned int esr)
> {
> - if (!kgdb_single_step)
> + if (user_mode(regs) || !kgdb_single_step)
> return DBG_HOOK_ERROR;
>
> kgdb_handle_exception(1, SIGTRAP, 0, regs);
> - return 0;
> + return DBG_HOOK_HANDLED;
> }
> NOKPROBE_SYMBOL(kgdb_step_brk_fn);
>
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index f17afb99890c..7fb6f3aa5ceb 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -450,6 +450,9 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
> struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
> int retval;
>
> + if (user_mode(regs))
> + return DBG_HOOK_ERROR;
> +
> /* return error if this is not our step */
> retval = kprobe_ss_hit(kcb, instruction_pointer(regs));
>
> @@ -466,6 +469,9 @@ kprobe_single_step_handler(struct pt_regs *regs, unsigned int esr)
> int __kprobes
> kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> {
> + if (user_mode(regs))
> + return DBG_HOOK_ERROR;
> +
> kprobe_handler(regs);
> return DBG_HOOK_HANDLED;
> }
> --
> 2.11.0
>
