On Sunday, November 20, 2016 10:31:31 AM Konstantin Belousov wrote:
> On Sat, Nov 19, 2016 at 01:36:44AM +0000, John Baldwin wrote:
> > Author: jhb
> > Date: Sat Nov 19 01:36:44 2016
> > New Revision: 308821
> > URL: https://svnweb.freebsd.org/changeset/base/308821
> > 
> > Log:
> >   MFamd64: Various fatal page fault fixes.
> >   
> >   - If a page fault is triggered due to reserved bits in a PTE, treat it
> >     as a fatal fault and panic.
> >   - If PG_NX is in use, report whether a fatal page fault is due to an
> >     instruction fetch or a data access.
> >   - If a fatal page fault is due to reserved bits in a PTE, report that as
> >     the page fault type rather than a protection violation.
> >   
> >   MFC after:        1 month
> > 
> > Modified:
> >   head/sys/i386/i386/trap.c
> > 
> > Modified: head/sys/i386/i386/trap.c
> > ==============================================================================
> > --- head/sys/i386/i386/trap.c       Sat Nov 19 01:34:12 2016        
> > (r308820)
> > +++ head/sys/i386/i386/trap.c       Sat Nov 19 01:36:44 2016        
> > (r308821)
> > @@ -857,6 +857,14 @@ trap_pfault(frame, usermode, eva)
> >     }
> >  
> >     /*
> > +    * If the trap was caused by errant bits in the PTE then panic.
> > +    */
> > +   if (frame->tf_err & PGEX_RSV) {
> > +           trap_fatal(frame, eva);
> > +           return (-1);
> > +   }
> > +
> > +   /*
> >      * PGEX_I is defined only if the execute disable bit capability is
> >      * supported and enabled.
> >      */
> > @@ -926,9 +934,15 @@ trap_fatal(frame, eva)
> >  #endif
> >     if (type == T_PAGEFLT) {
> >             printf("fault virtual address   = 0x%x\n", eva);
> > -           printf("fault code              = %s %s, %s\n",
> > +           printf("fault code              = %s %s%s, %s\n",
> >                     code & PGEX_U ? "user" : "supervisor",
> >                     code & PGEX_W ? "write" : "read",
> > +#if defined(PAE) || defined(PAE_TABLES)
> > +                   pg_nx != 0 ?
> > +                   (code & PGEX_I ? " instruction" : " data") :
> > +#endif
> I suggest to remove #ifdef guards, and the pg_nx check there, as
> well. The page fault exception error code bits have constant meaning
> regardless of the kernel and CPU configuration, so if we get the I bit
> set in the error word, we know that it was due to executing on PAE table
> with bit 63 (nx) set. This is true even if kernel was not configured to
> create such tables.
> 
> In other words, it would give more correct and useful information, which
> make it easier to track page tables corruption.

The SDM claims that this bit is only defined if pg_nx is enabled, so
an instruction fault when EFER_NXE is not enabled would report this bit
as clear resulting in the trap output saying "data" when it was really
"instruction".  That is, on i386 without pg_nx, the I/D being zero can
mean either instruction or data, not just data.  I felt always printing
data in that case could be misleading (someone might get a fatal trap
and be confused that it reports a data fault when it is the instruction
fetch that failed).

-- 
John Baldwin
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to