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 2A38BC4167B for ; Mon, 11 Dec 2023 18:49:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B699C6B01A1; Mon, 11 Dec 2023 13:49:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B18BF6B01A2; Mon, 11 Dec 2023 13:49:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A08286B01A3; Mon, 11 Dec 2023 13:49:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 8D8CC6B01A1 for ; Mon, 11 Dec 2023 13:49:45 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 623891C0E17 for ; Mon, 11 Dec 2023 18:49:45 +0000 (UTC) X-FDA: 81555426330.23.E6D2D0F Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf24.hostedemail.com (Postfix) with ESMTP id B5F67180027 for ; Mon, 11 Dec 2023 18:49:43 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1702320583; 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: in-reply-to:in-reply-to:references:references; bh=Oy6dkqW9vk4hB8+pEbvRb5m5YOQv0DonI8tve4MnzWc=; b=S3walM4T0u5eDjnKj6SabZ2KWahixiT804IKsVnAvDBChi9Q4MMlZCRYY0PLT+KZyzVXVP /Ab6TRNoCFvFqjfbp3EBPLMvZLk0cMXCXaWT4rmxWemIFfCchuTft5lRSIYSUJiJSxxVIb BbYXrHz0urv5GQ8kF7C3FxlfZQEjq/A= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=none; spf=pass (imf24.hostedemail.com: domain of cmarinas@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=cmarinas@kernel.org; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=arm.com (policy=none) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1702320583; a=rsa-sha256; cv=none; b=2C/0XacmYx7R10XglvZQbgnFKo22eFkh3n6P47UEOMoLu2zrpEEOt8r7zfSg4cSH/JVmd7 urYgHz1XW8Iu7o5Q5jiP2gTe3H7Wvcl/xBZXpb5BmP6XXYDvnxTSSqJQmrl2kHCV6r4X/1 5SWWDJJ9bzDWleu9rxGGGZ+9MJbxe7w= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id C2AEB61502; Mon, 11 Dec 2023 18:49:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9584DC433C8; Mon, 11 Dec 2023 18:49:39 +0000 (UTC) Date: Mon, 11 Dec 2023 18:49:37 +0000 From: Catalin Marinas To: Joey Gouly Cc: linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, aneesh.kumar@linux.ibm.com, broonie@kernel.org, dave.hansen@linux.intel.com, maz@kernel.org, oliver.upton@linux.dev, shuah@kernel.org, will@kernel.org, kvmarm@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, James Morse , Suzuki K Poulose , Zenghui Yu Subject: Re: [PATCH v3 14/25] arm64: implement PKEYS support Message-ID: References: <20231124163510.1835740-1-joey.gouly@arm.com> <20231124163510.1835740-15-joey.gouly@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20231124163510.1835740-15-joey.gouly@arm.com> X-Rspamd-Queue-Id: B5F67180027 X-Rspam-User: X-Stat-Signature: mk11ksqfr5unqkmykdrtiu9rk5ypg84o X-Rspamd-Server: rspam01 X-HE-Tag: 1702320583-547057 X-HE-Meta: U2FsdGVkX18sqzDkG7tcOhCLuDA2ljHHlWE2y8TFEbX+j6ESzCf2/KzMi7essITZGElsBx6vK1KKZ/XDseyKlqsmbRQUInI0bdjSmY0LDUAAMVxTO5LqWxLx5NUDswD2Ef5KALQVEph9WLUdVMS/U2y+Jb1PZ7vVelw+ExBpe4pgBzrPfG3k33Und5CDlForNmZ1s/ciIdtGTeAp+nEOp4Tt83fuwHKwRyCNERj2yvfzA/mJpP6yyJAphn0YJ84xEWtjTXBGhRoyD6yHr/NvtCupqmm8HJPJzzR1NqVUvHDyMBTaxp5KyG41pk2J2pPhaHIixUjdNwT4btlILmQYVwE9JwEWEuQbewvi7guBTxBOLLGodfnwT2t1FqGRF7WZ8cJkFpQapyvbaaxPNVZLPeIqUPpexRpBJD41aBwszsNF1bPJdTXWV1ZMnPabTgksOCkbQJW3bwmAoARGMhSaQMCFwakhcuFrbkpsQivmogYKu8ryG9OAErkVv5Q3DqcpVvf31QtVbnzLxkb8w8q0ECNIHWho0VdTWpyZC7U9XcWmVMPO+2q/iAX54oarTQ2On9v+qUDxv5hllrqcVt04ps5wI7avmR5MpzE67PHzX6MYGFjdMQjVMUAqHJ5UnS+uyyoIW99iWCLQJ4GiK/H78sjje40aF4oBilvJPfm2w6B4WeaGIur1LntprltNmpi/SUD/SKuFF23w+6NBBk4rAu80sLYFec1rpI77FC7uUTGOC85L6KcZLwA30CXK0Vzp8ITPHR4A3Z6BHesb/08xUC06CDnbI6MY9jsKGYp7H+8zNtt1k7zL+0djct1grEwixhbu3Vo+gFNJMthpZDV8tkgkrdcIUQJN2Il21vDdM6ezZIhF2XiZcgKLno3U/4NWXn4YVGwaeTm+uU2fAW/NptXFIVV9NgeFWtueQEC2Gttm941dxSFfeSeu0lKJvJy29GWcTC5ic7lwB7pmbzU GpHi+F6x WIrQ1yzLv0OYcjleJVXfdz6vH8mAndos35ujhtxaqjFw/x7YCyQ8sOQ1FOEsLyT5mBMsyWx+goHJgK2WPegAvw0Iwo62MZRW5ncKTfxmTBQi/BMtyMPktyi/Gv+KTBBlTzL/lFjTRo/ZtkMyeM0PukPiowP+N9ok34dNqOxZ9/Ex8CMg/hG5CmY/NkDJcavSaA10vh9TSQ/DbgzluSyZzYrX5y9NpMEugr5ezi27ukgmqhoe1q2fiFKkMEUwdczqoNSgrcbcUbC99LVu0+/Fv2+XngjuCIios0DxPrCm1Ez+BxNL7Z9Wxftji/aIcHwUeMQIL 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 Fri, Nov 24, 2023 at 04:34:59PM +0000, Joey Gouly wrote: > @@ -211,11 +212,24 @@ init_new_context(struct task_struct *tsk, struct mm_struct *mm) > { > atomic64_set(&mm->context.id, 0); > refcount_set(&mm->context.pinned, 0); > + > + // pkey 0 is the default, so always reserve it. > + mm->context.pkey_allocation_map = 0x1; Nit: use /* */ style comments. > @@ -151,7 +170,9 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys) > * PTE_VALID bit set. > */ > #define pte_access_permitted(pte, write) \ > - (((pte_val(pte) & (PTE_VALID | PTE_USER)) == (PTE_VALID | PTE_USER)) && (!(write) || pte_write(pte))) > + (((pte_val(pte) & (PTE_VALID | PTE_USER)) == (PTE_VALID | PTE_USER)) && \ > + (!(write) || pte_write(pte)) && \ > + por_el0_allows_pkey(FIELD_GET(PTE_PO_IDX_MASK, pte_val(pte)), write, false)) Do not change pte_access_permitted(), just let it handle the base permissions. This check is about the mm tables, not some current POR_EL0 setting of the thread. As an example, with this change Linux may decide not to clear the MTE tags just because the current POR_EL0 says no-access. The thread subsequently changes POR_EL0 and it can read the stale tags. I haven't checked what x86 and powerpc do here. There may be some implications on GUP but I'd rather ignore POE for this case. > #define pmd_access_permitted(pmd, write) \ > (pte_access_permitted(pmd_pte(pmd), (write))) > #define pud_access_permitted(pud, write) \ > diff --git a/arch/arm64/include/asm/pkeys.h b/arch/arm64/include/asm/pkeys.h > index 5761fb48fd53..a80c654da93d 100644 > --- a/arch/arm64/include/asm/pkeys.h > +++ b/arch/arm64/include/asm/pkeys.h [...] > static inline int execute_only_pkey(struct mm_struct *mm) > { > + // Execute-only mappings are handled by EPAN/FEAT_PAN3. > + WARN_ON_ONCE(!cpus_have_final_cap(ARM64_HAS_EPAN)); > + > return -1; > } Why the WARN_ON_ONCE() here? It will trigger if the user asks for PROT_EXEC and I can't see any subsequent patch that changes the core code not to call it. I think we need some arch_has_execute_only_pkey() to avoid going on this path. Our arch would support exec-only with any pkey. > @@ -1490,6 +1491,38 @@ void ptep_modify_prot_commit(struct vm_area_struct *vma, unsigned long addr, pte > #ifdef CONFIG_ARCH_HAS_PKEYS > int arch_set_user_pkey_access(struct task_struct *tsk, int pkey, unsigned long init_val) > { > - return -ENOSPC; > + u64 new_por = POE_RXW; > + u64 old_por; > + u64 pkey_shift; > + > + if (!arch_pkeys_enabled()) > + return -ENOSPC; > + > + /* > + * This code should only be called with valid 'pkey' > + * values originating from in-kernel users. Complain > + * if a bad value is observed. > + */ > + if (WARN_ON_ONCE(pkey >= arch_max_pkey())) > + return -EINVAL; > + > + /* Set the bits we need in POR: */ > + if (init_val & PKEY_DISABLE_ACCESS) > + new_por = POE_X; Does PKEY_DISABLE_ACCESS mean allow execute? Or does x86 not have a way to disable execution? > + else if (init_val & PKEY_DISABLE_WRITE) > + new_por = POE_RX; > + > + /* Shift the bits in to the correct place in POR for pkey: */ > + pkey_shift = pkey * POR_BITS_PER_PKEY; > + new_por <<= pkey_shift; > + > + /* Get old POR and mask off any old bits in place: */ > + old_por = read_sysreg_s(SYS_POR_EL0); > + old_por &= ~(POE_MASK << pkey_shift); > + > + /* Write old part along with new part: */ > + write_sysreg_s(old_por | new_por, SYS_POR_EL0); > + > + return 0; > } > #endif -- Catalin