[jgardou] 64242: [NTOS/MM] - Simplify and fix MmCopyVirtualMemory, where the processes where not correctly detached on failure. Fixes a few "Process xyz.exe" is a zombie.
23 Sep
2014
23 Sep
'14
6:06 p.m.
Author: jgardou
Date: Tue Sep 23 18:06:47 2014
New Revision: 64242
URL: http://svn.reactos.org/svn/reactos?rev=64242&view=rev
Log:
[NTOS/MM]
- Simplify and fix MmCopyVirtualMemory, where the processes where not correctly detached
on failure.
Fixes a few "Process xyz.exe" is a zombie.
Modified:
trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
Modified: trunk/reactos/ntoskrnl/mm/ARM3/virtual.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/ARM3/virtual.c…
==============================================================================
--- trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/mm/ARM3/virtual.c [iso-8859-1] Tue Sep 23 18:06:47 2014
@@ -774,10 +774,10 @@
PFN_NUMBER MdlBuffer[(sizeof(MDL) / sizeof(PFN_NUMBER)) + MI_MAPPED_COPY_PAGES + 1];
PMDL Mdl = (PMDL)MdlBuffer;
SIZE_T TotalSize, CurrentSize, RemainingSize;
- volatile BOOLEAN FailedInProbe = FALSE, FailedInMapping = FALSE, FailedInMoving;
- volatile BOOLEAN PagesLocked;
+ volatile BOOLEAN FailedInProbe = FALSE;
+ volatile BOOLEAN PagesLocked = FALSE;
PVOID CurrentAddress = SourceAddress, CurrentTargetAddress = TargetAddress;
- volatile PVOID MdlAddress;
+ volatile PVOID MdlAddress = NULL;
KAPC_STATE ApcState;
BOOLEAN HaveBadAddress;
ULONG_PTR BadAddress;
@@ -808,11 +808,10 @@
KeStackAttachProcess(&SourceProcess->Pcb, &ApcState);
//
- // Reset state for this pass
- //
- MdlAddress = NULL;
- PagesLocked = FALSE;
- FailedInMoving = FALSE;
+ // Check state for this pass
+ //
+ ASSERT(MdlAddress == NULL);
+ ASSERT(PagesLocked == FALSE);
ASSERT(FailedInProbe == FALSE);
//
@@ -847,35 +846,47 @@
MmInitializeMdl(Mdl, CurrentAddress, CurrentSize);
MmProbeAndLockPages(Mdl, PreviousMode, IoReadAccess);
PagesLocked = TRUE;
-
- //
- // Now map the pages
- //
- MdlAddress = MmMapLockedPagesSpecifyCache(Mdl,
- KernelMode,
- MmCached,
- NULL,
- FALSE,
- HighPagePriority);
- if (!MdlAddress)
- {
- //
- // Use our SEH handler to pick this up
- //
- FailedInMapping = TRUE;
- ExRaiseStatus(STATUS_INSUFFICIENT_RESOURCES);
- }
-
- //
- // Now let go of the source and grab to the target process
- //
- KeUnstackDetachProcess(&ApcState);
- KeStackAttachProcess(&TargetProcess->Pcb, &ApcState);
-
+ }
+ _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
+ {
+ Status = _SEH2_GetExceptionCode();
+ }
+ _SEH2_END
+
+ /* Detach from source process */
+ KeUnstackDetachProcess(&ApcState);
+
+ if (Status != STATUS_SUCCESS)
+ {
+ goto Exit;
+ }
+
+ //
+ // Now map the pages
+ //
+ MdlAddress = MmMapLockedPagesSpecifyCache(Mdl,
+ KernelMode,
+ MmCached,
+ NULL,
+ FALSE,
+ HighPagePriority);
+ if (!MdlAddress)
+ {
+ Status = STATUS_INSUFFICIENT_RESOURCES;
+ goto Exit;
+ }
+
+ //
+ // Grab to the target process
+ //
+ KeStackAttachProcess(&TargetProcess->Pcb, &ApcState);
+
+ _SEH2_TRY
+ {
//
// Check if this is our first time through
//
- if ((CurrentAddress == SourceAddress) && (PreviousMode !=
KernelMode))
+ if ((CurrentTargetAddress == TargetAddress) && (PreviousMode !=
KernelMode))
{
//
// Catch a failure here
@@ -896,58 +907,27 @@
//
// Now do the actual move
//
- FailedInMoving = TRUE;
RtlCopyMemory(CurrentTargetAddress, MdlAddress, CurrentSize);
}
_SEH2_EXCEPT(MiGetExceptionInfo(_SEH2_GetExceptionInformation(),
&HaveBadAddress,
&BadAddress))
{
- //
- // Detach from whoever we may be attached to
- //
- KeUnstackDetachProcess(&ApcState);
-
- //
- // Check if we had mapped the pages
- //
- if (MdlAddress) MmUnmapLockedPages(MdlAddress, Mdl);
-
- //
- // Check if we had locked the pages
- //
- if (PagesLocked) MmUnlockPages(Mdl);
-
- //
- // Check if we hit working set quota
- //
- if (_SEH2_GetExceptionCode() == STATUS_WORKING_SET_QUOTA)
- {
- //
- // Return the error
- //
- _SEH2_YIELD(return STATUS_WORKING_SET_QUOTA);
- }
-
- //
- // Check if we failed during the probe or mapping
- //
- if ((FailedInProbe) || (FailedInMapping))
+ *ReturnSize = BufferSize - RemainingSize;
+ //
+ // Check if we failed during the probe
+ //
+ if (FailedInProbe)
{
//
// Exit
//
Status = _SEH2_GetExceptionCode();
- _SEH2_YIELD(return Status);
- }
-
- //
- // Otherwise, we failed probably during the move
- //
- *ReturnSize = BufferSize - RemainingSize;
- if (FailedInMoving)
- {
- //
+ }
+ else
+ {
+ //
+ // Othewise we failed during the move.
// Check if we know exactly where we stopped copying
//
if (HaveBadAddress)
@@ -957,30 +937,32 @@
//
*ReturnSize = BadAddress - (ULONG_PTR)SourceAddress;
}
- }
-
- //
- // Return partial copy
- //
- Status = STATUS_PARTIAL_COPY;
+ //
+ // Return partial copy
+ //
+ Status = STATUS_PARTIAL_COPY;
+ }
}
_SEH2_END;
+ /* Detach from target process */
+ KeUnstackDetachProcess(&ApcState);
+
//
// Check for SEH status
//
- if (Status != STATUS_SUCCESS) return Status;
-
- //
- // Detach from target
- //
- KeUnstackDetachProcess(&ApcState);
+ if (Status != STATUS_SUCCESS)
+ {
+ goto Exit;
+ }
//
// Unmap and unlock
//
MmUnmapLockedPages(MdlAddress, Mdl);
+ MdlAddress = NULL;
MmUnlockPages(Mdl);
+ PagesLocked = FALSE;
//
// Update location and size
@@ -990,11 +972,18 @@
CurrentTargetAddress = (PVOID)((ULONG_PTR)CurrentTargetAddress + CurrentSize);
}
+Exit:
+ if (MdlAddress != NULL)
+ MmUnmapLockedPages(MdlAddress, Mdl);
+ if (PagesLocked)
+ MmUnlockPages(Mdl);
+
//
// All bytes read
//
- *ReturnSize = BufferSize;
- return STATUS_SUCCESS;
+ if (Status == STATUS_SUCCESS)
+ *ReturnSize = BufferSize;
+ return Status;
}
NTSTATUS
@@ -1009,7 +998,7 @@
{
UCHAR StackBuffer[MI_POOL_COPY_BYTES];
SIZE_T TotalSize, CurrentSize, RemainingSize;
- volatile BOOLEAN FailedInProbe = FALSE, FailedInMoving, HavePoolAddress = FALSE;
+ volatile BOOLEAN FailedInProbe = FALSE, HavePoolAddress = FALSE;
PVOID CurrentAddress = SourceAddress, CurrentTargetAddress = TargetAddress;
PVOID PoolAddress;
KAPC_STATE ApcState;
@@ -1018,6 +1007,9 @@
NTSTATUS Status = STATUS_SUCCESS;
PAGED_CODE();
+ DPRINT("Copying %Iu bytes from process %p (address %p) to process %p (Address
%p)\n",
+ BufferSize, SourceProcess, SourceAddress, TargetProcess, TargetAddress);
+
//
// Calculate the maximum amount of data to move
//
@@ -1061,11 +1053,9 @@
//
KeStackAttachProcess(&SourceProcess->Pcb, &ApcState);
- //
- // Reset state for this pass
- //
- FailedInMoving = FALSE;
+ /* Check that state is sane */
ASSERT(FailedInProbe == FALSE);
+ ASSERT(Status == STATUS_SUCCESS);
//
// Protect user-mode copy
@@ -1097,53 +1087,12 @@
// Do the copy
//
RtlCopyMemory(PoolAddress, CurrentAddress, CurrentSize);
-
- //
- // Now let go of the source and grab to the target process
- //
- KeUnstackDetachProcess(&ApcState);
- KeStackAttachProcess(&TargetProcess->Pcb, &ApcState);
-
- //
- // Check if this is our first time through
- //
- if ((CurrentAddress == SourceAddress) && (PreviousMode !=
KernelMode))
- {
- //
- // Catch a failure here
- //
- FailedInProbe = TRUE;
-
- //
- // Do the probe
- //
- ProbeForWrite(TargetAddress, BufferSize, sizeof(CHAR));
-
- //
- // Passed
- //
- FailedInProbe = FALSE;
- }
-
- //
- // Now do the actual move
- //
- FailedInMoving = TRUE;
- RtlCopyMemory(CurrentTargetAddress, PoolAddress, CurrentSize);
}
_SEH2_EXCEPT(MiGetExceptionInfo(_SEH2_GetExceptionInformation(),
&HaveBadAddress,
&BadAddress))
{
- //
- // Detach from whoever we may be attached to
- //
- KeUnstackDetachProcess(&ApcState);
-
- //
- // Check if we had allocated pool
- //
- if (HavePoolAddress) ExFreePoolWithTag(PoolAddress, 'VmRw');
+ *ReturnSize = BufferSize - RemainingSize;
//
// Check if we failed during the probe
@@ -1154,16 +1103,11 @@
// Exit
//
Status = _SEH2_GetExceptionCode();
- _SEH2_YIELD(return Status);
- }
-
- //
- // Otherwise, we failed, probably during the move
- //
- *ReturnSize = BufferSize - RemainingSize;
- if (FailedInMoving)
- {
- //
+ }
+ else
+ {
+ //
+ // We failed during the move.
// Check if we know exactly where we stopped copying
//
if (HaveBadAddress)
@@ -1173,24 +1117,101 @@
//
*ReturnSize = BadAddress - (ULONG_PTR)SourceAddress;
}
- }
-
- //
- // Return partial copy
- //
- Status = STATUS_PARTIAL_COPY;
+ //
+ // Return partial copy
+ //
+ Status = STATUS_PARTIAL_COPY;
+ }
+ }
+ _SEH2_END
+
+ /* Let go of the source */
+ KeUnstackDetachProcess(&ApcState);
+
+ if (Status != STATUS_SUCCESS)
+ {
+ goto Exit;
+ }
+
+ /* Grab the target process */
+ KeStackAttachProcess(&TargetProcess->Pcb, &ApcState);
+
+ _SEH2_TRY
+ {
+ //
+ // Check if this is our first time through
+ //
+ if ((CurrentTargetAddress == TargetAddress) && (PreviousMode !=
KernelMode))
+ {
+ //
+ // Catch a failure here
+ //
+ FailedInProbe = TRUE;
+
+ //
+ // Do the probe
+ //
+ ProbeForWrite(TargetAddress, BufferSize, sizeof(CHAR));
+
+ //
+ // Passed
+ //
+ FailedInProbe = FALSE;
+ }
+
+ //
+ // Now do the actual move
+ //
+ RtlCopyMemory(CurrentTargetAddress, PoolAddress, CurrentSize);
+ }
+ _SEH2_EXCEPT(MiGetExceptionInfo(_SEH2_GetExceptionInformation(),
+ &HaveBadAddress,
+ &BadAddress))
+ {
+ *ReturnSize = BufferSize - RemainingSize;
+ //
+ // Check if we failed during the probe
+ //
+ if (FailedInProbe)
+ {
+ //
+ // Exit
+ //
+ Status = _SEH2_GetExceptionCode();
+ }
+ else
+ {
+ //
+ // Otherwise we failed during the move.
+ // Check if we know exactly where we stopped copying
+ //
+ if (HaveBadAddress)
+ {
+ //
+ // Return the exact number of bytes copied
+ //
+ *ReturnSize = BadAddress - (ULONG_PTR)SourceAddress;
+ }
+ //
+ // Return partial copy
+ //
+ Status = STATUS_PARTIAL_COPY;
+ }
}
_SEH2_END;
//
+ // Detach from target
+ //
+ KeUnstackDetachProcess(&ApcState);
+
+ //
// Check for SEH status
//
- if (Status != STATUS_SUCCESS) return Status;
-
- //
- // Detach from target
- //
- KeUnstackDetachProcess(&ApcState);
+ if (Status != STATUS_SUCCESS)
+ {
+ goto Exit;
+ }
//
// Update location and size
@@ -1201,16 +1222,19 @@
CurrentSize);
}
+Exit:
//
// Check if we had allocated pool
//
- if (HavePoolAddress) ExFreePoolWithTag(PoolAddress, 'VmRw');
+ if (HavePoolAddress)
+ ExFreePoolWithTag(PoolAddress, 'VmRw');
//
// All bytes read
//
- *ReturnSize = BufferSize;
- return STATUS_SUCCESS;
+ if (Status == STATUS_SUCCESS)
+ *ReturnSize = BufferSize;
+ return Status;
}
NTSTATUS
3650
days inactive
3650
days old
0 comments
1 participants
participants (1)
-
jgardou＠svn.reactos.org