public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Will Deacon <will@kernel.org>
To: Amit Daniel Kachhap <amit.kachhap@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Brown <broonie@kernel.org>,
	James Morse <james.morse@arm.com>,
	Vincenzo Frascino <Vincenzo.Frascino@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v6 1/6] arm64: kprobe: add checks for ARMv8.3-PAuth combined instructions
Date: Mon, 7 Sep 2020 22:45:51 +0100	[thread overview]
Message-ID: <20200907214551.GD13815@willie-the-truck> (raw)
In-Reply-To: <20200904104209.32385-2-amit.kachhap@arm.com>

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@arm.com>
> Reviewed-by: Dave Martin <Dave.Martin@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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-09-07 21:47 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04 10:42 [PATCH v6 0/6] arm64: add Armv8.3 pointer authentication enhancements Amit Daniel Kachhap
2020-09-04 10:42 ` [PATCH v6 1/6] arm64: kprobe: add checks for ARMv8.3-PAuth combined instructions Amit Daniel Kachhap
2020-09-07 21:45   ` Will Deacon [this message]
2020-09-08 10:51     ` Dave Martin
2020-09-11 13:55       ` Will Deacon
2020-09-14  8:42         ` Amit Kachhap
2020-09-04 10:42 ` [PATCH v6 2/6] arm64: traps: Allow force_signal_inject to pass esr error code Amit Daniel Kachhap
2020-09-04 10:42 ` [PATCH v6 3/6] arm64: ptrauth: Introduce Armv8.3 pointer authentication enhancements Amit Daniel Kachhap
2020-09-04 10:42 ` [PATCH v6 4/6] arm64: cpufeature: Modify address authentication cpufeature to exact Amit Daniel Kachhap
2020-09-04 10:42 ` [PATCH v6 5/6] arm64: kprobe: disable probe of fault prone ptrauth instruction Amit Daniel Kachhap
2020-09-04 10:42 ` [PATCH v6 6/6] arm64: kprobe: clarify the comment of steppable hint instructions Amit Daniel Kachhap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200907214551.GD13815@willie-the-truck \
    --to=will@kernel.org \
    --cc=Dave.Martin@arm.com \
    --cc=Vincenzo.Frascino@arm.com \
    --cc=amit.kachhap@arm.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox