Re: [ros-dev] [ros-diffs] [tfaber] 74305: [NTOS:KE] - Gracefully handle page faults during V86 code execution. This is a bit of a hack because with our limited use of V86 code it is unclear how a page fault can even occur. ...

[Thomas Faber]

I said it's unclear how, not that it doesn't happen ;)

This change fixes boot on the machine mentioned in
https://jira.reactos.org/browse/CORE-12993

What happens is there's a bug in the BIOS that causes a ret to an
broken address, then we execute garbage and fault on a stack address.
The stack didn't overflow so I'm not sure what caused the page to
become invalid; but the machine doesn't have a serial port and I'm at
the end of what I'm able (well, willing) to do with printf debugging.
Hence the hack. I'm always open to better suggestions :)

On 2017-04-14 18:51, Alex Ionescu wrote:
> So why implement this code path if we have nothing that hits it?
>
> Best regards,
> Alex Ionescu
>
> On Fri, Apr 14, 2017 at 4:18 AM, [e-mail] wrote:
> > Author: tfaber
> > Date: Fri Apr 14 11:18:22 2017
> > New Revision: 74305
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=74305&view=rev
> > Log:
> > [NTOS:KE]
> > Gracefully handle page faults during V86 code execution. This is a bit
> >
> > of a hack because with our limited use of V86 code it is unclear how a page
> > fault can even occur.
> > CORE-12993 #resolve
> >
> > Modified:
> >     trunk/reactos/ntoskrnl/include/internal/i386/ke.h
> >     trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> >     trunk/reactos/ntoskrnl/vdm/vdmexec.c
> >
> > Modified: trunk/reactos/ntoskrnl/include/internal/i386/ke.h
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/
> > ntoskrnl/include/internal/i386/ke.h?rev=74305&r1=74304&r2=74305&view=diff
> > ============================================================
> > ==================
> > --- trunk/reactos/ntoskrnl/include/internal/i386/ke.h   [iso-8859-1]
> > (original)
> > +++ trunk/reactos/ntoskrnl/include/internal/i386/ke.h   [iso-8859-1] Fri
> > Apr 14 11:18:22 2017
> > @@ -478,6 +478,12 @@
> >  NTAPI
> >  VdmDispatchBop(
> >      IN PKTRAP_FRAME TrapFrame
> > +);
> >
> > +BOOLEAN
> > +NTAPI
> > +VdmDispatchPageFault(
> > _In_ PKTRAP_FRAME TrapFrame
> >
> > );
> >
> > BOOLEAN
> >
> > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/
> > ntoskrnl/ke/i386/traphdlr.c?rev=74305&r1=74304&r2=74305&view=diff
> > ============================================================
> > ==================
> > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c   [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c   [iso-8859-1] Fri Apr 14
> > 11:18:22 2017
> > @@ -1304,8 +1304,20 @@
> >          UNIMPLEMENTED_FATAL();
> >      }
> >  #endif
> > /* Check for VDM trap */
> > ASSERT((KiVdmTrap(TrapFrame)) == FALSE);
> > if (KiVdmTrap(TrapFrame))
> > {
> >    DPRINT1("VDM PAGE FAULT at %lx:%lx for address %lx\n",
> >            TrapFrame->SegCs, TrapFrame->Eip, Cr2);
> >    if (VdmDispatchPageFault(TrapFrame))
> >    {
> >        /* Return and end VDM execution */
> >        DPRINT1("VDM page fault with status 0x%lx resolved\n",
> >
> > Status);
> >        KiEoiHelper(TrapFrame);
> >    }
> >    DPRINT1("VDM page fault with status 0x%lx NOT resolved\n",
> >
> > Status);
> >
> > }
> >
> > /* Either kernel or user trap (non VDM) so dispatch exception */
> > if (Status == STATUS_ACCESS_VIOLATION)
> >
> > Modified: trunk/reactos/ntoskrnl/vdm/vdmexec.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/
> > ntoskrnl/vdm/vdmexec.c?rev=74305&r1=74304&r2=74305&view=diff
> > ============================================================
> > ==================
> > --- trunk/reactos/ntoskrnl/vdm/vdmexec.c        [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/vdm/vdmexec.c        [iso-8859-1] Fri Apr 14
> > 11:18:22 2017
> > @@ -239,7 +239,7 @@
> >          KeLowerIrql(OldIrql);
> >          return STATUS_INVALID_SYSTEM_SERVICE;
> >      }
> > /* Now do the VDM Swap */
> >    VdmSwapContext(VdmFrame, &VdmTib->MonitorContext, &VdmContext);
> >
> > @@ -269,7 +269,7 @@
> >  /* Make a copy of the monitor context */
> >  Context = VdmTib->MonitorContext;
> > /* Check if V86 mode was enabled or the trap was edited */
> >    if ((Context.EFlags & EFLAGS_V86_MASK) || (Context.SegCs &
> >
> > FRAME_EDITED))
> >      {
> > @@ -291,7 +291,7 @@
> >                  /* Remove real IF flag */
> >                  VdmTib->VdmContext.EFlags &= ~EFLAGS_INTERRUPT_MASK;
> >              }
> >        /* Turn off VIP and VIF */
> >        TrapFrame->EFlags &= ~(EFLAGS_VIP | EFLAGS_VIF);
> >        VdmTib->VdmContext.EFlags &= ~(EFLAGS_VIP | EFLAGS_VIF);
> >
> > @@ -362,4 +362,45 @@
> >      return TRUE;
> >  }
> >
> > +BOOLEAN
> > +NTAPI
> > +VdmDispatchPageFault(
> > _In_ PKTRAP_FRAME TrapFrame)
> >
> > +{
> > NTSTATUS Status;
> > PVDM_TIB VdmTib;
> > PAGED_CODE();
> > /* Get the VDM TIB so we can terminate V86 execution */
> > Status = VdmpGetVdmTib(&VdmTib);
> > if (!NT_SUCCESS(Status))
> > {
> >    /* Not a proper VDM fault, keep looking */
> >    DPRINT1("VdmDispatchPageFault: no VDM TIB, Vdm=%p\n",
> >
> > NtCurrentTeb()->Vdm);
> >    return FALSE;
> > }
> > /* Must be coming from V86 code */
> > ASSERT(TrapFrame->EFlags & EFLAGS_V86_MASK);
> > _SEH2_TRY
> > {
> >    /* Fill out a VDM Event */
> >    VdmTib->EventInfo.Event = VdmMemAccess;
> >    VdmTib->EventInfo.InstructionSize = 0;
> >    /* End VDM Execution */
> >    VdmEndExecution(TrapFrame, VdmTib);
> > }
> > _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
> > {
> >    Status = _SEH2_GetExceptionCode();
> > }
> > _SEH2_END;
> > /* Consider the exception handled if we succeeded */
> > DPRINT1("VdmDispatchPageFault EFlags %lx exit with 0x%lx\n",
> >
> > TrapFrame->EFlags, Status);
> > return NT_SUCCESS(Status);
> >
> > +}