| Message ID | 20190827155708.34699-1-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | b333b0ba2346f2071390533b8f37f36f191d705d |
| Headers | show |
| Series | arm64: fix fixmap copy for 16K pages and 48-bit VA | expand |
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.
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,
On 08/27/2019 09:27 PM, 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> Tested-by: Anshuman Khandual <anshuman.khandual@arm.com>
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();
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(-)