Re: [PATCH] arm64: fix fixmap copy for 16K pages and 48-bit VA

[Steve Capper <Steve.Capper@arm.com>]

On Tue, Aug 27, 2019 at 04:57:08PM +0100, Mark Rutland wrote:
> With 16K pages and 48-bit VAs, the PGD level of table has two entries,
> and so the fixmap shares a PGD with the kernel image. Since commit:
> 
>   f9040773b7bbbd9e ("arm64: move kernel image to base of vmalloc area")
> 
> ... we copy the existing fixmap to the new fine-grained page tables at
> the PUD level in this case. When walking to the new PUD, we forgot to
> offset the PGD entry and always used the PGD entry at index 0, but this
> worked as the kernel image and fixmap were in the low half of the TTBR1
> address space.
> 
> As of commit:
> 
>   14c127c957c1c607 ("arm64: mm: Flip kernel VA space")
> 
> ... the kernel image and fixmap are in the high half of the TTBR1
> address space, and hence use the PGD at index 1, but we didn't update
> the fixmap copying code to account for this.
> 
> Thus, we'll erroneously try to copy the fixmap slots into a PUD under
> the PGD entry at index 0. At the point we do so this PGD entry has not
> been initialised, and thus we'll try to write a value to a small offset
> from physical address 0, causing a number of potential problems.
> 
> Fix this be correctly offsetting the PGD. This is split over a few steps
> for legibility.
> 
> Fixes: 14c127c957c1c607 ("arm64: mm: Flip kernel VA space")
> Reported-by: Anshuman Khandual <anshuman.khandual@arm.com>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Steve Capper <Steve.Capper@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/mm/mmu.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 1d4247f9a496..4197f27f86e5 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -646,6 +646,8 @@ static void __init map_kernel(pgd_t *pgdp)
>  		set_pgd(pgd_offset_raw(pgdp, FIXADDR_START),
>  			READ_ONCE(*pgd_offset_k(FIXADDR_START)));
>  	} else if (CONFIG_PGTABLE_LEVELS > 3) {
> +		pgd_t *bm_pgdp;
> +		pud_t *bm_pudp;
>  		/*
>  		 * The fixmap shares its top level pgd entry with the kernel
>  		 * mapping. This can really only occur when we are running
> @@ -653,9 +655,9 @@ static void __init map_kernel(pgd_t *pgdp)
>  		 * entry instead.
>  		 */
>  		BUG_ON(!IS_ENABLED(CONFIG_ARM64_16K_PAGES));
> -		pud_populate(&init_mm,
> -			     pud_set_fixmap_offset(pgdp, FIXADDR_START),
> -			     lm_alias(bm_pmd));
> +		bm_pgdp = pgd_offset_raw(pgdp, FIXADDR_START);
> +		bm_pudp = pud_set_fixmap_offset(bm_pgdp, FIXADDR_START);
> +		pud_populate(&init_mm, bm_pudp, lm_alias(bm_pmd));
>  		pud_clear_fixmap();
>  	} else {
>  		BUG();

Thanks Mark,

FWIW:
Acked-by: Steve Capper <steve.capper@arm.com>
Tested-by: Steve Capper <steve.capper@arm.com>

Cheers,
-- 
Steve

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel