svn commit: r308821 - head/sys/i386/i386

[John Baldwin]

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