Messages in this thread | | | Subject | Re: [PATCH v2] arm64/mm: avoid fixmap race condition when create pud mapping | From | Anshuman Khandual <> | Date | Mon, 13 Dec 2021 12:26:13 +0530 |
| |
On 12/10/21 3:24 PM, Jianyong Wu wrote: > fixmap is a global resource and is used recursively in create pud mapping. > It may lead to race condition when alloc_init_pud is called concurrently. > > Fox example: > alloc_init_pud is called when kernel_init. If memory hotplug > thread, which will also call alloc_init_pud, happens during > kernel_init, the race for fixmap occurs. > > The race condition flow can be: > > *************** begin ************** > > kerenl_init thread virtio-mem workqueue thread > ================== ======== ================== > alloc_init_pud(...) > pudp = pud_set_fixmap_offset(..) alloc_init_pud(...) > ... ... > READ_ONCE(*pudp) //OK! pudp = pud_set_fixmap_offset( > ... ... > pud_clear_fixmap() //fixmap break > READ_ONCE(*pudp) //CRASH! > > **************** end *************** > > Hence, a spin lock is introduced to protect the fixmap during create pdg > mapping. > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > --- > arch/arm64/mm/mmu.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c > index acfae9b41cc8..98ac09ae9588 100644 > --- a/arch/arm64/mm/mmu.c > +++ b/arch/arm64/mm/mmu.c > @@ -63,6 +63,7 @@ static pmd_t bm_pmd[PTRS_PER_PMD] __page_aligned_bss __maybe_unused; > static pud_t bm_pud[PTRS_PER_PUD] __page_aligned_bss __maybe_unused; > > static DEFINE_SPINLOCK(swapper_pgdir_lock); > +static DEFINE_SPINLOCK(fixmap_lock); > > void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd) > { > @@ -329,6 +330,11 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > } > BUG_ON(p4d_bad(p4d)); > > + /* > + * fixmap is global resource, thus it needs to be protected by a lock > + * in case of race condition. > + */ > + spin_lock(&fixmap_lock); > pudp = pud_set_fixmap_offset(p4dp, addr); > do { > pud_t old_pud = READ_ONCE(*pudp); > @@ -359,6 +365,7 @@ static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long end, > } while (pudp++, addr = next, addr != end); > > pud_clear_fixmap(); > + spin_unlock(&fixmap_lock); > } > > static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys, >
As the race could only happen with memory hotplug being enabled, could not we wrap this around with CONFIG_MEMORY_HOTPLUG, just to narrow its scope possibly speed up other non-hotplug cases ?
| |