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=-8.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,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 A4240C4360C for ; Fri, 4 Oct 2019 13:31:55 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 77F2D2084D for ; Fri, 4 Oct 2019 13:31: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="K8ndnNeC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 77F2D2084D 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+infradead-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=bombadil.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=pNGPmhju3t9MoPkcdw0GO2/lymIUJJgrlvigxOYCSFo=; b=K8ndnNeCrYaKJC uAg3pRvUhpQVr01+SWJaFInAQ1Aeir2Lqf9XVVhq0E/aqjF5P61nrbVbaPTWXMVmfNiUKj54EW2/5 +FGgE/BloSdi43q6snw1vQBbHms72HW8bcLKStJAQn2khb1bra5kxqujJlDNpkmZS11K7jxMIB7KU 9+ZIuYD2iowc31tJ4iMSK0TWBQ2jc5uvUBKedJsDNDMT6Xj+qUOEjkeFQr3MfCrcZK47tM7Dq97Gj jbATk6lEYh7uQWP5hTmvnp8Mw634MgZ5SeIoLkvkQ16BZ29BsgfCET5arwWZw3gV7g4zENJUMW7Qg q2oDyEh5FRdjWwJbd34g==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.2 #3 (Red Hat Linux)) id 1iGNgY-0004yN-Ty; Fri, 04 Oct 2019 13:31:54 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.2 #3 (Red Hat Linux)) id 1iGNgU-0004xq-Mi for linux-arm-kernel@lists.infradead.org; Fri, 04 Oct 2019 13:31:52 +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 AA8CF15A1; Fri, 4 Oct 2019 06:31:49 -0700 (PDT) Received: from lakrids.cambridge.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 C25243F534; Fri, 4 Oct 2019 06:31:48 -0700 (PDT) Date: Fri, 4 Oct 2019 14:31:46 +0100 From: Mark Rutland To: James Morse Subject: Re: [PATCH 8/8] arm64: entry-common: don't touch daif before bp-hardening Message-ID: <20191004133146.GG34756@lakrids.cambridge.arm.com> References: <20191003171642.135652-1-james.morse@arm.com> <20191003171642.135652-9-james.morse@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20191003171642.135652-9-james.morse@arm.com> User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191004_063150_829622_47697936 X-CRM114-Status: GOOD ( 25.81 ) 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: Catalin Marinas , Will Deacon , Julien Thierry , linux-arm-kernel@lists.infradead.org, Masami Hiramatsu Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Oct 03, 2019 at 06:16:42PM +0100, James Morse wrote: > The previous patches mechanically transformed the assembly version of > entry.S to entry-common.c for synchronous exceptions. > > The C version of local_daif_restore() doesn't quite do the same thing > as the assembly versions if pseudo-NMI is in use. In particular, > | local_daif_restore(DAIF_PROCCTX_NOIRQ) > will still allow pNMI to be delivered. This is not the behaviour > do_el0_ia_bp_hardening() and do_sp_pc_abort() want as it should not > be possible for the PMU handler to run as an NMI until the bp-hardening > sequence has run. > > The bp-hardening calls were placed where they are because this was the > first C code to run after the relevant exceptions. As we've now moved > that point earlier, move the checks and calls earlier too. > > This makes it clearer that this stuff runs before any kind of exception, > and saves modifying PSTATE twice. > > Signed-off-by: James Morse > Cc: Julien Thierry > --- > arch/arm64/include/asm/processor.h | 7 +++++++ > arch/arm64/kernel/entry-common.c | 18 +++++++++++++++--- > arch/arm64/mm/fault.c | 29 +---------------------------- > 3 files changed, 23 insertions(+), 31 deletions(-) > > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h > index 5623685c7d13..c0c28c4589a8 100644 > --- a/arch/arm64/include/asm/processor.h > +++ b/arch/arm64/include/asm/processor.h > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include Nit: alphabetical order please! > > @@ -214,6 +215,12 @@ static inline void start_thread(struct pt_regs *regs, unsigned long pc, > regs->sp = sp; > } > > +static inline bool is_ttbr0_addr(unsigned long addr) > +{ > + /* entry assembly clears tags for TTBR0 addrs */ > + return addr < TASK_SIZE; > +} Could we move is_ttbr1_addr() here too? I guess there might be include ordering issues, but if not it would be nice if they lived in the same place. > + > #ifdef CONFIG_COMPAT > static inline void compat_start_thread(struct pt_regs *regs, unsigned long pc, > unsigned long sp) > diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c > index 176969e55677..eb73d250a081 100644 > --- a/arch/arm64/kernel/entry-common.c > +++ b/arch/arm64/kernel/entry-common.c > @@ -14,6 +14,7 @@ > #include > #include > #include > +#include > #include > > static void notrace el1_abort(struct pt_regs *regs, unsigned long esr) > @@ -112,9 +113,17 @@ static void notrace el0_ia(struct pt_regs *regs, unsigned long esr) > { > unsigned long far = read_sysreg(far_el1); > > + /* > + * We've taken an instruction abort from userspace and not yet > + * re-enabled IRQs. If the address is a kernel address, apply > + * BP hardening prior to enabling IRQs and pre-emption. > + */ > + if (!is_ttbr0_addr(far)) > + arm64_apply_bp_hardening(); > + > user_exit_irqoff(); > - local_daif_restore(DAIF_PROCCTX_NOIRQ); > - do_el0_ia_bp_hardening(far, esr, regs); > + local_daif_restore(DAIF_PROCCTX); > + do_mem_abort(far, esr, regs); > } > NOKPROBE_SYMBOL(el0_ia); > > @@ -154,8 +163,11 @@ static void notrace el0_pc(struct pt_regs *regs, unsigned long esr) > { > unsigned long far = read_sysreg(far_el1); > > + if (!is_ttbr0_addr(instruction_pointer(regs))) > + arm64_apply_bp_hardening(); > + > user_exit_irqoff(); > - local_daif_restore(DAIF_PROCCTX_NOIRQ); > + local_daif_restore(DAIF_PROCCTX); > do_sp_pc_abort(far, esr, regs); > } > NOKPROBE_SYMBOL(el0_pc); This is much nicer, and AFAICT is correct, so: Reviewed-by: Mark Rutland Mark. > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 0857c2fc38b9..88e4bd4bc103 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -102,12 +103,6 @@ static void mem_abort_decode(unsigned int esr) > data_abort_decode(esr); > } > > -static inline bool is_ttbr0_addr(unsigned long addr) > -{ > - /* entry assembly clears tags for TTBR0 addrs */ > - return addr < TASK_SIZE; > -} > - > static inline bool is_ttbr1_addr(unsigned long addr) > { > /* TTBR1 addresses may have a tag if KASAN_SW_TAGS is in use */ > @@ -749,30 +744,8 @@ void do_el0_irq_bp_hardening(void) > } > NOKPROBE_SYMBOL(do_el0_irq_bp_hardening); > > -void do_el0_ia_bp_hardening(unsigned long addr, unsigned int esr, > - struct pt_regs *regs) > -{ > - /* > - * We've taken an instruction abort from userspace and not yet > - * re-enabled IRQs. If the address is a kernel address, apply > - * BP hardening prior to enabling IRQs and pre-emption. > - */ > - if (!is_ttbr0_addr(addr)) > - arm64_apply_bp_hardening(); > - > - local_daif_restore(DAIF_PROCCTX); > - do_mem_abort(addr, esr, regs); > -} > -NOKPROBE_SYMBOL(do_el0_ia_bp_hardening); > - > void do_sp_pc_abort(unsigned long addr, unsigned int esr, struct pt_regs *regs) > { > - if (user_mode(regs)) { > - if (!is_ttbr0_addr(instruction_pointer(regs))) > - arm64_apply_bp_hardening(); > - local_daif_restore(DAIF_PROCCTX); > - } > - > arm64_notify_die("SP/PC alignment exception", regs, > SIGBUS, BUS_ADRALN, (void __user *)addr, esr); > } > -- > 2.20.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel