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

[Marc Zyngier <maz@kernel.org>]

On 2019-08-27 16:57, 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();

I've thrown this at a guest running on a TX1 box, and the guest
booted flawlessly, which is a major improvement.

Acked-by: Marc Zyngier <maz@kernel.org>
Tested-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...

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