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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8677AC36010 for ; Fri, 4 Apr 2025 07:57:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9B13A6B0008; Fri, 4 Apr 2025 03:57:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 95F986B000A; Fri, 4 Apr 2025 03:57:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 827E66B000C; Fri, 4 Apr 2025 03:57:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 65EE46B0008 for ; Fri, 4 Apr 2025 03:57:13 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 46699121741 for ; Fri, 4 Apr 2025 07:57:13 +0000 (UTC) X-FDA: 83295605946.19.40FBCBD Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf25.hostedemail.com (Postfix) with ESMTP id 30CC1A0006 for ; Fri, 4 Apr 2025 07:57:11 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf25.hostedemail.com: domain of kevin.brodsky@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=kevin.brodsky@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1743753431; a=rsa-sha256; cv=none; b=eeY9iQs6tFOFIu3vG0GI0CuLgLElb0F/ElXAiLoJD1A81ZprSuXlV7aMfBR0ikKESJKpjn 60HJu3VNqbDZOCLLgGzqoQejFxFPOuzszwj2v4DV84JGHNULHVl8LfNEGSqffW9egszQ/H UYXS7bfxOWGDh9wb3i3oUqseSyjQbwg= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf25.hostedemail.com: domain of kevin.brodsky@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=kevin.brodsky@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1743753431; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9JeqvOReHWh/bsTIrcVRAGVDKWqZxvYEbZDt95bTYXk=; b=q0dV2A2FciZze5E/g27nurmktP0+WhRf1RKXWGO6F4Ss/EnoCL6bk/gjyt45QX/SG7Ec8T E7dvrhLUmlHAKW2dmgVsDNcU5MOt3ehKZej9KnLBb72YVYkBjOBvzDb/KDLUeE6G8euVm9 KqmxpNk6gMTFbJ19aMtIi7pj3+o8JM4= 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 77F131063; Fri, 4 Apr 2025 00:57:12 -0700 (PDT) Received: from [10.57.71.100] (unknown [10.57.71.100]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 26D223F59E; Fri, 4 Apr 2025 00:57:05 -0700 (PDT) Message-ID: <107650bf-a8c1-4a71-a302-2e80abd5d062@arm.com> Date: Fri, 4 Apr 2025 09:57:02 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [RFC PATCH v3 00/15] pkeys-based page table hardening To: Maxwell Bland Cc: linux-kernel@vger.kernel.org, Andrew Morton , Mark Brown , Catalin Marinas , Dave Hansen , Jann Horn , Jeff Xu , Joey Gouly , Kees Cook , Linus Walleij , Andy Lutomirski , Marc Zyngier , Peter Zijlstra , Pierre Langlois , Quentin Perret , "Mike Rapoport (IBM)" , Ryan Roberts , Thomas Gleixner , Will Deacon , Matthew Wilcox , Qi Zheng , linux-arm-kernel@lists.infradead.org, linux-mm@kvack.org, x86@kernel.org References: <20250203101839.1223008-1-kevin.brodsky@arm.com> <802963a0-32bd-49e8-82b1-34443acdd5ae@arm.com> Content-Language: en-GB From: Kevin Brodsky In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 30CC1A0006 X-Stat-Signature: 9ecuh3b5gosr36qczwcgbqnjdj7y9be9 X-HE-Tag: 1743753431-727355 X-HE-Meta: U2FsdGVkX18RPQE7bNlNo03I0KQQBSHpbhYS63U72cbEAque4APUP9l/rJSrucYPa54AxpR1KpbgBK0MQK3SwHMBxh/5g2pecvO/7GtqT0aQss3wdEQShZn0aB79wdgZMDIsXHuYemHP0jLIykCD1RLn00RDLHZN+kn3opAOWVG3nBlAyp5ZnO63coCbKeGaFi/kxBIYNrH+tMP5qDOqO7xI9Tigo2eJTvoVletyUSAhEOazw2ndJYtGtjJJs6HDGvbFSLwPxKYjd/78UJbXTnRSnFQpWiU4DQcoefb2YHD3gHMjRrSf2kFiYS+LROXWAmxdPvJIwZZEJZuQ3EfYQv6KolF0ruTSjjfAAfAAunHb0WEW+Aozpu0fsxZpxAwohZM+Tq50VTxLcuA6Mx3ZwzqhWGo6/cEkdssGQhvyKx0M89q/n9YCip/pSKqZNjMW9nqQxAZKrO8Z8bp/OUQieR8bQNHf4DOTRl+GDnf+fnUAU7Iac4bZGcevN4auyMim3Pkp+B19Dm7E2XV8ly8a5jHopmymgvGQOLtZ+b2GauSVSDmDXlNnxvFcBTFPC1O3gpvOdgkByPiPoquQzWCi0SvXEyZsHDAoLraVtvKFaU5+cC7+9lnnOpHnjP8cn1Cg58jb4y3zlgEgDaUTx8D8kSJRjlZ3dXDkUbddbx6UIdo0Fup6VGElsDGa7RXRONe8MdaXrykNVnVJowNQQk1Nlbln97LWrRitVnHafhJ1V/fuTmJQ+0UbedTeH02pKmtsPk8Xsv0f8MELQG42D1dTpkWWqhYUmmT/zN0ARSH4OqbavVvLLrEES/k9b+/zG+Zp/eAvZol+RLxP5bknzNLTZuswgM7YS9Tz5nNjgHpHDU8e0GeyzI2e9A8K8Au8OwHNeU4tt2vXJxX8XVRPX0L60xCYjWIdZfMxUqP6M1Kv3sBYnB29pnZo9sGzNjaHQh1LbIreOpcdyV0/hUIOOgc 4ZDjIbF8 NZcS4FWMWQOwhUmUFlRvpl9T3U1UoAeBEB855kN3ogUcNdWBFM3vUIb6XydIbFY8011WLx05+/aENf/tTCpaf2horhdS9bp+YHoVLXlJJtNotPesue0H1/vDltzieyAUhVfl/iNh9oEBNy661AafUysLL2A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 28/03/2025 17:15, Maxwell Bland wrote: > [...] > > However, the "real" idea here is: > > (1) "splitting up" each RW data structure ($X$) into mutable and non-mutable > parts would be difficult and difficult to maintain, so I was looking for > a solution outside of that. > (2) The primitive we do have, thanks to POE and this patch, is > the ability to make a page of memory writable only from a specific > context, and read only otherwise. > (3) The well-trodden solution is to allocate a field on a second, > independent page $p$ when allocating $X$, with some field/hash/magic to > guarantee the integrity of writes to the immutable fields of $X$ > (sometimes called shadow memory). > > Valid, CFI-guaranteed functions would have access to $p$, and would > be instrumented to update $p$ with a new hash of the fields of $X$ > when updating $X$, but likely something more performance-friendly. > When readig from $X$, the data is pulled from $p$ to ensure the > field being read from $X$ is valid and has not been modified. It is > not possible to modify $p$ outside of the valid contexts in which > we can modify the read-mostly or constant fields of $X$. > > Importantly, this does not rely on the confidentiality of $p$, which I > think was an issue with trying an approach like this with ARM MTE > previously, or at least the version of ARM MTE that Juhee Kim et al. at > Georgia Tech broke with speculative exectuion, iirc. > > I think performance is difficult here, but that's more of a development > concern, I hope, than a concern in theory. Thank you for elaborating, this is much clearer now. I suppose this requires that all read-mostly fields are accessed via helpers that will check/update $p$. Painful when those fields are directly accessed today, as in the case of task_struct, but the required changes are hopefully easy to automate (using Coccinelle for instance). And as you point out further down, this could be done via a compiler attribute instead. The performance impact on reads is clearly a concern, but it is not an all-or-nothing scheme - we can choose which members are protected, meaning we can make trade-offs. Overall this seems worth investigating. I wonder, have you considered how accessors would find the shadow memory? It could of course be linked directly from task_struct, but then nothing prevents that pointer from being corrupted. I can't think of another cheap way to link $p$ though. This is not a full-blown shadow memory approach, so I'm not sure we can reserve a whole chunk of the address space for that purpose. >> [...] >> >> A different angle would be use an attribute to mark struct members, >> rather than functions. That would instruct the compiler to switch kpkeys >> level whenever that member is written (and only then). You would >> probably want to forbid taking the address of the member too, as the >> compiler can't easily track that. That said this doesn't solve the >> bigger issue of existing code expecting to be able to write to other >> members in the struct. It most likely works only if the kpkeys switch is >> done when writing to any member of the struct (which may get very >> expensive). > We agree. Doing something like this doesn't crash stuff, but it makes > the phone sluggish and terrible to use. (-: Hence, I may try the above: > keep the struct read-write, but when reading from "critical fields" > (pointers to function pointers), require the compiler to inject a check > for an integrity value stored on a mostly-read-only page. That integrity > value can only be updated by code that is resonsible for writing said > critical fields. > > Since the supposition is things like f_ops don't really need to change > much ($p$ does not need to be accessed much), and otherwise the data > structure is fully writable, the performance impact seems like it would > not be significant. Agreed, it doesn't have to be very expensive if used sparingly. > That said, and if I am not mistaken, the downside is it'd require > Clang/GCC support, same as CFI. Indeed. For experimenting a Coccinelle script to convert direct access to certain members to a function call is probably easier :) > >>> [...] >>> Noticed as well, just now, the reliance on the static_key for branch >>> unlikely ... the following data structure (at the end of this email) may >>> be of interest as an FYI ... it can be used to track whether the kernel >>> self patching API is only being used to patch expected locations. >>> >>> I use this right now with additional checks that the instruction being >>> written for the indirect branch matches the expected offset. >> Are there exploits out there corrupting the address of a static branch? >> This seems difficult to me in practice because the __jump_table section >> where the addresses of instructions to be patched are stored is marked >> __ro_after_init. > There are a couple of different ways. You can do the attack this patch > is intended to prevent, change the page tables unwillingly to give > yourself permissions to write to the static key struct as part of a > larger chain, there's the ability to inject code into a kernel > module to call the patch text API and use it as a write gadget for the > rest of the kernel, etc.. Right. I'd argue that static keys are not your biggest worry if the attacker can control page tables or call arbitrary functions - and in fact kpkeys are easily defeated if the attacker is able to modify the control flow (one could simply call kpkeys_set_level()). > There were a lot of claims back in the day that kernel code would be > marked strictly read-only by the EL2 secure monitor or kernel proection > systems, but there's self-modifying code built right into it under many > KConfigs. To have a guarantee of CFI you kind of have to ensure the > kernel can't patch itself. Self-patching is used extensively on boot (on arm64 at least) to support optional extensions such as POE. I suppose this kind of patching isn't really a concern as it occurs very early. Static keys can be used at any point and are therefore more dangerous, but the performance uplift is likely significant in many cases. - Kevin