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 BAB5DC282DE for ; Thu, 13 Mar 2025 12:32:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CFD15280002; Thu, 13 Mar 2025 08:32:49 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CAE73280001; Thu, 13 Mar 2025 08:32:49 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B756C280002; Thu, 13 Mar 2025 08:32:49 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 9AE9A280001 for ; Thu, 13 Mar 2025 08:32:49 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 5C736142010 for ; Thu, 13 Mar 2025 12:32:50 +0000 (UTC) X-FDA: 83216466900.13.377EDF8 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf22.hostedemail.com (Postfix) with ESMTP id 7E398C0021 for ; Thu, 13 Mar 2025 12:32:48 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf22.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=1741869168; a=rsa-sha256; cv=none; b=hRj0OQbT61QKuMjJ044WJO9Vk9BsGpLmrWBUXOD997y+/eV5NwszN3/id07TDTh00t/+24 eqmsWXiH6qLNObXRIw8Jn+KsCZj5LzILygdDMTmu7i8MKMh0zBtN4uMqEf0UrzHn8DDW+e /JgDWktgymZXVYUi16Lf9h7cwxtk0Ag= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf22.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=1741869168; 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=6BWoHZS+Irq6JCkkTZ531xu2Qq8vmZ0tkfTVINeJP7c=; b=Q/LUHTz1nW1M4gLrAAlfDXkTq0cUOSMPjERdgvNKvQx4EqDJBZCxrbSgojdibpm5oRc5wb xOoQYmdJNa3T6qvmLBQkdQMsM061I7Gel8gskKr8uQdwHWQ9uSfmrON+yKKpFyb1gFgqAW K+Xu2EJELOd8gPzhDbTULx1Wm5/3l2I= 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 E0B091C00; Thu, 13 Mar 2025 05:32:57 -0700 (PDT) Received: from [10.44.160.94] (e126510-lin.lund.arm.com [10.44.160.94]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 99EC03F673; Thu, 13 Mar 2025 05:32:42 -0700 (PDT) Message-ID: <802963a0-32bd-49e8-82b1-34443acdd5ae@arm.com> Date: Thu, 13 Mar 2025 13:32:31 +0100 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> 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-Queue-Id: 7E398C0021 X-Rspamd-Server: rspam05 X-Stat-Signature: cdt1ikucin5s848scgq9tbwodtj1i4ug X-HE-Tag: 1741869168-819076 X-HE-Meta: U2FsdGVkX1/tbExp/0jY9DF5IpnfKrP2eBBXNf8ibo9/JGVV+MVYfQcBf5fZzJ5uPj9lAA/Y9rJwthOk6NgRTPzjURPbHO4A7WyMCVPbVi9rgyQxQYQDb6q/Bu1gCtNF81etwSVejJ3lHB0YWujmgV4Q8jE2etcjVpjLJbvEga6VSrx+Jssc4Xjt+CKpyAU4ZPGO9Px4JbEgPdXOYb+2QacMz2oH+bYcp3Lct4/TWYokaZFlUCh/HoF6kwF20o6qar80lgzHluXLcfx2wmfzN0ssFY6O+22/EsG21+Itz0eJhsdi/i4ncXUEhHmF0ezvYjonx53R+pDRseOYmN9G3RpVABW9LCyDWIB4wx/it1Dje9NN7jqXx2LNlrydL1L4PNRbtrBgXCqcimBTBtYNcao3DTvR2YK+rSSfRovPYOd8Un7v1NqF1vtcxmbCsBT/MrZpJsRTxoiye6zfiUa3K13L6q5oYG8aF25RATNwHoTxS0xzPsl607EkToYOtySMUC5Qh10O3NXSvrlyCY8SzeqWu/TjCADFlsLdD4HpSCbrFU3CjwLTGjyCw53yr0kQLlYO8hrwb5BO1zEc9p5vQuAlz6RDw174ULAVFrOkJS2WVzL3ZZ1LNSkEM/JKKEOfQY5CTHV86gJ7dC3VXFqRwISfYLl6Btp3h1EuZB9zjknwpxd8khwopeVBNUt+lLWCs5UYCJkjoSY3ddkhs7t3ZyhGCQeMhi7M1AVkZA+pbxW+0FVoCZURe7C/v2tIRTg3S3a29WCQP9WNwP5x03Wxj55Nonrgsbp9uFxExkRxOuEnsPHkdJurEetBDzD6fLrBAJkk9zGFUtfI7BNE12xSWMe78hIdtoJ8F1ew1yHuYaZItRbjfhw8Kaf2U/hxigspvRaZ7VKhUk2zAPpbV2rLL5qrsQJSv0HDaTtr3UiFZkghZPStYxrFupbwk5JH8+O24V0cX1cMXGbpZTaflsQ SzwjQ/pm 7t1njwYGp7A0JSyRtKIK1WumL1CMS7s12qTDBISM4gQ/zR7gAqJKZX2rrhTl35auPpCPqAN/BhpwkBfaFmCoEL+cXS9ocbbw2TfhD+nMDAyR2TeFdpy4Qp4UW4TkumJSRkTHAp0b0JxMfdUuP6YuaB4Cwrz/AzFNNOa3qKxGvA+FyZVddSguv9YLd2PKAnPQqiCYQiikiEdWRbXiCrGzN80EsoXxR+BVesOeE 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 06/03/2025 17:23, Maxwell Bland wrote: > On Mon, Feb 03, 2025 at 10:18:24AM +0000, Kevin Brodsky wrote: >> This is a proposal to leverage protection keys (pkeys) to harden >> critical kernel data, by making it mostly read-only. The series includes >> a simple framework called "kpkeys" to manipulate pkeys for in-kernel use, >> as well as a page table hardening feature based on that framework >> (kpkeys_hardened_pgtables). Both are implemented on arm64 as a proof of >> concept, but they are designed to be compatible with any architecture >> implementing pkeys. > Hi Kevin, > > This is awesome. When working on some of these problems, I also thought > of leveraging the POE feature, but was not sure of a good way to make > it work. Hi Maxwell, I'm glad to hear about your interest! >> Page tables were chosen as they are a popular (and critical) target for >> attacks, but there are of course many others - this is only a starting >> point (see section "Further use-cases"). It has become more and more >> common for accesses to such target data to be mediated by a hypervisor >> in vendor kernels; the hope is that kpkeys can provide much of that >> protection in a simpler manner. No benchmarking has been performed at >> this stage, but the runtime overhead should also be lower (though likely >> not negligible). > Some notes here, having implemented similar page table protections, > albeit using stage-2 page table permissions and a fault handler. > > https://lore.kernel.org/all/2wf4kmoqqmod6njviaq33lbxbx6gvdqbksljxykgltwnxo6ruy@7ueumwmxxh72/ > > I wanted to know your thoughts on associating specific policies to > page table updates in cases where an adversary is able to corrupt > other state associated with parameters to the page table infrastructure > code, e.g. > > arch/arm64/net/bpf_jit_comp.c > 2417: if (set_memory_rw(PAGE_MASK & ((uintptr_t)&plt->target), > > Is this something we would assume is handled via the security_* hook > infrastructure, a shadow stack CFI method, or changing the kernel code > to reverify the data non-modularly, some combination of the above? This is a good question and the short answer is that I don't know what the best approach would be. As far as kpkeys are concerned, I'm not sure they can really help here - it seems to be about the integrity of a generated BPF program, and surely that program is already made read-only once generated? >> - Pages in the linear mapping are assigned a pkey using set_memory_pkey(). >> This is sufficient for this series, but of course higher-level >> interfaces can be introduced later to ask allocators to return pages >> marked with a given pkey. It should also be possible to extend this to >> vmalloc() if needed. > One of the interesting points here, acknowledged below, was that this > relies on having guarantees around the PC value/CFI of the function. > Since this is the baseline assumption, it feels natural that the > locking/unlocking would be associated into the existing CFI > instrumentation, since (from experience) point-patching each mutable > data structure allocation/deallocation is difficult. Could you elaborate on this point? Are you referring to protecting arbitrary data structures with kpkeys, rather than what this series covers (i.e. pgtables)? >> # Further use-cases >> >> It should be possible to harden various targets using kpkeys, including: >> >> - struct cred (enforcing a "mostly read-only" state once committed) >> >> - fixmap (occasionally used even after early boot, e.g. >> set_swapper_pgd() in arch/arm64/mm/mmu.c) >> >> - SELinux state (e.g. struct selinux_state::initialized) >> >> ... and many others. > Be wary that it is not just struct cred but pointers to struct cred. We > quickly run into a problem of, for example, in updates to f_op, > bookkeeping which f_op is OK to have in the file, e.g., in Android's > 6.1.x: > > drivers/gpu/drm/i810/i810_dma.c > 138: file_priv->filp->f_op = &i810_buffer_fops; > 144: file_priv->filp->f_op = old_fops; > > drivers/staging/android/ashmem.c > 436: vmfile->f_op = &vmfile_fops; > > Where overwriting f_op is a "classic" bypass of protection systems like > this one. Indeed, this has been pointed out to me on a few occasions. On that note I should reference the follow-up series that aims at protecting struct cred [1]. Using kpkeys to ensure that the task->cred pointer itself is mostly read-only is not straightforward, because there is just too much existing code that directly writes to other fields in task_struct - meaning we can't simply make the whole task_struct mostly read-only. What could be done is to split out the cred fields in a separate kpkeys-protected page, and link that page from task_struct. There is a clear overhead associated with this approach though, and maybe more importantly we have just displaced the issue as the pointer to that protected page remains writeable... I'm not sure how much a page-based technique like pkeys can help here. [1] https://lore.kernel.org/linux-hardening/20250203102809.1223255-1-kevin.brodsky@arm.com/ > I think this problem may be totally solvable if POE was integrated into > something like CFI, since we can guarantee only the code that sets f_op > to "vmfile_fops" can unlock/relock the file's page. I'm interested to hear more about this - I don't think I'm aware of the CFI instrumentation you're referring to. > Maybe another approach would work better, though? > >> # Open questions >> >> A few aspects in this RFC that are debatable and/or worth discussing: >> >> - There is currently no restriction on how kpkeys levels map to pkeys >> permissions. A typical approach is to allocate one pkey per level and >> make it writable at that level only. As the number of levels >> increases, we may however run out of pkeys, especially on arm64 (just >> 8 pkeys with POE). Depending on the use-cases, it may be acceptable to >> use the same pkey for the data associated to multiple levels. > Honestly, I associate each protected virtual page in stage-2 with a > unique tag (manually, right now, but Kees Cook has some magic that > does the same via alloc_tag.h), and this works really well to track > specific resources and resource modification semantics "over" a generic > protection ring. > > I think, though, that the code you provided could be used to bootstrap > such a system by using the overlay to protect a similar page tag lookup > table, which then can provide the fine-grained protection semantics. This sounds sensible. Considering that the size of pkeys is between 3 and 5 bits depending on the architecture, to keep the approach generic we really don't have many keys to play with (7 excluding the default key on arm64/POE). kpkeys approaches keys in a static way (no dynamic allocation), so it's best to think of each key as corresponding to one subsystem defined at compile time. > I.e. use this baseline to isolate a secure monitor system. > > Hopefully that makes sense! (-: > >> - kpkeys_set_level() and kpkeys_restore_pkey_reg() are not symmetric: >> the former takes a kpkeys level and returns a pkey register value, to >> be consumed by the latter. It would be more intuitive to manipulate >> kpkeys levels only. However this assumes that there is a 1:1 mapping >> between kpkeys levels and pkey register values, while in principle >> the mapping is 1:n (certain pkeys may be used outside the kpkeys >> framework). > Another issue I'm not confident in is the assumption of adversary's > inability to manipulate system control registers. This is true in the > context of a Heki-like system (or any well-made HVCI), but not totally > true of a pure EL1 implementation? This is entirely reliant on the threat model, i.e. only data-only attacks are in scope. If the adversary successfully hijacks the control flow to jump in the middle of a function, or achieves arbitrary code execution, pkeys won't be of much help. Robust CFI is therefore a clear prerequisite. >> - An architecture that supports kpkeys is expected to select >> CONFIG_ARCH_HAS_KPKEYS and always enable them if available - there is >> no CONFIG_KPKEYS to control this behaviour. Since this creates no >> significant overhead (at least on arm64), it seemed better to keep it >> simple. Each hardening feature does have its own option and arch >> opt-in if needed (CONFIG_KPKEYS_HARDENED_PGTABLES, >> CONFIG_ARCH_HAS_KPKEYS_HARDENED_PGTABLES). > There's so many pieces of data though/data structures! In this model, > you'd have a separate switch for thousands of types of data! But I do > think protecting PTs is the first step to a more complicated security > monitor, since it allows you to have integrity for specific physical > pages (or IPAs). Agreed. I think this is good enough for kpkeys where we use roughly one key per feature (meaning that the total number of features is pretty limited), but this wouldn't scale if we had a framework to protect many data types. In that case we'd need a different approach, probably runtime-based. The advantage of having a CONFIG option per feature though is that it incurs no overhead at all unless selected. >> >> Any comment or feedback will be highly appreciated, be it on the >> high-level approach or implementation choices! > Last note, I'd not totallllyyy trust the compiler to inline the > functions.... I've met cases where functions on memory protections > I expected to be inlined were not. I think __forceinline *may* work > here, vs standard "static inline", but am not confident/sure. That's a fair point, __always_inline could be used to make sure set_pkey/restore_pkey_reg (and the helpers they call) are actually inlined. I'll change that in v4. The guard object constructor/destructor are defined as regular inline functions in though, so the inlining might not be complete. > Hopefully the above is valuable at all. Thanks! It is! Thank you for your thoughts and suggestions. - Kevin