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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id F053CCF11CD for ; Thu, 10 Oct 2024 10:52:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Transfer-Encoding:Content-Type: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=9YZp5De49qUR1IVIXlFWH/xslV5ejFpzLGUga4nL4bA=; b=gPBx6U34qN7hDvXBEVfevqKNlx UB8RqoBO1wtPem67S5rHVcWbrZ25G7yFLL9SVcVn37rphvmdZ/Z/MQ/wdu91ebuE68tJ4qSvAMSFV ATlkWchm73BViNKEgHGQ3fZLMxGjMCIQSjY7U9yV6iT+O9VFOoKg6kpso6AtIIC4nyFVwZDNAYAHd AqHgKhwaQzchRYxtAkVnwSZ6dPFChHO1O1hMb1fvydmH1bexmUOu5HU9+z9gn/h69hax7Gu2St4Lk zq3gtLwAC290MMzeF9NU80IfA/vXn/GtgGKx5bzd/qTXBaOzPc3XM6kcrLeaqQRkj95Uqzv109ZYx D27swTuQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1syqmd-0000000CSxu-0GqD; Thu, 10 Oct 2024 10:52:39 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1syqfV-0000000CRf9-2Q2r; Thu, 10 Oct 2024 10:45:21 +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 31F8B497; Thu, 10 Oct 2024 03:45:46 -0700 (PDT) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1225C3F58B; Thu, 10 Oct 2024 03:45:11 -0700 (PDT) Date: Thu, 10 Oct 2024 11:45:07 +0100 From: Mark Rutland To: Alice Ryhl Cc: Matthew Maurer , Sami Tolvanen , Catalin Marinas , Will Deacon , Huacai Chen , WANG Xuerui , Paul Walmsley , Palmer Dabbelt , Albert Ou , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?B?QmrDtnJu?= Roy Baron , Benno Lossin , Andreas Hindborg , Trevor Gross , Kees Cook , "Peter Zijlstra (Intel)" , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, loongarch@lists.linux.dev, linux-riscv@lists.infradead.org, rust-for-linux@vger.kernel.org Subject: Re: [PATCH] cfi: rust: pass -Zpatchable-function-entry on all architectures Message-ID: References: <20241008-cfi-patchable-all-v1-1-512481fd731d@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20241010_034518_333421_09BC5258 X-CRM114-Status: GOOD ( 41.17 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Oct 09, 2024 at 10:15:35PM +0200, Alice Ryhl wrote: > On Wed, Oct 9, 2024 at 7:43 PM Mark Rutland wrote: > > > > Hi Alice, > > > > On Tue, Oct 08, 2024 at 05:37:16PM +0000, Alice Ryhl wrote: > > > The KCFI sanitizer stores the CFI tag of a function just before its > > > machine code. However, the patchable-function-entry flag can be used to > > > introduce additional nop instructions before the machine code, taking up > > > the space that normally holds the CFI tag. > > > > To clarify, when you say "before the machine code", do you mean when > > NOPs are placed before the function entry point? e.g. if we compiled > > with -fpatchable-function-entry=M,N where N > 0? I'll refer tho this as > > "pre-function NOPs" below. > > > > There's an existing incompatibility between CFI and pre-function NOPs > > for C code, because we override -fpatchable-function-entry on a > > per-function basis (e.g. for noinstr and notrace), and we don't > > currently have a mechanism to ensure the CFI tag is in the same place > > regardless. This is why arm64 has CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS > > depend on !CFI. > > > > For C code at least, just using regular -fpatchable-function-entry=M or > > -fpatchable-function-entry=M,0 shouldn't change the location of the CFI > > tag relative to the function entrypoint, and so should have no adverse > > effect on CFI. > > > > Is Rust any different here? > > Ah, no it shouldn't be. Sami can you confirm? > > > > In this case, a backwards offset is applied to the CFI tag to move > > > them out of the way of the nop instructions. To ensure that C and Rust > > > agree on the offset used by CFI tags, pass the > > > -Zpatchable-function-entry to rustc whenever it is passed to the C > > > compiler. > > > > As above, I suspect this isn't necessary to make CFI work, for any case > > that works with C today, due to -fpatchable-funtion-entry being > > overridden on a per-function basis. Are you seeing a problem in > > practice, or was this found by inspection? > > > > However IIUC this will allow rust to be traced via ftrace (assuming rust > > records the instrumented locations as gcc and clang do); is that the > > case? Assuming so, is there any ABI difference that might bite us? On > > arm64 we require that anything marked instrumented with > > patchable-function-entry strictly follows the AAPCS64 calling convention > > and our ftrace trampolines save/restore the minimal set of necessary > > registers, and I don't know how rust whether rust will behave the same > > or e.g. use specialized calling conventions internally. > > Well, I was told that it's a problem and was able to trigger a failure > on x86. I didn't manage to trigger one on arm64, but I wasn't sure > whether that was me doing something wrong, or whether the problem only > exists on x86. We already have the flag on x86 for FINEIBT, I believe that hte problem only exists on x86, becaause they use patchable-function-entry for their FINEIBT patching (and use -mfentry for ftrace), whereas everyone else uses patchable-function-entry for ftrace. > but I thought on the off chance that it's not a problem in practice on > arm, it still doesn't hurt to add the flag. It won't adversely affect CFI, but it will open up rust code for ftrace, so I'm not sure that "it doesn't hurt". AFAICT at the moment this isn't necessary for CFI, so can we drop this patch for now? If we want to pass these flags for !x86, the justification should be to enable ftrace for rust code, and we should test that actually works (e.g. by testing ftrace with rust code). What happens on x86 for ftrace and rust? > Regarding the AAPCS64 calling convention thing ... rustc uses the Rust > calling convention for functions internally in Rust code and I don't > know whether that changes anything relevant for what you mention. > Matthew/Sami do you know? >From their replies it sounds like that happens to be true in practice today, but as above I think we should go test this actually works. Mark.