Messages in this thread | | | Date | Thu, 16 Dec 2021 16:19:30 +0100 | Subject | Re: [PATCH v3] arm64/mm: avoid fixmap race condition when create pud mapping | From | David Hildenbrand <> |
| |
On 16.12.21 09:28, Jianyong Wu wrote: > The 'fixmap' is a global resource and is used recursively by > create pud mapping(), leading to a potential race condition in the > presence of a concurrent call to alloc_init_pud(): > > kernel_init thread virtio-mem workqueue thread > ================== =========================== > > alloc_init_pud(...) alloc_init_pud(...) > pudp = pud_set_fixmap_offset(...) pudp = pud_set_fixmap_offset(...) > READ_ONCE(*pudp) > pud_clear_fixmap(...) > READ_ONCE(*pudp) // CRASH! > > As kernel may sleep during creating pud mapping, introduce a mutex lock to > serialise use of the fixmap entries by alloc_init_pud(). > > Signed-off-by: Jianyong Wu <jianyong.wu@arm.com> > --- > > Change log: > > from v2 to v3: > change spin lock to mutex lock as kernel may sleep when create pud > map. > > 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..e680a6a8ca40 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_MUTEX(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)); > > + /* > + * We only have one fixmap entry per page-table level, so take > + * the fixmap lock until we're done. > + */ > + mutex_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(); > + mutex_unlock(&fixmap_lock); > } > > static void __create_pgd_mapping(pgd_t *pgdir, phys_addr_t phys, >
Hopefully now we get it right and there are not any "special" callpaths :)
Acked-by: David Hildenbrand <david@redhat.com>
-- Thanks,
David / dhildenb
| |