From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4CD7CC43461 for ; Tue, 8 Sep 2020 10:52:56 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C61CE2087C for ; Tue, 8 Sep 2020 10:52:55 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="c4zNpaim" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C61CE2087C Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=I/Dx8LVzKCqMJuvbcr04vzYrkaq9QTrBhNyHsc3vaVc=; b=c4zNpaimb/ru+0odTZinbdvQF gGY/E3CbOwnaRKhazB3E8TuHNIiDhXhibPCTFcy1PMnCmOwu07K+yIZrQmBF58moh2WweE6yfF8kr kQ+hrY3O9dUlnaRTGZZFQcFT5GyvQyiTPoOfwkUMVgzKsFC1g0z/eUQ+tysviRUthnVQ8/nV+Fbh3 qGlh+v67UNWa4F5qsI/6nlBtkBI4SyDQvzxwAcQ990O0DtEnWmLoy4maNh44uIA76axrA+Cch5c9y anbaPnIV241Pn+TbyvVAhkmRtpJKAWy6Y6Ei55KTMNW3ce+FuKv3sZNbVfyMTz/DktoKIg5iPMJ47 bN6nKc3Kg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFbDd-0007MG-69; Tue, 08 Sep 2020 10:51:21 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kFbDY-0007Ln-Gj for linux-arm-kernel@lists.infradead.org; Tue, 08 Sep 2020 10:51:18 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 2C95331B; Tue, 8 Sep 2020 03:51:12 -0700 (PDT) Received: from arm.com (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id EA8E23F66E; Tue, 8 Sep 2020 03:51:10 -0700 (PDT) Date: Tue, 8 Sep 2020 11:51:08 +0100 From: Dave Martin To: Will Deacon Subject: Re: [PATCH v6 1/6] arm64: kprobe: add checks for ARMv8.3-PAuth combined instructions Message-ID: <20200908105107.GQ6642@arm.com> References: <20200904104209.32385-1-amit.kachhap@arm.com> <20200904104209.32385-2-amit.kachhap@arm.com> <20200907214551.GD13815@willie-the-truck> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200907214551.GD13815@willie-the-truck> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200908_065116_692022_5F0A1A63 X-CRM114-Status: GOOD ( 25.52 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Suzuki K Poulose , Catalin Marinas , Mark Brown , James Morse , Amit Daniel Kachhap , Vincenzo Frascino , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Sep 07, 2020 at 10:45:51PM +0100, Will Deacon wrote: > 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 > > Reviewed-by: Dave Martin > > --- > > 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*. >From my side: I thought about this myself, but I thought that this may be easier to maintain if we avoid lumping instructions together. Some of these cases are probably trivial enough that they can be merged at low risk, though, and we also have some other merged instruction classes here already. Avoiding pointless distinctions here will also help the efficiency of code that uses these definitions in some cases, though I don't have a feel for how significant the difference will be -- probably not very. If we become concerned about performance, we would probably want a bigger overhaul of the affected code anyway. I guess I'm happy either way. Cheers ---Dave _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel