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
