[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 dced562..91f381a 100644 (file)
--- 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