Messages in this thread | | | From | Jianyong Wu <> | Subject | RE: [PATCH v3] arm64/mm: avoid fixmap race condition when create pud mapping | Date | Mon, 31 Jan 2022 08:13:05 +0000 |
| |
Hi Catalin,
> -----Original Message----- > From: Catalin Marinas <catalin.marinas@arm.com> > Sent: Thursday, January 27, 2022 8:34 PM > To: David Hildenbrand <david@redhat.com> > Cc: Jianyong Wu <Jianyong.Wu@arm.com>; Ard Biesheuvel > <ardb@kernel.org>; Justin He <Justin.He@arm.com>; will@kernel.org; > Anshuman Khandual <Anshuman.Khandual@arm.com>; akpm@linux- > foundation.org; quic_qiancai@quicinc.com; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; gshan@redhat.com; nd <nd@arm.com> > Subject: Re: [PATCH v3] arm64/mm: avoid fixmap race condition when create > pud mapping > > On Thu, Jan 27, 2022 at 01:22:47PM +0100, David Hildenbrand wrote: > > > Yes, system_state can roughly separate these callers of > __create_pgd_mapping. When system_state > SYSTEM_BOOTING we can > add the lock. > > > Thus, I have the following change: > > > > > > static DEFINE_SPINLOCK(swapper_pgdir_lock); > > > +static DEFINE_MUTEX(fixmap_lock); > > > > > > void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd) { @@ -329,6 +330,8 > @@ > > > static void alloc_init_pud(pgd_t *pgdp, unsigned long addr, unsigned long > end, > > > } > > > BUG_ON(p4d_bad(p4d)); > > > > > > + if (system_state > SYSTEM_BOOTING) > > > > As there is nothing smaller than SYSTEM_BOOTING, you can use > > if (system_state != SYSTEM_BOOTING) > > > > ... > > > > > > > > It seems work and somehow simper. But I don't know if it is > > > reasonable to do this. So, any idea? @Ard Biesheuvel @Catalin > > > Marinas > > > > It's worth looking at kernel/notifier.c, e.g., > > blocking_notifier_chain_register() > > > > if (unlikely(system_state == SYSTEM_BOOTING)) > > return notifier_chain_register(&nh->head, n); > > > > down_write(&nh->rwsem); > > ret = notifier_chain_register(&nh->head, n); up_write(&nh->rwsem); > > > > If we decide to go down that path, we should make sure to add a > > comment like > > > > /* > > * No need for locking during early boot. And it doesn't work as > > * expected with KASLR enabled where we might clear BSS twice. > > */ > > A similar approach sounds fine to me. >
OK, I'll send the next version based on David's comments.
Thanks Jianyong
> -- > Catalin
| |