> Date: Sat, 19 Dec 2020 18:07:41 -0300
> From: Martin Pieuchot <[email protected]>
>
> On 18/12/20(Fri) 08:04, Todd C. Miller wrote:
> > On Fri, 18 Dec 2020 13:34:39 +0100, Mark Kettenis wrote:
> >
> > > Anyway, your analysis is right. When a kernel thread wants to use
> > > pmap_extract(9) on a userland pmap, it needs to lock pm_apte_mtx to
> > > prevent another thread from simultaniously activating a userland pmap
> > > too. So indeed, pm_apte_mtx needs to be properly initialized for the
> > > kernel pmap.
> > >
> > > However, pm_mtx should never be used for the kernel pmap. If we don't
> > > initialize the lock, witness will help us catching this condition, so
> > > maybe we shouldn't...
> >
> > I think a comment is warranted if we don't want to initialize the
> > lock to prevent someone from fixing this in the future ;-)
>
> A solution based on a comment and a non-enabled by option seems very
> fragile to me. I came up with the idea of poisoning the ipl of the
> mutex. What do you think?
Not sure if it makes sense to do this only for i386. And is
splraise() the right place for the check? Instead of mtx_enter()?
> Index: arch/i386/i386/machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.641
> diff -u -p -r1.641 machdep.c
> --- arch/i386/i386/machdep.c 8 Nov 2020 20:37:23 -0000 1.641
> +++ arch/i386/i386/machdep.c 19 Dec 2020 20:57:03 -0000
> @@ -3996,6 +3996,8 @@ splraise(int ncpl)
> {
> int ocpl;
>
> + KASSERT(ncpl >= IPL_NONE);
> +
> _SPLRAISE(ocpl, ncpl);
> return (ocpl);
> }
> Index: arch/i386/i386/pmap.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/pmap.c,v
> retrieving revision 1.209
> diff -u -p -r1.209 pmap.c
> --- arch/i386/i386/pmap.c 24 Sep 2020 11:36:50 -0000 1.209
> +++ arch/i386/i386/pmap.c 19 Dec 2020 20:58:48 -0000
> @@ -961,6 +961,8 @@ pmap_bootstrap(vaddr_t kva_start)
> */
>
> kpm = pmap_kernel();
> + mtx_init(&kpm->pm_mtx, -1); /* must not be used */
> + mtx_init(&kpm->pm_apte_mtx, IPL_VM);
> uvm_objinit(&kpm->pm_obj, NULL, 1);
> bzero(&kpm->pm_list, sizeof(kpm->pm_list)); /* pm_list not used */
> kpm->pm_pdir = (vaddr_t)(proc0.p_addr->u_pcb.pcb_cr3 + KERNBASE);
>
>
