[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.
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
-
jgardou＠svn.reactos.org