[ros-diffs] [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