From: Jérôme Gardou Date: Tue, 23 Sep 2014 18:06:47 +0000 (+0000) Subject: [NTOS/MM] X-Git-Tag: backups/0.3.17@66124~495 X-Git-Url: https://git.reactos.org/?p=reactos.git;a=commitdiff_plain;h=1b2d2c4f1b7983f200cf09ca662451349b372743 [NTOS/MM] - Simplify and fix MmCopyVirtualMemory, where the processes where not correctly detached on failure. Fixes a few "Process xyz.exe" is a zombie. svn path=/trunk/; revision=64242 --- diff --git a/reactos/ntoskrnl/mm/ARM3/virtual.c b/reactos/ntoskrnl/mm/ARM3/virtual.c index dced562185b..91f381aad1f 100644 --- a/reactos/ntoskrnl/mm/ARM3/virtual.c +++ b/reactos/ntoskrnl/mm/ARM3/virtual.c @@ -774,10 +774,10 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, 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 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, KeStackAttachProcess(&SourceProcess->Pcb, &ApcState); // - // Reset state for this pass + // Check state for this pass // - MdlAddress = NULL; - PagesLocked = FALSE; - FailedInMoving = FALSE; + ASSERT(MdlAddress == NULL); + ASSERT(PagesLocked == FALSE); ASSERT(FailedInProbe == FALSE); // @@ -847,35 +846,47 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, MmInitializeMdl(Mdl, CurrentAddress, CurrentSize); MmProbeAndLockPages(Mdl, PreviousMode, IoReadAccess); PagesLocked = TRUE; + } + _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER) + { + Status = _SEH2_GetExceptionCode(); + } + _SEH2_END - // - // 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); - } + /* Detach from source process */ + KeUnstackDetachProcess(&ApcState); - // - // Now let go of the source and grab to the target process - // - KeUnstackDetachProcess(&ApcState); - KeStackAttachProcess(&TargetProcess->Pcb, &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 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, // // Now do the actual move // - FailedInMoving = TRUE; RtlCopyMemory(CurrentTargetAddress, MdlAddress, CurrentSize); } _SEH2_EXCEPT(MiGetExceptionInfo(_SEH2_GetExceptionInformation(), &HaveBadAddress, &BadAddress)) { + *ReturnSize = BufferSize - RemainingSize; // - // 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 + // Check if we failed during the probe // - if ((FailedInProbe) || (FailedInMapping)) + 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 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, // *ReturnSize = BadAddress - (ULONG_PTR)SourceAddress; } + // + // Return partial copy + // + Status = STATUS_PARTIAL_COPY; } - - // - // Return partial copy - // - Status = STATUS_PARTIAL_COPY; } _SEH2_END; - // - // Check for SEH status - // - if (Status != STATUS_SUCCESS) return Status; + /* Detach from target process */ + KeUnstackDetachProcess(&ApcState); // - // Detach from target + // Check for SEH status // - 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 @@ MiDoMappedCopy(IN PEPROCESS SourceProcess, 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 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, { 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 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, 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 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, // 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,17 +1087,61 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, // Do the copy // RtlCopyMemory(PoolAddress, CurrentAddress, CurrentSize); + } + _SEH2_EXCEPT(MiGetExceptionInfo(_SEH2_GetExceptionInformation(), + &HaveBadAddress, + &BadAddress)) + { + *ReturnSize = BufferSize - RemainingSize; // - // Now let go of the source and grab to the target process + // Check if we failed during the probe // - KeUnstackDetachProcess(&ApcState); - KeStackAttachProcess(&TargetProcess->Pcb, &ApcState); + if (FailedInProbe) + { + // + // Exit + // + Status = _SEH2_GetExceptionCode(); + } + else + { + // + // 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 + /* 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 ((CurrentAddress == SourceAddress) && (PreviousMode != KernelMode)) + if ((CurrentTargetAddress == TargetAddress) && (PreviousMode != KernelMode)) { // // Catch a failure here @@ -1128,23 +1162,13 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, // // 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 +1178,11 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, // Exit // Status = _SEH2_GetExceptionCode(); - _SEH2_YIELD(return Status); } - - // - // Otherwise, we failed, probably during the move - // - *ReturnSize = BufferSize - RemainingSize; - if (FailedInMoving) + else { // + // Otherwise we failed during the move. // Check if we know exactly where we stopped copying // if (HaveBadAddress) @@ -1173,24 +1192,26 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, // *ReturnSize = BadAddress - (ULONG_PTR)SourceAddress; } + // + // Return partial copy + // + Status = STATUS_PARTIAL_COPY; } - - // - // Return partial copy - // - Status = STATUS_PARTIAL_COPY; } _SEH2_END; // - // Check for SEH status + // Detach from target // - if (Status != STATUS_SUCCESS) return Status; + KeUnstackDetachProcess(&ApcState); // - // Detach from target + // Check for SEH status // - KeUnstackDetachProcess(&ApcState); + if (Status != STATUS_SUCCESS) + { + goto Exit; + } // // Update location and size @@ -1201,16 +1222,19 @@ MiDoPoolCopy(IN PEPROCESS SourceProcess, 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