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 X-Spam-Level: X-Spam-Status: No, score=-14.3 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B8A6BC433DB for ; Tue, 9 Mar 2021 12:38:01 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 4BD466523F for ; Tue, 9 Mar 2021 12:38:01 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 4BD466523F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=kzPb+ZFoqimvQmtLVw+kef7ZfXRfiCNGGcXXWNFJWnQ=; b=jfPFyoraZhHk4oxWl5oIjKFtP OcHYWi3Bv3o/FgBOB3RqYr2JuDcwELIkZHA1H/6ntv+FrnG27ed9DNzUrSmywluC5BtPFpgEZW/3R Seh3Hj4digEdW2kBbT3qS87zpLtsHKiV9zhZG2qJ15TE327ZC6z/E7LhlOMISa5TZaVnNFR1I3pds Fh7UN8exQze3PL4N8FPVVbZWfoXGhUpuKuMzbLvvhYWmCweEJ9brSntaBrYxG7Hli67gZXEmkhkQA rI7c0PtDZDdbOG14/QXr4jJEZNtDJsEshDcXQAc9T0r9vXOJGK+zHk4/gMQ5Ydzsnw1MnyJzNyScA 2wgvirtGg==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lJbbH-004TFn-FQ; Tue, 09 Mar 2021 12:36:35 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lJbbA-004TF4-6s for linux-arm-kernel@lists.infradead.org; Tue, 09 Mar 2021 12:36:30 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 967536523F for ; Tue, 9 Mar 2021 12:36:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1615293386; bh=Q3RRpnXNKWQ59zK2dWVBO8SEYUDTLT/Z7MeLBCPy/6U=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=pg9sikwmwbSbom0ZMKb5OPSNN17b61wzNPTfHQD4Oht3CpGSXLAvTsTDSRCDaH0uA oHrTkHwf0f7/bnUURwqrekxjQjTVMemwZ7MZ1r0D127oMnLS/EVeNRUltae4dE34Cs knd9haipBZcJpRu/VA3Qdmh/Uke0vcqD2L8o5bvN7+d7ZQLkdn/wSFSjoUs8qqxjyh ZatGL8Fd2LYDkPYJya80DlmAunHivRTDKlfiQyw/DVJscX8TnROThkkEmna6mE6NQB 9kKJjcdmTA8uv/nO/SvaU+w2xWtcxsA5/qK8ftlpFbrVMhzKScpRcfY5/IPzRge6DU ZGQNX+orhB5pg== Received: by mail-oi1-f173.google.com with SMTP id u198so9787031oia.4 for ; Tue, 09 Mar 2021 04:36:26 -0800 (PST) X-Gm-Message-State: AOAM532Ah7F+XM2gEYlDyh3R2ffXXps3IsFUK2dDdYonxChhU3q3Te0W 4Qj+YIN9f1mYS+q+zKOd2Zy1XYBKFKtoaQNk3sQ= X-Google-Smtp-Source: ABdhPJyB9SyEQ5RV/aicogvbESGc8zDoCeefEpwW63mD67SN41bDT5jf2JfJjmtrj/ZlDP0GtgmaHJUKn7uMg5Eyrd8= X-Received: by 2002:aca:ab86:: with SMTP id u128mr2792751oie.47.1615293385824; Tue, 09 Mar 2021 04:36:25 -0800 (PST) MIME-Version: 1.0 References: <20210308181535.16230-1-ardb@kernel.org> <20210308181535.16230-3-ardb@kernel.org> <4e671a40-2220-9059-06ff-945cae1d5af0@arm.com> In-Reply-To: <4e671a40-2220-9059-06ff-945cae1d5af0@arm.com> From: Ard Biesheuvel Date: Tue, 9 Mar 2021 13:36:14 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 2/3] arm64: mm: use XN table mapping attributes for the linear region To: Anshuman Khandual Cc: Linux ARM , Marc Zyngier , Catalin Marinas , Will Deacon , Mark Rutland , Quentin Perret , Android Kernel Team X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210309_123628_651395_2BF027FE X-CRM114-Status: GOOD ( 38.20 ) 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 On Tue, 9 Mar 2021 at 06:08, Anshuman Khandual wrote: > > > > On 3/8/21 11:45 PM, Ard Biesheuvel wrote: > > The way the arm64 kernel virtual address space is constructed guarantees > > that swapper PGD entries are never shared between the linear region on > > the one hand, and the vmalloc region on the other, which is where all > > kernel text, module text and BPF text mappings reside. > > While here, wondering if it would be better to validate this assumption > via a BUILD_BUG_ON() or something similar ? > Sure. Something like BUILD_BUG_ON(pgd_index(_PAGE_END(VA_BITS_MIN) - 1) == pgd_index(_PAGE_END(VA_BITS_MIN))); perhaps? > > > > This means that mappings in the linear region (which never require > > executable permissions) never share any table entries at any level with > > mappings that do require executable permissions, and so we can set the > > table-level PXN attributes for all table entries that are created while > > setting up mappings in the linear region. Since swapper's PGD level page > > table is mapped r/o itself, this adds another layer of robustness to the > > way the kernel manages its own page tables. While at it, set the UXN > > attribute as well for all kernel mappings created at boot. > > > What happens when FEAT_HPDS is implemented and also being enabled ? Would > there be any adverse affect here or at least break the assumption that the > linear mapping page table entries are safer than before ? Hence, it might > be still better to check FEAT_HPDS feature enablement here, even it it is > not being used right now. I would assume that enabling HPDS is only done to free up those bits for some other purpose, and until that time, I don't think we need to worry about this, given that the values are just going to be ignored otherwise. > > Signed-off-by: Ard Biesheuvel > > Acked-by: Mark Rutland > > --- > > arch/arm64/include/asm/pgtable-hwdef.h | 6 +++++ > > arch/arm64/mm/mmu.c | 27 +++++++++++++++----- > > 2 files changed, 26 insertions(+), 7 deletions(-) > > > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > > index e64e77a345b2..b82575a33f8b 100644 > > --- a/arch/arm64/include/asm/pgtable-hwdef.h > > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > > @@ -101,6 +101,8 @@ > > #define P4D_TYPE_MASK (_AT(p4dval_t, 3) << 0) > > #define P4D_TYPE_SECT (_AT(p4dval_t, 1) << 0) > > #define P4D_SECT_RDONLY (_AT(p4dval_t, 1) << 7) /* AP[2] */ > > +#define P4D_TABLE_PXN (_AT(p4dval_t, 1) << 59) > > +#define P4D_TABLE_UXN (_AT(p4dval_t, 1) << 60) > > > > /* > > * Level 1 descriptor (PUD). > > @@ -110,6 +112,8 @@ > > #define PUD_TYPE_MASK (_AT(pudval_t, 3) << 0) > > #define PUD_TYPE_SECT (_AT(pudval_t, 1) << 0) > > #define PUD_SECT_RDONLY (_AT(pudval_t, 1) << 7) /* AP[2] */ > > +#define PUD_TABLE_PXN (_AT(pudval_t, 1) << 59) > > +#define PUD_TABLE_UXN (_AT(pudval_t, 1) << 60) > > > > /* > > * Level 2 descriptor (PMD). > > @@ -131,6 +135,8 @@ > > #define PMD_SECT_CONT (_AT(pmdval_t, 1) << 52) > > #define PMD_SECT_PXN (_AT(pmdval_t, 1) << 53) > > #define PMD_SECT_UXN (_AT(pmdval_t, 1) << 54) > > +#define PMD_TABLE_PXN (_AT(pmdval_t, 1) << 59) > > +#define PMD_TABLE_UXN (_AT(pmdval_t, 1) << 60) > > > > /* > > * AttrIndx[2:0] encoding (mapping attributes defined in the MAIR* registers). > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > > index 029091474042..9de59fce0450 100644 > > --- a/arch/arm64/mm/mmu.c > > +++ b/arch/arm64/mm/mmu.c > > @@ -39,6 +39,7 @@ > > > > #define NO_BLOCK_MAPPINGS BIT(0) > > #define NO_CONT_MAPPINGS BIT(1) > > +#define NO_EXEC_MAPPINGS BIT(2) > > NO_EXEC_MAPPINGS flag basically selects PXX_TABLE_PXN because PXX_TABLE_UXN > is now a default, which is already included as a protection flag. Should not > this be renamed as NO_KERNEL_EXEC_MAPPINGS matching what PXX_TABLE_PXN is > going to achieve. > I don't think renaming this flag is going to make things more clear tbh. > > > > u64 idmap_t0sz = TCR_T0SZ(VA_BITS); > > u64 idmap_ptrs_per_pgd = PTRS_PER_PGD; > > @@ -185,10 +186,14 @@ static void alloc_init_cont_pte(pmd_t *pmdp, unsigned long addr, > > > > BUG_ON(pmd_sect(pmd)); > > if (pmd_none(pmd)) { > > + pmdval_t pmdval = PMD_TYPE_TABLE | PMD_TABLE_UXN; > > phys_addr_t pte_phys; > > + > > + if (flags & NO_EXEC_MAPPINGS) > > + pmdval |= PMD_TABLE_PXN; > > BUG_ON(!pgtable_alloc); > > pte_phys = pgtable_alloc(PAGE_SHIFT); > > - __pmd_populate(pmdp, pte_phys, PMD_TYPE_TABLE); > > + __pmd_populate(pmdp, pte_phys, pmdval); > > pmd = READ_ONCE(*pmdp); > > } > > BUG_ON(pmd_bad(pmd)); > > @@ -259,10 +264,14 @@ static void alloc_init_cont_pmd(pud_t *pudp, unsigned long addr, > > */ > > BUG_ON(pud_sect(pud)); > > if (pud_none(pud)) { > > + pudval_t pudval = PUD_TYPE_TABLE | PUD_TABLE_UXN; > > phys_addr_t pmd_phys; > > + > > + if (flags & NO_EXEC_MAPPINGS) > > + pudval |= PUD_TABLE_PXN; > > BUG_ON(!pgtable_alloc); > > pmd_phys = pgtable_alloc(PMD_SHIFT); > > - __pud_populate(pudp, pmd_phys, PUD_TYPE_TABLE); > > + __pud_populate(pudp, pmd_phys, pudval); > > pud = READ_ONCE(*pudp); > > } > > BUG_ON(pud_bad(pud)); > > @@ -306,10 +315,14 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > > p4d_t p4d = READ_ONCE(*p4dp); > > > > if (p4d_none(p4d)) { > > + p4dval_t p4dval = P4D_TYPE_TABLE | P4D_TABLE_UXN; > > phys_addr_t pud_phys; > > + > > + if (flags & NO_EXEC_MAPPINGS) > > + p4dval |= P4D_TABLE_PXN; > > BUG_ON(!pgtable_alloc); > > pud_phys = pgtable_alloc(PUD_SHIFT); > > - __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); > > + __p4d_populate(p4dp, pud_phys, p4dval); > > p4d = READ_ONCE(*p4dp); > > } > > BUG_ON(p4d_bad(p4d)); > > @@ -489,11 +502,11 @@ static void __init map_mem(pgd_t *pgdp) > > phys_addr_t kernel_start = __pa_symbol(_stext); > > phys_addr_t kernel_end = __pa_symbol(__init_begin); > > phys_addr_t start, end; > > - int flags = 0; > > + int flags = NO_EXEC_MAPPINGS; > > u64 i; > > > > if (rodata_full || crash_mem_map || debug_pagealloc_enabled()) > > - flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS; > > + flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS; > > > > /* > > * Take care not to create a writable alias for the > > @@ -1462,7 +1475,7 @@ struct range arch_get_mappable_range(void) > > int arch_add_memory(int nid, u64 start, u64 size, > > struct mhp_params *params) > > { > > - int ret, flags = 0; > > + int ret, flags = NO_EXEC_MAPPINGS; > > > > VM_BUG_ON(!mhp_range_allowed(start, size, true)); > > > > @@ -1472,7 +1485,7 @@ int arch_add_memory(int nid, u64 start, u64 size, > > */ > > if (rodata_full || debug_pagealloc_enabled() || > > IS_ENABLED(CONFIG_KFENCE)) > > - flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS; > > + flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS; > > > > __create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start), > > size, params->pgprot, __pgd_pgtable_alloc, > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel