[PATCH v6 1/6] arm64: kprobe: add checks for ARMv8.3-PAuth combined instructions

[Will Deacon]

On Fri, Sep 04, 2020 at 04:12:04PM +0530, Amit Daniel Kachhap wrote:
> Currently the ARMv8.3-PAuth combined branch instructions (braa, retaa
> etc.) are not simulated for out-of-line execution with a handler. Hence the
> uprobe of such instructions leads to kernel warnings in a loop as they are
> not explicitly checked and fall into INSN_GOOD categories. Other combined
> instructions like LDRAA and LDRBB can be probed.
> 
> The issue of the combined branch instructions is fixed by adding
> definitions of all such instructions and rejecting their probes.
> 
> Warning log:
>  WARNING: CPU: 5 PID: 249 at arch/arm64/kernel/probes/uprobes.c:182 uprobe_single_step_handler+0x34/0x50
>  Modules linked in:
>  CPU: 5 PID: 249 Comm: func Tainted: G        W         5.8.0-rc4-00005-ge658591d66d1-dirty #160
>  Hardware name: Foundation-v8A (DT)
>  pstate: 204003c9 (nzCv DAIF +PAN -UAO BTYPE=--)
>  pc : uprobe_single_step_handler+0x34/0x50
>  lr : single_step_handler+0x70/0xf8
>  sp : ffff800012afbe30
>  x29: ffff800012afbe30 x28: ffff000879f00ec0
>  x27: 0000000000000000 x26: 0000000000000000
>  x25: 0000000000000000 x24: 0000000000000000
>  x23: 0000000060001000 x22: 00000000cb000022
>  x21: ffff800011fc5a68 x20: ffff800012afbec0
>  x19: ffff800011fc86c0 x18: 0000000000000000
>  x17: 0000000000000000 x16: 0000000000000000
>  x15: 0000000000000000 x14: 0000000000000000
>  x13: 0000000000000000 x12: 0000000000000000
>  x11: 0000000000000000 x10: 0000000000000000
>  x9 : ffff800010085d50 x8 : 0000000000000000
>  x7 : 0000000000000000 x6 : ffff800011fba9c0
>  x5 : ffff800011fba000 x4 : ffff800012283070
>  x3 : ffff8000100a78e0 x2 : 00000000004005f0
>  x1 : 0000fffffffff008 x0 : ffff800012afbec0
>  Call trace:
>   uprobe_single_step_handler+0x34/0x50
>   single_step_handler+0x70/0xf8
>   do_debug_exception+0xb8/0x130
>   el0_sync_handler+0x7c/0x188
>   el0_sync+0x158/0x180
> 
> Fixes: 74afda4016a7 ("arm64: compile the kernel with ptrauth return address signing")
> Fixes: 04ca3204fa09 ("arm64: enable pointer authentication")
> Signed-off-by: Amit Daniel Kachhap <amit.kachhap at arm.com>
> Reviewed-by: Dave Martin <Dave.Martin at arm.com>
> ---
> Changes since v5: 
> * Slight change in commit log.
> * Added Reviewed-by.
> 
>  arch/arm64/include/asm/insn.h          | 12 ++++++++++++
>  arch/arm64/kernel/insn.c               | 14 ++++++++++++--
>  arch/arm64/kernel/probes/decode-insn.c |  4 +++-
>  3 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
> index 0bc46149e491..324234068fee 100644
> --- a/arch/arm64/include/asm/insn.h
> +++ b/arch/arm64/include/asm/insn.h
> @@ -359,9 +359,21 @@ __AARCH64_INSN_FUNCS(brk,	0xFFE0001F, 0xD4200000)
>  __AARCH64_INSN_FUNCS(exception,	0xFF000000, 0xD4000000)
>  __AARCH64_INSN_FUNCS(hint,	0xFFFFF01F, 0xD503201F)
>  __AARCH64_INSN_FUNCS(br,	0xFFFFFC1F, 0xD61F0000)
> +__AARCH64_INSN_FUNCS(braaz,	0xFFFFFC1F, 0xD61F081F)
> +__AARCH64_INSN_FUNCS(brabz,	0xFFFFFC1F, 0xD61F0C1F)
> +__AARCH64_INSN_FUNCS(braa,	0xFFFFFC00, 0xD71F0800)
> +__AARCH64_INSN_FUNCS(brab,	0xFFFFFC00, 0xD71F0C00)

When do we need to distinguish these variants? Can we modify the mask/value
pair so that we catch bra* in one go? That would match how they are
documented in the Arm ARM.

>  __AARCH64_INSN_FUNCS(blr,	0xFFFFFC1F, 0xD63F0000)
> +__AARCH64_INSN_FUNCS(blraaz,	0xFFFFFC1F, 0xD63F081F)
> +__AARCH64_INSN_FUNCS(blrabz,	0xFFFFFC1F, 0xD63F0C1F)
> +__AARCH64_INSN_FUNCS(blraa,	0xFFFFFC00, 0xD73F0800)
> +__AARCH64_INSN_FUNCS(blrab,	0xFFFFFC00, 0xD73F0C00)

Same here for blra*

>  __AARCH64_INSN_FUNCS(ret,	0xFFFFFC1F, 0xD65F0000)
> +__AARCH64_INSN_FUNCS(retaa,	0xFFFFFFFF, 0xD65F0BFF)
> +__AARCH64_INSN_FUNCS(retab,	0xFFFFFFFF, 0xD65F0FFF)
>  __AARCH64_INSN_FUNCS(eret,	0xFFFFFFFF, 0xD69F03E0)
> +__AARCH64_INSN_FUNCS(eretaa,	0xFFFFFFFF, 0xD69F0BFF)
> +__AARCH64_INSN_FUNCS(eretab,	0xFFFFFFFF, 0xD69F0FFF)

... and here for ereta*.

Will