From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, maz@kernel.org,
catalin.marinas@arm.com, will@kernel.org,
anshuman.khandual@arm.com, qperret@google.com,
kernel-team@android.com
Subject: Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region
Date: Thu, 4 Mar 2021 17:39:03 +0000 [thread overview]
Message-ID: <20210304173903.GB60457@C02TD0UTHF1T.local> (raw)
In-Reply-To: <20210304171145.12281-3-ardb@kernel.org>
On Thu, Mar 04, 2021 at 06:11:45PM +0100, 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.
>
> 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/UXN 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.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm64/include/asm/pgtable-hwdef.h | 3 +++
> arch/arm64/mm/mmu.c | 27 +++++++++++++++-----
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
> index e64e77a345b2..b6f50abd63d7 100644
> --- a/arch/arm64/include/asm/pgtable-hwdef.h
> +++ b/arch/arm64/include/asm/pgtable-hwdef.h
> @@ -101,6 +101,7 @@
> #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_XN (_AT(p4dval_t, 3) << 59) /* UXN:PXN */
I reckon it's worth having separate P?D_TABLE_UXN and P?D_TABLE_PXN
definitions, so that it's easier/possible to:
* Set UXNTable for *all* kernel mappings
* Set PXNTable for *all* user mappings
... though we don't need to do either immediately.
Otherwise, this looks good to me, and with or without that split:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
>
> /*
> * Level 1 descriptor (PUD).
> @@ -110,6 +111,7 @@
> #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_XN (_AT(pudval_t, 3) << 59) /* UXN:PXN */
>
> /*
> * Level 2 descriptor (PMD).
> @@ -131,6 +133,7 @@
> #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_XN (_AT(pmdval_t, 3) << 59) /* UXN:PXN */
>
> /*
> * 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..ff5cbca403be 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)
>
> 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;
> phys_addr_t pte_phys;
> +
> + if (flags & NO_EXEC_MAPPINGS)
> + pmdval |= PMD_TABLE_XN;
> 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;
> phys_addr_t pmd_phys;
> +
> + if (flags & NO_EXEC_MAPPINGS)
> + pudval |= PUD_TABLE_XN;
> 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;
> phys_addr_t pud_phys;
> +
> + if (flags & NO_EXEC_MAPPINGS)
> + p4dval |= P4D_TABLE_XN;
> 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,
> --
> 2.20.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-03-04 17:41 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-04 17:11 [PATCH 0/2] arm64: use XN table mappings for the linear region Ard Biesheuvel
2021-03-04 17:11 ` [PATCH 1/2] arm64: mm: add missing P4D definitions and use them consistently Ard Biesheuvel
2021-03-04 17:39 ` Mark Rutland
2021-03-08 9:06 ` Anshuman Khandual
2021-03-08 9:07 ` Ard Biesheuvel
2021-03-04 17:11 ` [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region Ard Biesheuvel
2021-03-04 17:39 ` Mark Rutland [this message]
2021-03-05 8:13 ` Ard Biesheuvel
2021-03-05 19:06 ` Catalin Marinas
2021-03-05 19:17 ` Ard Biesheuvel
2021-03-05 20:37 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210304173903.GB60457@C02TD0UTHF1T.local \
--to=mark.rutland@arm.com \
--cc=anshuman.khandual@arm.com \
--cc=ardb@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=kernel-team@android.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
--cc=qperret@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.