[ros-diffs] [mnordell] 29632: Probe arguments if coming from usermode. Use previous mode when referencing process handle. Actually specify PsProcessType for process handle. Some minor cleanup (less compiled code and some speedup).

mnordell at svn.reactos.org mnordell at svn.reactos.org
Wed Oct 17 10:16:20 CEST 2007


Author: mnordell
Date: Wed Oct 17 12:16:20 2007
New Revision: 29632

URL: http://svn.reactos.org/svn/reactos?rev=29632&view=rev
Log:
Probe arguments if coming from usermode. Use previous mode when referencing process handle. Actually specify PsProcessType for process handle. Some minor cleanup (less compiled code and some speedup).

Modified:
    trunk/reactos/ntoskrnl/mm/anonmem.c

Modified: trunk/reactos/ntoskrnl/mm/anonmem.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/mm/anonmem.c?rev=29632&r1=29631&r2=29632&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/mm/anonmem.c (original)
+++ trunk/reactos/ntoskrnl/mm/anonmem.c Wed Oct 17 12:16:20 2007
@@ -516,12 +516,12 @@
  * @implemented
  */
 NTSTATUS STDCALL
-NtAllocateVirtualMemory(IN HANDLE ProcessHandle,
-                        IN OUT PVOID*  UBaseAddress,
-                        IN ULONG ZeroBits,
+NtAllocateVirtualMemory(IN     HANDLE ProcessHandle,
+                        IN OUT PVOID* UBaseAddress,
+                        IN     ULONG  ZeroBits,
                         IN OUT PULONG URegionSize,
-                        IN ULONG AllocationType,
-                        IN ULONG Protect)
+                        IN     ULONG  AllocationType,
+                        IN     ULONG  Protect)
 /*
  * FUNCTION: Allocates a block of virtual memory in the process address space
  * ARGUMENTS:
@@ -542,6 +542,10 @@
  *                PAGE_EXECUTE_READ, PAGE_EXECUTE_READWRITE, PAGE_GUARD,
  *                PAGE_NOACCESS
  * RETURNS: Status
+ * NOTES: Must run at IRQL PASSIVE_LEVEL? (or is APC_LEVEL cool too?)
+ *        MSDN states that ZwAllocateVirtualMemory IRQL must be PASSIVE_LEVEL,
+ *        but why wouldn't APC_LEVEL be valid (or is that only for the Zw* version
+ *        and Nt* can indeed run at APC_LEVEL?)
  */
 {
    PEPROCESS Process;
@@ -555,6 +559,12 @@
    PVOID PBaseAddress;
    ULONG PRegionSize;
    PHYSICAL_ADDRESS BoundaryAddressMultiple;
+   KPROCESSOR_MODE PreviousMode;
+
+   // TMN: Someone Pick one of these. Until it's clear which
+   // level is allowed, I play it safe and check for <= APC_LEVEL
+   PAGED_CODE();
+//   ASSERT(KeGetCurrentIrql() == PASSIVE_LEVEL);
 
    DPRINT("NtAllocateVirtualMemory(*UBaseAddress %x, "
           "ZeroBits %d, *URegionSize %x, AllocationType %x, Protect %x)\n",
@@ -576,7 +586,7 @@
    }
 
    /* Check for valid Allocation Types */
-   if ((AllocationType &~ (MEM_COMMIT | MEM_RESERVE | MEM_RESET | MEM_PHYSICAL |
+   if ((AllocationType & ~(MEM_COMMIT | MEM_RESERVE | MEM_RESET | MEM_PHYSICAL |
                            MEM_TOP_DOWN | MEM_WRITE_WATCH)))
    {
       DPRINT1("Invalid Allocation Type\n");
@@ -593,7 +603,7 @@
    /* MEM_RESET is an exclusive flag, make sure that is valid too */
    if ((AllocationType & MEM_RESET) && (AllocationType != MEM_RESET))
    {
-      DPRINT1("MEM_RESET used illegaly\n");
+      DPRINT1("Invalid use of MEM_RESET\n");
       return STATUS_INVALID_PARAMETER_5;
    }
 
@@ -623,12 +633,30 @@
       }
    }
 
-   PBaseAddress = *UBaseAddress;
-   PRegionSize = *URegionSize;
+   PreviousMode = KeGetPreviousMode();
+
+   _SEH_TRY
+   {
+      if (PreviousMode != KernelMode)
+      {
+         ProbeForWritePointer(UBaseAddress);
+         ProbeForWriteUlong(URegionSize);
+      }
+      PBaseAddress = *UBaseAddress;
+      PRegionSize  = *URegionSize;
+   }
+   _SEH_HANDLE
+   {
+      /* Get the exception code */
+      Status = _SEH_GetExceptionCode();
+      return(Status); /* Correct? NT5 returns STATUS_ACCESS_VIOLATION ? */
+   }
+   _SEH_END;
+
    BoundaryAddressMultiple.QuadPart = 0;
 
    BaseAddress = (PVOID)PAGE_ROUND_DOWN(PBaseAddress);
-   RegionSize = PAGE_ROUND_UP(PBaseAddress + PRegionSize) -
+   RegionSize = PAGE_ROUND_UP((ULONG_PTR)PBaseAddress + PRegionSize) -
                 PAGE_ROUND_DOWN(PBaseAddress);
 
    /* 
@@ -638,12 +666,12 @@
     */
    if (BaseAddress >= MM_HIGHEST_USER_ADDRESS)
    {
-      DPRINT1("Virtual allocation above User Space\n");
+      DPRINT1("Virtual allocation base above User Space\n");
       return STATUS_INVALID_PARAMETER_2;
    }
    if (!RegionSize)
    {
-      DPRINT1("Region size is invalid\n");
+      DPRINT1("Region size is invalid (zero)\n");
       return STATUS_INVALID_PARAMETER_4;
    }
    if (((ULONG_PTR)MM_HIGHEST_USER_ADDRESS - (ULONG_PTR)BaseAddress) < RegionSize)
@@ -657,7 +685,7 @@
     * but there may be other cases...needs more testing
     */
    if ((!BaseAddress || (AllocationType & MEM_RESERVE)) && 
-       ((Protect & PAGE_WRITECOPY) || (Protect & PAGE_EXECUTE_WRITECOPY)))
+       (Protect & (PAGE_WRITECOPY | PAGE_EXECUTE_WRITECOPY)))
    {
       DPRINT1("Copy on write is not supported by VirtualAlloc\n");
       return STATUS_INVALID_PAGE_PROTECTION;
@@ -666,8 +694,8 @@
 
    Status = ObReferenceObjectByHandle(ProcessHandle,
                                       PROCESS_VM_OPERATION,
-                                      NULL,
-                                      UserMode,
+                                      PsProcessType,
+                                      PreviousMode,
                                       (PVOID*)(&Process),
                                       NULL);
    if (!NT_SUCCESS(Status))
@@ -750,10 +778,10 @@
                       MemoryAreaLength, Type, Protect);
 
    if ((AllocationType & MEM_COMMIT) &&
-         ((Protect & PAGE_READWRITE) ||
-          (Protect & PAGE_EXECUTE_READWRITE)))
-   {
-      MmReserveSwapPages(MemoryAreaLength);
+       (Protect & (PAGE_READWRITE | PAGE_EXECUTE_READWRITE)))
+   {
+      const ULONG nPages = PAGE_ROUND_UP(MemoryAreaLength) >> PAGE_SHIFT;
+      MmReserveSwapPages(nPages);
    }
 
    *UBaseAddress = BaseAddress;
@@ -816,17 +844,11 @@
    {
       ULONG_PTR MemoryAreaLength = (ULONG_PTR)MemoryArea->EndingAddress -
                                    (ULONG_PTR)MemoryArea->StartingAddress;
-
-      /* FiN TODO: Optimize loop counter! */
-      for (i = 0; i < PAGE_ROUND_UP(MemoryAreaLength) / PAGE_SIZE; i++)
+      const ULONG nPages = PAGE_ROUND_UP(MemoryAreaLength) >> PAGE_SHIFT;
+
+      for (i = 0; i < nPages && MemoryArea->PageOpCount != 0; ++i)
       {
          PMM_PAGEOP PageOp;
-
-         if (MemoryArea->PageOpCount == 0)
-         {
-            break;
-         }
-
          PageOp = MmCheckForPageOp(MemoryArea, Process->UniqueProcessId,
                                    (PVOID)((ULONG_PTR)MemoryArea->StartingAddress + (i * PAGE_SIZE)),
                                    NULL, 0);
@@ -899,7 +921,7 @@
           *PRegionSize,FreeType);
 
    BaseAddress = (PVOID)PAGE_ROUND_DOWN((*PBaseAddress));
-   RegionSize = PAGE_ROUND_UP((*PBaseAddress) + (*PRegionSize)) -
+   RegionSize = PAGE_ROUND_UP((ULONG_PTR)(*PBaseAddress) + (*PRegionSize)) -
                 PAGE_ROUND_DOWN((*PBaseAddress));
 
    Status = ObReferenceObjectByHandle(ProcessHandle,
@@ -919,9 +941,8 @@
    MemoryArea = MmLocateMemoryAreaByAddress(AddressSpace, BaseAddress);
    if (MemoryArea == NULL)
    {
-      MmUnlockAddressSpace(AddressSpace);
-      ObDereferenceObject(Process);
-      return(STATUS_UNSUCCESSFUL);
+      Status = STATUS_UNSUCCESSFUL;
+      goto unlock_deref_and_return;
    }
 
    switch (FreeType)
@@ -931,14 +952,13 @@
          if (MemoryArea->StartingAddress != BaseAddress ||
              MemoryArea->Type != MEMORY_AREA_VIRTUAL_MEMORY)
          {
-            MmUnlockAddressSpace(AddressSpace);
-            ObDereferenceObject(Process);
-            return(STATUS_UNSUCCESSFUL);
+            Status = STATUS_UNSUCCESSFUL;
+            goto unlock_deref_and_return;
          }
+
          MmFreeVirtualMemory(Process, MemoryArea);
-         MmUnlockAddressSpace(AddressSpace);
-         ObDereferenceObject(Process);
-         return(STATUS_SUCCESS);
+         Status = STATUS_SUCCESS;
+         goto unlock_deref_and_return;
 
       case MEM_DECOMMIT:
          Status =
@@ -950,13 +970,17 @@
                           MEM_RESERVE,
                           PAGE_NOACCESS,
                           MmModifyAttributes);
-         MmUnlockAddressSpace(AddressSpace);
-         ObDereferenceObject(Process);
-         return(Status);
-   }
+         goto unlock_deref_and_return;
+   }
+
+   Status = STATUS_NOT_IMPLEMENTED;
+
+unlock_deref_and_return:
+
    MmUnlockAddressSpace(AddressSpace);
    ObDereferenceObject(Process);
-   return(STATUS_NOT_IMPLEMENTED);
+
+   return(Status);
 }
 
 NTSTATUS




More information about the Ros-diffs mailing list