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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 43848C433EF for ; Tue, 9 Nov 2021 17:56:50 +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 06910610A0 for ; Tue, 9 Nov 2021 17:56:50 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 06910610A0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=fgAgy0eD/CM2WsuYXVEa1xzvE0QoSgZMhAkwUOmr8PY=; b=oXnYR3MLPC8XoL g+gTEtfUdjUZCHN3rJK4HY3rI4C+1+WMUvKA2JxQk316Cv/Sn8u4Ce9L8e/k6F5g3kIUQqVq52/ff wkI1fAVfV9PlE266D6Xbf0PStBLo9q7nNUs5XHZcEcyueiM9TFHVDFsXNAa18RNlCcnS+xnbCo1Tu eVwjeSHulv8ghwqpoVfyKvLSWlf9be9l+NEUV6IgoOEQG5Canos0lFG54fT3357nekCIpZPGibrRh xAdaxECIoVTzIFMAvkyIgDADZyMiXY+9FhIsQUwQSJ+SbImRNyI/2DWLILg0d60bVR5xFKN6MQ/N0 gBjbSWIBkbbt9pqIlCZA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mkVLK-002vw6-Ak; Tue, 09 Nov 2021 17:55:34 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mkVLG-002vv7-3O for linux-arm-kernel@lists.infradead.org; Tue, 09 Nov 2021 17:55:31 +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 DD0B4ED1; Tue, 9 Nov 2021 09:55:28 -0800 (PST) Received: from FVFF77S0Q05N (unknown [10.57.58.237]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A0F003F7F5; Tue, 9 Nov 2021 09:55:26 -0800 (PST) Date: Tue, 9 Nov 2021 17:55:17 +0000 From: Mark Rutland To: Ard Biesheuvel Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Quentin Perret , Catalin Marinas , James Morse , Will Deacon , Frederic Weisbecker , Peter Zijlstra , Kees Cook , Sami Tolvanen , Andy Lutomirski , Josh Poimboeuf , Steven Rostedt Subject: Re: [PATCH v6 2/2] arm64: implement support for static call trampolines Message-ID: References: <20211105145917.2828911-1-ardb@kernel.org> <20211105145917.2828911-3-ardb@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20211105145917.2828911-3-ardb@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211109_095530_286637_A5EEC670 X-CRM114-Status: GOOD ( 27.56 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Hi Ard, On Fri, Nov 05, 2021 at 03:59:17PM +0100, Ard Biesheuvel wrote: > +static void *strip_cfi_jt(void *addr) > +{ > + if (IS_ENABLED(CONFIG_CFI_CLANG)) { > + void *p = addr; > + u32 insn; > + > + /* > + * Taking the address of a function produces the address of the > + * jump table entry when Clang CFI is enabled. Such entries are > + * ordinary jump instructions, preceded by a BTI C instruction > + * if BTI is enabled for the kernel. > + */ > + if (IS_ENABLED(CONFIG_ARM64_BTI_KERNEL)) > + p += 4; > + > + insn = le32_to_cpup(p); > + if (aarch64_insn_is_b(insn)) > + return p + aarch64_get_branch_offset(insn); > + > + WARN_ON(1); > + } > + return addr; > +} I'm somewhat uncomfortable with this, because it seems like the compiler could easily violate our expectations in future, and then we're in for a massive headache. I assume clang doesn't provide any guarnatee as to the format of the jump table entries (and e.g. I can see scope for branch padding breaking this). In trying to sidestep that I ended up with: https://lore.kernel.org/linux-arm-kernel/20211109172408.49641-1-mark.rutland@arm.com/ ... which I think is a good option for PREEMPT_DYNAMIC, but I don't know if there were other places where we believe static calls would be critical for performance rather than a nice-to-have, and whether we truly need static calls on arm64. My mind is leaning towards "avoid if reasonable" at the moment (or at least make that mutually exclusive with CFI so we can avoid that specific fun). I see you had at least one other user in: https://lore.kernel.org/r/20211109120336.3561463-1-ardb@kernel.org ... what were your thoughts on the criticality of that? FWIW other than the above this looks good to me. My major concern here is fragility/maintenance, and secondly whether we're gaining much in practice. So if you think we really need this, I'm not going to stand in the way. Thanks Mark. > +void arch_static_call_transform(void *site, void *tramp, void *func, bool tail) > +{ > + /* > + * -0x8 > + * 0x0 bti c <--- trampoline entry point > + * 0x4 > + * 0x8 ldr x16, > + * 0xc cbz x16, 20 > + * 0x10 br x16 > + * 0x14 ret > + */ > + struct { > + u64 literal; > + __le32 insn[2]; > + } insns; > + u32 insn; > + int ret; > + > + insn = aarch64_insn_gen_hint(AARCH64_INSN_HINT_BTIC); > + insns.literal = (u64)func; > + insns.insn[0] = cpu_to_le32(insn); > + > + if (!func) { > + insn = aarch64_insn_gen_branch_reg(AARCH64_INSN_REG_LR, > + AARCH64_INSN_BRANCH_RETURN); > + } else { > + insn = aarch64_insn_gen_branch_imm((u64)tramp + 4, > + (u64)strip_cfi_jt(func), > + AARCH64_INSN_BRANCH_NOLINK); > + > + /* > + * Use a NOP if the branch target is out of range, and rely on > + * the indirect call instead. > + */ > + if (insn == AARCH64_BREAK_FAULT) > + insn = aarch64_insn_gen_hint(AARCH64_INSN_HINT_NOP); > + } > + insns.insn[1] = cpu_to_le32(insn); > + > + ret = __aarch64_insn_write(tramp - 8, &insns, sizeof(insns)); > + if (!WARN_ON(ret)) > + caches_clean_inval_pou((u64)tramp - 8, sizeof(insns)); > } > > int __kprobes aarch64_insn_patch_text_nosync(void *addr, u32 insn) > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 50bab186c49b..e16860a14eaf 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -173,6 +173,7 @@ SECTIONS > HIBERNATE_TEXT > KEXEC_TEXT > TRAMP_TEXT > + STATIC_CALL_TEXT > *(.gnu.warning) > . = ALIGN(16); > *(.got) /* Global offset table */ > -- > 2.30.2 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel