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. ...
14 Apr
2017
14 Apr
'17
4:51 p.m.
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(a)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);
+}
+
14 Apr
14 Apr
9:36 p.m.
New subject: [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. ...
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(a)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);
> +}
> +
>
>
2716
days inactive
2716
days old
1 comments
2 participants
participants (2)
-
Alex Ionescu -
Thomas Faber