[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, <tfaber at svn.reactos.org> 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);
>> +}
>> +
>>
>>