Re: [RFC PATCH 11/14] khwasan: add brk handler for inline instrumentation

[Andrey Konovalov <andreyknvl@google.com>]

On Mon, Mar 26, 2018 at 11:36 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Fri, Mar 23, 2018 at 04:59:36PM +0100, Andrey Konovalov wrote:
>> I saw BUG handler using this (which also inserts a brk), so I used it
>> as well.
>
> Ah; I think that's broken today.
>
>> What should I do instead to jump over the faulting brk instruction?
>
> I don't think we have anything to do this properly today.
>
> The simplest fix would be to split arm64_skip_faulting_instruction()
> into separate functions for user/kernel, something like the below.

OK, will do that!

>
> It would be nice to drop _user_ in the name of the userspace-specific
> helper, though.

I'm not familiar with the code, but having "user" in a
userspace-specific function name sounds logical :) I think I'm not
going to include this change, and it probably needs to be done in a
separate patch/patchset anyway.

>
> Thanks
> Mark.
>
> ---->8----
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index eb2d15147e8d..101e3d4ed6c8 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -235,9 +235,14 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
>         }
>  }
>
> -void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
> +void __arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
>  {
>         regs->pc += size;
> +}
> +
> +void arm64_skip_faulting_instruction(struct pt_regs *regs, unsigned long size)
> +{
> +       __arm64_skip_faulting_instruction(regs, size);
>
>         /*
>          * If we were single stepping, we want to get the step exception after
> @@ -761,7 +766,7 @@ static int bug_handler(struct pt_regs *regs, unsigned int esr)
>         }
>
>         /* If thread survives, skip over the BUG instruction and continue: */
> -       arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
> +       __arm64_skip_faulting_instruction(regs, AARCH64_INSN_SIZE);
>         return DBG_HOOK_HANDLED;
>  }
>
>