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?
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);
