On Mon, Sep 14, 2020 at 01:25:03PM +0200, Mark Kettenis wrote: > > Date: Sun, 13 Sep 2020 19:48:19 +0200 > > From: Sebastien Marie <sema...@online.fr> > > > > On Sun, Sep 13, 2020 at 04:49:48PM +0200, Sebastien Marie wrote: > > > On Sun, Sep 13, 2020 at 03:29:57PM +0200, Martin Pieuchot wrote: > > > > I'm no longer able to reproduce the corruption while building lang/go > > > > with the diff below. Something relevant to threading change in go since > > > > march? > > > > > > > > Can someone try this diff and tell me if go and/or rust still fail? > > > > > > quickly tested with rustc build (nightly here), and it is failing at > > > random places (not always at the same) with memory errors (signal 11, > > > compiler ICE signal 6...) > > > > > > > A first hint. > > > > With the help of deraadt@, it was found that disabling > > uvm_map_inentry() call in usertrap() is enough to avoid the crashes. > > > > To be clear, I am using the following diff: > > The diff below fixes at (for amd64). > > What's happening is that uvm_map_inentry() may sleep to grab the lock > of the map. The fault address is read from cr2 in pageflttrap() which > gets called after this check and if the check sleeps, cr2 is likely to > be clobbered by a page fault in another process. > > Diff below fixes this by reading cr2 early and passing it to pageflttrap(). > > ok?
it makes sens. and I can't trigger the crashes with rustc build anymore. ok semarie@ > > Index: arch/amd64/amd64/trap.c > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/amd64/trap.c,v > retrieving revision 1.80 > diff -u -p -r1.80 trap.c > --- arch/amd64/amd64/trap.c 19 Aug 2020 10:10:57 -0000 1.80 > +++ arch/amd64/amd64/trap.c 14 Sep 2020 11:17:35 -0000 > @@ -92,7 +92,7 @@ > > #include "isa.h" > > -int pageflttrap(struct trapframe *, int _usermode); > +int pageflttrap(struct trapframe *, uint64_t, int _usermode); > void kerntrap(struct trapframe *); > void usertrap(struct trapframe *); > void ast(struct trapframe *); > @@ -157,12 +157,11 @@ fault(const char *format, ...) > * if something was so broken that we should panic. > */ > int > -pageflttrap(struct trapframe *frame, int usermode) > +pageflttrap(struct trapframe *frame, uint64_t cr2, int usermode) > { > struct proc *p = curproc; > struct pcb *pcb; > int error; > - uint64_t cr2; > vaddr_t va; > struct vm_map *map; > vm_prot_t ftype; > @@ -172,7 +171,6 @@ pageflttrap(struct trapframe *frame, int > > map = &p->p_vmspace->vm_map; > pcb = &p->p_addr->u_pcb; > - cr2 = rcr2(); > va = trunc_page((vaddr_t)cr2); > > KERNEL_LOCK(); > @@ -280,6 +278,7 @@ void > kerntrap(struct trapframe *frame) > { > int type = (int)frame->tf_trapno; > + uint64_t cr2 = rcr2(); > > verify_smap(__func__); > uvmexp.traps++; > @@ -299,7 +298,7 @@ kerntrap(struct trapframe *frame) > /*NOTREACHED*/ > > case T_PAGEFLT: /* allow page faults in kernel mode */ > - if (pageflttrap(frame, 0)) > + if (pageflttrap(frame, cr2, 0)) > return; > goto we_re_toast; > > @@ -333,6 +332,7 @@ usertrap(struct trapframe *frame) > { > struct proc *p = curproc; > int type = (int)frame->tf_trapno; > + uint64_t cr2 = rcr2(); > union sigval sv; > int sig, code; > > @@ -381,7 +381,7 @@ usertrap(struct trapframe *frame) > break; > > case T_PAGEFLT: /* page fault */ > - if (pageflttrap(frame, 1)) > + if (pageflttrap(frame, cr2, 1)) > goto out; > /* FALLTHROUGH */ > -- Sebastien Marie