* [PATCH 0/2] arm64: use XN table mappings for the linear region @ 2021-03-04 17:11 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:11 ` [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region Ard Biesheuvel 0 siblings, 2 replies; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-04 17:11 UTC (permalink / raw) To: linux-arm-kernel Cc: Ard Biesheuvel, maz, catalin.marinas, will, mark.rutland, anshuman.khandual, qperret, kernel-team This patch tweaks the kernel page table population code to set the UXNTable and PXNTable bits on all table entries that cover the linear region. This removes the ability for lower level mappings to grant executable permissions which are never needed in the linear region. And given that swapper's PGD level is mapped r/o and can only be updated via the fixmap API, this cannot be trivially reverted by poking writable memory. This does not address a known exploit or vulnerability, but it applies the principle of least privilege in a way that does not result in any space or runtime overhead. Cc: maz@kernel.org Cc: catalin.marinas@arm.com Cc: will@kernel.org Cc: mark.rutland@arm.com Cc: anshuman.khandual@arm.com Cc: qperret@google.com Cc: kernel-team@android.com Ard Biesheuvel (2): arm64: mm: add missing P4D definitions and use them consistently arm64: mm: use XN table mapping attributes for the linear region arch/arm64/include/asm/pgtable-hwdef.h | 12 ++++++++ arch/arm64/mm/mmu.c | 31 ++++++++++++++------ 2 files changed, 34 insertions(+), 9 deletions(-) -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] arm64: mm: add missing P4D definitions and use them consistently 2021-03-04 17:11 [PATCH 0/2] arm64: use XN table mappings for the linear region Ard Biesheuvel @ 2021-03-04 17:11 ` Ard Biesheuvel 2021-03-04 17:39 ` Mark Rutland 2021-03-08 9:06 ` Anshuman Khandual 2021-03-04 17:11 ` [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region Ard Biesheuvel 1 sibling, 2 replies; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-04 17:11 UTC (permalink / raw) To: linux-arm-kernel Cc: Ard Biesheuvel, maz, catalin.marinas, will, mark.rutland, anshuman.khandual, qperret, kernel-team Even though level 0, 1 and 2 descriptors share the same attribute encodings, let's be a bit more consistent about using the right one at the right level. So add new macros for level 0/P4D definitions, and clean up some inconsistencies involving these macros. Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- arch/arm64/include/asm/pgtable-hwdef.h | 9 +++++++++ arch/arm64/mm/mmu.c | 6 +++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h index 42442a0ae2ab..e64e77a345b2 100644 --- a/arch/arm64/include/asm/pgtable-hwdef.h +++ b/arch/arm64/include/asm/pgtable-hwdef.h @@ -94,6 +94,15 @@ /* * Hardware page table definitions. * + * Level 0 descriptor (P4D). + */ +#define P4D_TYPE_TABLE (_AT(p4dval_t, 3) << 0) +#define P4D_TABLE_BIT (_AT(p4dval_t, 1) << 1) +#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] */ + +/* * Level 1 descriptor (PUD). */ #define PUD_TYPE_TABLE (_AT(pudval_t, 3) << 0) diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 3802cfbdd20d..029091474042 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -309,7 +309,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, phys_addr_t pud_phys; BUG_ON(!pgtable_alloc); pud_phys = pgtable_alloc(PUD_SHIFT); - __p4d_populate(p4dp, pud_phys, PUD_TYPE_TABLE); + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); p4d = READ_ONCE(*p4dp); } BUG_ON(p4d_bad(p4d)); @@ -1209,11 +1209,11 @@ void __init early_fixmap_init(void) pudp = pud_offset_kimg(p4dp, addr); } else { if (p4d_none(p4d)) - __p4d_populate(p4dp, __pa_symbol(bm_pud), PUD_TYPE_TABLE); + __p4d_populate(p4dp, __pa_symbol(bm_pud), P4D_TYPE_TABLE); pudp = fixmap_pud(addr); } if (pud_none(READ_ONCE(*pudp))) - __pud_populate(pudp, __pa_symbol(bm_pmd), PMD_TYPE_TABLE); + __pud_populate(pudp, __pa_symbol(bm_pmd), PUD_TYPE_TABLE); pmdp = fixmap_pmd(addr); __pmd_populate(pmdp, __pa_symbol(bm_pte), PMD_TYPE_TABLE); -- 2.20.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] arm64: mm: add missing P4D definitions and use them consistently 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 1 sibling, 0 replies; 11+ messages in thread From: Mark Rutland @ 2021-03-04 17:39 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-arm-kernel, maz, catalin.marinas, will, anshuman.khandual, qperret, kernel-team On Thu, Mar 04, 2021 at 06:11:44PM +0100, Ard Biesheuvel wrote: > Even though level 0, 1 and 2 descriptors share the same attribute > encodings, let's be a bit more consistent about using the right one at > the right level. So add new macros for level 0/P4D definitions, and > clean up some inconsistencies involving these macros. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Acked-by: Mark Rutland <mark.rutland@arm.com> > --- > arch/arm64/include/asm/pgtable-hwdef.h | 9 +++++++++ > arch/arm64/mm/mmu.c | 6 +++--- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > index 42442a0ae2ab..e64e77a345b2 100644 > --- a/arch/arm64/include/asm/pgtable-hwdef.h > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > @@ -94,6 +94,15 @@ > /* > * Hardware page table definitions. > * > + * Level 0 descriptor (P4D). > + */ > +#define P4D_TYPE_TABLE (_AT(p4dval_t, 3) << 0) > +#define P4D_TABLE_BIT (_AT(p4dval_t, 1) << 1) > +#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] */ > + > +/* > * Level 1 descriptor (PUD). > */ > #define PUD_TYPE_TABLE (_AT(pudval_t, 3) << 0) > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index 3802cfbdd20d..029091474042 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -309,7 +309,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > phys_addr_t pud_phys; > BUG_ON(!pgtable_alloc); > pud_phys = pgtable_alloc(PUD_SHIFT); > - __p4d_populate(p4dp, pud_phys, PUD_TYPE_TABLE); > + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); > p4d = READ_ONCE(*p4dp); > } > BUG_ON(p4d_bad(p4d)); > @@ -1209,11 +1209,11 @@ void __init early_fixmap_init(void) > pudp = pud_offset_kimg(p4dp, addr); > } else { > if (p4d_none(p4d)) > - __p4d_populate(p4dp, __pa_symbol(bm_pud), PUD_TYPE_TABLE); > + __p4d_populate(p4dp, __pa_symbol(bm_pud), P4D_TYPE_TABLE); > pudp = fixmap_pud(addr); > } > if (pud_none(READ_ONCE(*pudp))) > - __pud_populate(pudp, __pa_symbol(bm_pmd), PMD_TYPE_TABLE); > + __pud_populate(pudp, __pa_symbol(bm_pmd), PUD_TYPE_TABLE); > pmdp = fixmap_pmd(addr); > __pmd_populate(pmdp, __pa_symbol(bm_pte), PMD_TYPE_TABLE); > > -- > 2.20.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] arm64: mm: add missing P4D definitions and use them consistently 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 1 sibling, 1 reply; 11+ messages in thread From: Anshuman Khandual @ 2021-03-08 9:06 UTC (permalink / raw) To: Ard Biesheuvel, linux-arm-kernel Cc: maz, catalin.marinas, will, mark.rutland, qperret, kernel-team On 3/4/21 10:41 PM, Ard Biesheuvel wrote: > Even though level 0, 1 and 2 descriptors share the same attribute > encodings, let's be a bit more consistent about using the right one at > the right level. So add new macros for level 0/P4D definitions, and > clean up some inconsistencies involving these macros. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/arm64/include/asm/pgtable-hwdef.h | 9 +++++++++ > arch/arm64/mm/mmu.c | 6 +++--- > 2 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > index 42442a0ae2ab..e64e77a345b2 100644 > --- a/arch/arm64/include/asm/pgtable-hwdef.h > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > @@ -94,6 +94,15 @@ > /* > * Hardware page table definitions. > * > + * Level 0 descriptor (P4D). > + */ > +#define P4D_TYPE_TABLE (_AT(p4dval_t, 3) << 0) > +#define P4D_TABLE_BIT (_AT(p4dval_t, 1) << 1) > +#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] */ > + > +/* > * Level 1 descriptor (PUD). > */ > #define PUD_TYPE_TABLE (_AT(pudval_t, 3) << 0) > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index 3802cfbdd20d..029091474042 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -309,7 +309,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > phys_addr_t pud_phys; > BUG_ON(!pgtable_alloc); > pud_phys = pgtable_alloc(PUD_SHIFT); > - __p4d_populate(p4dp, pud_phys, PUD_TYPE_TABLE); > + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); > p4d = READ_ONCE(*p4dp); > } > BUG_ON(p4d_bad(p4d)); > @@ -1209,11 +1209,11 @@ void __init early_fixmap_init(void) > pudp = pud_offset_kimg(p4dp, addr); > } else { > if (p4d_none(p4d)) > - __p4d_populate(p4dp, __pa_symbol(bm_pud), PUD_TYPE_TABLE); > + __p4d_populate(p4dp, __pa_symbol(bm_pud), P4D_TYPE_TABLE); > pudp = fixmap_pud(addr); > } > if (pud_none(READ_ONCE(*pudp))) > - __pud_populate(pudp, __pa_symbol(bm_pmd), PMD_TYPE_TABLE); > + __pud_populate(pudp, __pa_symbol(bm_pmd), PUD_TYPE_TABLE); > pmdp = fixmap_pmd(addr); > __pmd_populate(pmdp, __pa_symbol(bm_pte), PMD_TYPE_TABLE); > > I guess there are still some more similar inconsistencies around. diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h index 3c6a7f5988b1..27cc643d0509 100644 --- a/arch/arm64/include/asm/pgalloc.h +++ b/arch/arm64/include/asm/pgalloc.h @@ -27,7 +27,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot) static inline void pud_populate(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp) { - __pud_populate(pudp, __pa(pmdp), PMD_TYPE_TABLE); + __pud_populate(pudp, __pa(pmdp), PUD_TYPE_TABLE); } #else static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot) @@ -45,7 +45,7 @@ static inline void __p4d_populate(p4d_t *p4dp, phys_addr_t pudp, p4dval_t prot) static inline void p4d_populate(struct mm_struct *mm, p4d_t *p4dp, pud_t *pudp) { - __p4d_populate(p4dp, __pa(pudp), PUD_TYPE_TABLE); + __p4d_populate(p4dp, __pa(pudp), P4D_TYPE_TABLE); } #else static inline void __p4d_populate(p4d_t *p4dp, phys_addr_t pudp, p4dval_t prot) diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c index d8e66c78440e..9fe40cbbd8c0 100644 --- a/arch/arm64/mm/kasan_init.c +++ b/arch/arm64/mm/kasan_init.c @@ -79,7 +79,7 @@ static pmd_t *__init kasan_pmd_offset(pud_t *pudp, unsigned long addr, int node, phys_addr_t pmd_phys = early ? __pa_symbol(kasan_early_shadow_pmd) : kasan_alloc_zeroed_page(node); - __pud_populate(pudp, pmd_phys, PMD_TYPE_TABLE); + __pud_populate(pudp, pmd_phys, PUD_TYPE_TABLE); } return early ? pmd_offset_kimg(pudp, addr) : pmd_offset(pudp, addr); @@ -92,7 +92,7 @@ static pud_t *__init kasan_pud_offset(p4d_t *p4dp, unsigned long addr, int node, phys_addr_t pud_phys = early ? __pa_symbol(kasan_early_shadow_pud) : kasan_alloc_zeroed_page(node); - __p4d_populate(p4dp, pud_phys, PMD_TYPE_TABLE); + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); } return early ? pud_offset_kimg(p4dp, addr) : pud_offset(p4dp, addr); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] arm64: mm: add missing P4D definitions and use them consistently 2021-03-08 9:06 ` Anshuman Khandual @ 2021-03-08 9:07 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-08 9:07 UTC (permalink / raw) To: Anshuman Khandual Cc: Linux ARM, Marc Zyngier, Catalin Marinas, Will Deacon, Mark Rutland, Quentin Perret, Android Kernel Team On Mon, 8 Mar 2021 at 10:06, Anshuman Khandual <anshuman.khandual@arm.com> wrote: > > > > On 3/4/21 10:41 PM, Ard Biesheuvel wrote: > > Even though level 0, 1 and 2 descriptors share the same attribute > > encodings, let's be a bit more consistent about using the right one at > > the right level. So add new macros for level 0/P4D definitions, and > > clean up some inconsistencies involving these macros. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > arch/arm64/include/asm/pgtable-hwdef.h | 9 +++++++++ > > arch/arm64/mm/mmu.c | 6 +++--- > > 2 files changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > > index 42442a0ae2ab..e64e77a345b2 100644 > > --- a/arch/arm64/include/asm/pgtable-hwdef.h > > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > > @@ -94,6 +94,15 @@ > > /* > > * Hardware page table definitions. > > * > > + * Level 0 descriptor (P4D). > > + */ > > +#define P4D_TYPE_TABLE (_AT(p4dval_t, 3) << 0) > > +#define P4D_TABLE_BIT (_AT(p4dval_t, 1) << 1) > > +#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] */ > > + > > +/* > > * Level 1 descriptor (PUD). > > */ > > #define PUD_TYPE_TABLE (_AT(pudval_t, 3) << 0) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > > index 3802cfbdd20d..029091474042 100644 > > --- a/arch/arm64/mm/mmu.c > > +++ b/arch/arm64/mm/mmu.c > > @@ -309,7 +309,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > > phys_addr_t pud_phys; > > BUG_ON(!pgtable_alloc); > > pud_phys = pgtable_alloc(PUD_SHIFT); > > - __p4d_populate(p4dp, pud_phys, PUD_TYPE_TABLE); > > + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); > > p4d = READ_ONCE(*p4dp); > > } > > BUG_ON(p4d_bad(p4d)); > > @@ -1209,11 +1209,11 @@ void __init early_fixmap_init(void) > > pudp = pud_offset_kimg(p4dp, addr); > > } else { > > if (p4d_none(p4d)) > > - __p4d_populate(p4dp, __pa_symbol(bm_pud), PUD_TYPE_TABLE); > > + __p4d_populate(p4dp, __pa_symbol(bm_pud), P4D_TYPE_TABLE); > > pudp = fixmap_pud(addr); > > } > > if (pud_none(READ_ONCE(*pudp))) > > - __pud_populate(pudp, __pa_symbol(bm_pmd), PMD_TYPE_TABLE); > > + __pud_populate(pudp, __pa_symbol(bm_pmd), PUD_TYPE_TABLE); > > pmdp = fixmap_pmd(addr); > > __pmd_populate(pmdp, __pa_symbol(bm_pte), PMD_TYPE_TABLE); > > > > > > I guess there are still some more similar inconsistencies around. > Indeed - I will fix those as well in v2. Thanks, Ard. > diff --git a/arch/arm64/include/asm/pgalloc.h b/arch/arm64/include/asm/pgalloc.h > index 3c6a7f5988b1..27cc643d0509 100644 > --- a/arch/arm64/include/asm/pgalloc.h > +++ b/arch/arm64/include/asm/pgalloc.h > @@ -27,7 +27,7 @@ static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot) > > static inline void pud_populate(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp) > { > - __pud_populate(pudp, __pa(pmdp), PMD_TYPE_TABLE); > + __pud_populate(pudp, __pa(pmdp), PUD_TYPE_TABLE); > } > #else > static inline void __pud_populate(pud_t *pudp, phys_addr_t pmdp, pudval_t prot) > @@ -45,7 +45,7 @@ static inline void __p4d_populate(p4d_t *p4dp, phys_addr_t pudp, p4dval_t prot) > > static inline void p4d_populate(struct mm_struct *mm, p4d_t *p4dp, pud_t *pudp) > { > - __p4d_populate(p4dp, __pa(pudp), PUD_TYPE_TABLE); > + __p4d_populate(p4dp, __pa(pudp), P4D_TYPE_TABLE); > } > #else > static inline void __p4d_populate(p4d_t *p4dp, phys_addr_t pudp, p4dval_t prot) > diff --git a/arch/arm64/mm/kasan_init.c b/arch/arm64/mm/kasan_init.c > index d8e66c78440e..9fe40cbbd8c0 100644 > --- a/arch/arm64/mm/kasan_init.c > +++ b/arch/arm64/mm/kasan_init.c > @@ -79,7 +79,7 @@ static pmd_t *__init kasan_pmd_offset(pud_t *pudp, unsigned long addr, int node, > phys_addr_t pmd_phys = early ? > __pa_symbol(kasan_early_shadow_pmd) > : kasan_alloc_zeroed_page(node); > - __pud_populate(pudp, pmd_phys, PMD_TYPE_TABLE); > + __pud_populate(pudp, pmd_phys, PUD_TYPE_TABLE); > } > > return early ? pmd_offset_kimg(pudp, addr) : pmd_offset(pudp, addr); > @@ -92,7 +92,7 @@ static pud_t *__init kasan_pud_offset(p4d_t *p4dp, unsigned long addr, int node, > phys_addr_t pud_phys = early ? > __pa_symbol(kasan_early_shadow_pud) > : kasan_alloc_zeroed_page(node); > - __p4d_populate(p4dp, pud_phys, PMD_TYPE_TABLE); > + __p4d_populate(p4dp, pud_phys, P4D_TYPE_TABLE); > } > > return early ? pud_offset_kimg(p4dp, addr) : pud_offset(p4dp, addr); _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 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:11 ` Ard Biesheuvel 2021-03-04 17:39 ` Mark Rutland 2021-03-05 19:06 ` Catalin Marinas 1 sibling, 2 replies; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-04 17:11 UTC (permalink / raw) To: linux-arm-kernel Cc: Ard Biesheuvel, maz, catalin.marinas, will, mark.rutland, anshuman.khandual, qperret, kernel-team 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 */ /* * 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 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 2021-03-05 8:13 ` Ard Biesheuvel 2021-03-05 19:06 ` Catalin Marinas 1 sibling, 1 reply; 11+ messages in thread From: Mark Rutland @ 2021-03-04 17:39 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-arm-kernel, maz, catalin.marinas, will, anshuman.khandual, qperret, kernel-team 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 2021-03-04 17:39 ` Mark Rutland @ 2021-03-05 8:13 ` Ard Biesheuvel 0 siblings, 0 replies; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-05 8:13 UTC (permalink / raw) To: Mark Rutland Cc: Linux ARM, Marc Zyngier, Catalin Marinas, Will Deacon, Anshuman Khandual, Quentin Perret, Android Kernel Team On Thu, 4 Mar 2021 at 18:39, Mark Rutland <mark.rutland@arm.com> wrote: > > 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> > Yeah, it's probably better to split them right away. > > > > /* > > * 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 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 @ 2021-03-05 19:06 ` Catalin Marinas 2021-03-05 19:17 ` Ard Biesheuvel 1 sibling, 1 reply; 11+ messages in thread From: Catalin Marinas @ 2021-03-05 19:06 UTC (permalink / raw) To: Ard Biesheuvel Cc: linux-arm-kernel, maz, will, mark.rutland, anshuman.khandual, qperret, kernel-team 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. In ARMv8.1 the architecture added the possibility of disabling the hierarchical page table permissions (FEAT_HPDS) so that we can use these bits for software. Is there any big advantage to using the hierarchical permissions vs some sanity check in set_pte() for example? -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 2021-03-05 19:06 ` Catalin Marinas @ 2021-03-05 19:17 ` Ard Biesheuvel 2021-03-05 20:37 ` Catalin Marinas 0 siblings, 1 reply; 11+ messages in thread From: Ard Biesheuvel @ 2021-03-05 19:17 UTC (permalink / raw) To: Catalin Marinas Cc: Linux ARM, Marc Zyngier, Will Deacon, Mark Rutland, Anshuman Khandual, Quentin Perret, Android Kernel Team On Fri, 5 Mar 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote: > > 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. > > In ARMv8.1 the architecture added the possibility of disabling the > hierarchical page table permissions (FEAT_HPDS) so that we can use these > bits for software. > Sure, but I don't think there is a shortage of software bits in table descriptors, right? And we don't enable the feature in the first place. > Is there any big advantage to using the hierarchical permissions vs > some sanity check in set_pte() for example? > There is a big advantage: the fact that the permissions are both hierarchical and subtractive. Sanity checks in set_pte() only cover page mappings that were created in the correct way. But that does not help us if an attacker manages a single 64-bit write that creates a page or table entry pointing to a page under their control. Taking away the exec permissions at the levels above makes it much more difficult to carry out such an attack, especially given that the root level is not mapped read-write to begin with. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] arm64: mm: use XN table mapping attributes for the linear region 2021-03-05 19:17 ` Ard Biesheuvel @ 2021-03-05 20:37 ` Catalin Marinas 0 siblings, 0 replies; 11+ messages in thread From: Catalin Marinas @ 2021-03-05 20:37 UTC (permalink / raw) To: Ard Biesheuvel Cc: Linux ARM, Marc Zyngier, Will Deacon, Mark Rutland, Anshuman Khandual, Quentin Perret, Android Kernel Team On Fri, Mar 05, 2021 at 08:17:07PM +0100, Ard Biesheuvel wrote: > On Fri, 5 Mar 2021 at 20:06, Catalin Marinas <catalin.marinas@arm.com> wrote: > > 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. > > > > In ARMv8.1 the architecture added the possibility of disabling the > > hierarchical page table permissions (FEAT_HPDS) so that we can use these > > bits for software. > > Sure, but I don't think there is a shortage of software bits in table > descriptors, right? And we don't enable the feature in the first > place. We are short of software bits but in the *pte*, so disabling the hierarchical permissions doesn't anyway help. So, ignore me, the patches are fine ;). -- Catalin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-03-08 9:08 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).