[ros-diffs] [hbirr] 18743: - Set the correct type and state in MiQueryVirtualMemory.

- Set the correct type and state in MiQueryVirtualMemory.  
- Return an error in MiProtectVirtualMemory if we trying to access a
region other than a section view or virtual memory.  
- Don't allow to read or write kernel memory in
NtRead/WriteVirtualMemory.  
- Don't unprotect the memory in NtWriteVirtualMemory.  
- Don't allow to change the memory protection of kernel address space.  
- If it is possible, access the memory aligned by pages (in
ProbeForWrite).
Modified: trunk/reactos/ntoskrnl/mm/virtual.c
  _____  

Modified: trunk/reactos/ntoskrnl/mm/virtual.c
--- trunk/reactos/ntoskrnl/mm/virtual.c	2005-10-24 17:13:08 UTC (rev
18742)
+++ trunk/reactos/ntoskrnl/mm/virtual.c	2005-10-24 17:21:23 UTC (rev
18743)
@@ -184,8 +184,8 @@

                                                  ResultLength);
                      break;
 		  case MEMORY_AREA_NO_ACCESS:
-	             Info->Type = 0;
-                     Info->State = MEM_FREE;
+	             Info->Type = MEM_PRIVATE;
+                     Info->State = MEM_RESERVE;
 	             Info->Protect = MemoryArea->Attributes;
 		     Info->AllocationProtect = MemoryArea->Attributes;
 	             Info->BaseAddress = MemoryArea->StartingAddress;
@@ -196,7 +196,7 @@
                      *ResultLength = sizeof(MEMORY_BASIC_INFORMATION);
 	             break;
 		  case MEMORY_AREA_SHARED_DATA:
-	             Info->Type = 0;
+	             Info->Type = MEM_PRIVATE;
                      Info->State = MEM_COMMIT;
 	             Info->Protect = MemoryArea->Attributes;
 		     Info->AllocationProtect = MemoryArea->Attributes;
@@ -422,7 +422,7 @@
    else
    {
       /* FIXME: Should we return failure or success in this case? */
-      Status = STATUS_SUCCESS;
+      Status = STATUS_CONFLICTING_ADDRESSES;
    }
 
    MmUnlockAddressSpace(AddressSpace);
@@ -480,6 +480,13 @@
        NumberOfBytesToProtect = *UnsafeNumberOfBytesToProtect;
      }
 
+   if ((ULONG_PTR)BaseAddress + NumberOfBytesToProtect - 1 <
(ULONG_PTR)BaseAddress ||
+       (ULONG_PTR)BaseAddress + NumberOfBytesToProtect - 1 >=
MmUserProbeAddress)
+     {
+       /* Don't allow to change the protection of a kernel mode address
*/
+       return STATUS_INVALID_PARAMETER_2;
+     }
+
    /* (tMk 2004.II.5) in Microsoft SDK I read:
     * 'if this parameter is NULL or does not point to a valid variable,
the function fails'
     */
@@ -554,15 +561,46 @@
 
    PAGED_CODE();
 
+   DPRINT("NtReadVirtualMemory(ProcessHandle %x, BaseAddress %x, "
+          "Buffer %x, NumberOfBytesToRead
%d)\n",ProcessHandle,BaseAddress,
+          Buffer,NumberOfBytesToRead);
+
+   if ((ULONG_PTR)BaseAddress + NumberOfBytesToRead - 1 <
(ULONG_PTR)BaseAddress ||
+       (ULONG_PTR)BaseAddress + NumberOfBytesToRead - 1 >=
MmUserProbeAddress)
+     {
+       /* Don't allow to read from kernel space */
+       return STATUS_ACCESS_VIOLATION;
+     }
+
    PreviousMode = ExGetPreviousMode();
 
+   if (PreviousMode != KernelMode)
+     {
+       if ((ULONG_PTR)Buffer + NumberOfBytesToRead - 1 <
(ULONG_PTR)Buffer ||
+           (ULONG_PTR)Buffer + NumberOfBytesToRead - 1 >=
MmUserProbeAddress)
+         {
+           /* Don't allow to write into kernel space */
+           return STATUS_ACCESS_VIOLATION;
+         }
+     }
+
+   Status = ObReferenceObjectByHandle(ProcessHandle,
+                                      PROCESS_VM_READ,
+                                      NULL,
+                                      PreviousMode,
+                                      (PVOID*)(&Process),
+                                      NULL);
+   if (!NT_SUCCESS(Status))
+   {
+      return(Status);
+   }
+
+   CurrentProcess = PsGetCurrentProcess();
+
    if(PreviousMode != KernelMode)
    {
      _SEH_TRY
      {
-       ProbeForWrite(Buffer,
-                     NumberOfBytesToRead,
-                     1);
        if(NumberOfBytesRead != NULL)
        {
          ProbeForWriteUlong(NumberOfBytesRead);
@@ -580,23 +618,7 @@
      }
    }
 
-   DPRINT("NtReadVirtualMemory(ProcessHandle %x, BaseAddress %x, "
-          "Buffer %x, NumberOfBytesToRead
%d)\n",ProcessHandle,BaseAddress,
-          Buffer,NumberOfBytesToRead);
 
-   Status = ObReferenceObjectByHandle(ProcessHandle,
-                                      PROCESS_VM_READ,
-                                      NULL,
-                                      PreviousMode,
-                                      (PVOID*)(&Process),
-                                      NULL);
-   if (!NT_SUCCESS(Status))
-   {
-      return(Status);
-   }
-
-   CurrentProcess = PsGetCurrentProcess();
-
    if (Process == CurrentProcess)
    {
       _SEH_TRY
@@ -639,7 +661,6 @@
 
           Status = STATUS_SUCCESS;
           _SEH_TRY {
-              ProbeForRead(BaseAddress, NumberOfBytesToRead, 1);
               Status = STATUS_PARTIAL_COPY;
               RtlCopyMemory(SystemAddress, BaseAddress,
NumberOfBytesToRead);
               Status = STATUS_SUCCESS;
@@ -750,9 +771,6 @@
    PMDL Mdl;
    PVOID SystemAddress;
    PEPROCESS Process;
-   ULONG OldProtection = 0;
-   PVOID ProtectBaseAddress;
-   ULONG ProtectNumberOfBytes;
    KPROCESSOR_MODE PreviousMode;
    NTSTATUS CopyStatus, Status = STATUS_SUCCESS;
 
@@ -760,24 +778,40 @@
           "Buffer %x, NumberOfBytesToWrite
%d)\n",ProcessHandle,BaseAddress,
           Buffer,NumberOfBytesToWrite);
 
+   if ((ULONG_PTR)BaseAddress + NumberOfBytesToWrite - 1 <
(ULONG_PTR)BaseAddress ||
+       (ULONG_PTR)BaseAddress + NumberOfBytesToWrite - 1 >=
MmUserProbeAddress)
+     {
+       /* Don't allow to write into kernel space */
+       return STATUS_ACCESS_VIOLATION;
+     }
+
    PreviousMode = ExGetPreviousMode();
    
-   if (PreviousMode != KernelMode && NumberOfBytesWritten != NULL)
+   if (PreviousMode != KernelMode)
      {
-       _SEH_TRY
+       if ((ULONG_PTR)Buffer + NumberOfBytesToWrite - 1 <
(ULONG_PTR)Buffer ||
+           (ULONG_PTR)Buffer + NumberOfBytesToWrite - 1 >=
MmUserProbeAddress)
          {
-           ProbeForWriteUlong(NumberOfBytesWritten);
+           /* Don't allow to read from kernel space */
+           return STATUS_ACCESS_VIOLATION;
          }
-       _SEH_HANDLE
+       if (NumberOfBytesWritten != NULL)
          {
-           Status = _SEH_GetExceptionCode();
-         }
-       _SEH_END;
+           _SEH_TRY
+             {
+               ProbeForWriteUlong(NumberOfBytesWritten);
+             }
+           _SEH_HANDLE
+             {
+               Status = _SEH_GetExceptionCode();
+             }
+           _SEH_END;
        
-       if (!NT_SUCCESS(Status))
-         {
-           return Status;
-         }
+           if (!NT_SUCCESS(Status))
+             {
+               return Status;
+             }
+          }
      }
 
    Status = ObReferenceObjectByHandle(ProcessHandle,
@@ -791,35 +825,14 @@
       return(Status);
    }
 
-   /* We have to make sure the target memory is writable.
-    *
-    * I am not sure if it is correct to do this in any case, but it has
to be
-    * done at least in some cases because you can use
WriteProcessMemory to
-    * write into the .text section of a module where memcpy() would
crash.
-    *  -blight (2005/01/09)
-    */
-   ProtectBaseAddress = BaseAddress;
-   ProtectNumberOfBytes = NumberOfBytesToWrite;
-   
    CopyStatus = STATUS_SUCCESS;
 
    /* Write memory */
    if (Process == PsGetCurrentProcess())
    {
-      Status = MiProtectVirtualMemory(Process,
-                                      &ProtectBaseAddress,
-                                      &ProtectNumberOfBytes,
-                                      PAGE_READWRITE,
-                                      &OldProtection);
-      if (!NT_SUCCESS(Status))
-      {
-         ObDereferenceObject(Process);
-         return Status;
-      }
-
       if (PreviousMode != KernelMode)
         {
-          _SEH_TRY
+	  _SEH_TRY
             {
               memcpy(BaseAddress, Buffer, NumberOfBytesToWrite);
             }
@@ -841,51 +854,48 @@
                         Buffer,
                         NumberOfBytesToWrite);
       if(Mdl == NULL)
-      {
-         ObDereferenceObject(Process);
-         return(STATUS_NO_MEMORY);
-      }
+        {
+          ObDereferenceObject(Process);
+          return(STATUS_NO_MEMORY);
+        }
+      _SEH_TRY
+        {
+          /* Map the MDL. */
+          MmProbeAndLockPages(Mdl,
+                              UserMode,
+                              IoReadAccess);
+	}
+      _SEH_HANDLE
+        {
+	  CopyStatus = _SEH_GetExceptionCode();
+        }
+      _SEH_END;
 
-      /* Make the target area writable. */
-      Status = MiProtectVirtualMemory(Process,
-                                      &ProtectBaseAddress,
-                                      &ProtectNumberOfBytes,
-                                      PAGE_READWRITE,
-                                      &OldProtection);
-      if (!NT_SUCCESS(Status))
-      {
-         ObDereferenceObject(Process);
-         ExFreePool(Mdl);
-         return Status;
-      }
-
-      /* Map the MDL. */
-      MmProbeAndLockPages(Mdl,
-                          UserMode,
-                          IoReadAccess);
-
-      /* Copy memory from the mapped MDL into the target buffer. */
-      KeAttachProcess(&Process->Pcb);
-
-      SystemAddress = MmGetSystemAddressForMdl(Mdl);
-      if (PreviousMode != KernelMode)
+      if (NT_SUCCESS(CopyStatus))
         {
-          _SEH_TRY
+          /* Copy memory from the mapped MDL into the target buffer. */
+          KeAttachProcess(&Process->Pcb);
+
+          SystemAddress = MmGetSystemAddressForMdl(Mdl);
+          if (PreviousMode != KernelMode)
             {
-              memcpy(BaseAddress, SystemAddress, NumberOfBytesToWrite);
+              _SEH_TRY
+                {
+                  memcpy(BaseAddress, SystemAddress,
NumberOfBytesToWrite);
+                }
+              _SEH_HANDLE
+                {
+                  CopyStatus = _SEH_GetExceptionCode();
+                }
+              _SEH_END;
             }
-          _SEH_HANDLE
+          else
             {
-              CopyStatus = _SEH_GetExceptionCode();
+              memcpy(BaseAddress, SystemAddress, NumberOfBytesToWrite);
             }
-          _SEH_END;
-        }
-      else
-        {
-          memcpy(BaseAddress, SystemAddress, NumberOfBytesToWrite);
-        }
 
-      KeDetachProcess();
+          KeDetachProcess();
+	}
 
       /* Free the MDL. */
       if (Mdl->MappedSystemVa != NULL)
@@ -895,22 +905,9 @@
       MmUnlockPages(Mdl);
       ExFreePool(Mdl);
    }
-
-   /* Reset the protection of the target memory. */
-   Status = MiProtectVirtualMemory(Process,
-                                   &ProtectBaseAddress,
-                                   &ProtectNumberOfBytes,
-                                   OldProtection,
-                                   &OldProtection);
-   if (!NT_SUCCESS(Status))
-   {
-      DPRINT1("Failed to reset protection of the target memory! (Status
0x%x)\n", Status);
-      /* FIXME: Should we bugcheck here? */
-   }
-
    ObDereferenceObject(Process);
 
-   if (NumberOfBytesWritten != NULL)
+   if (NT_SUCCESS(CopyStatus) && NumberOfBytesWritten != NULL)
      {
        if (PreviousMode != KernelMode)
          {
@@ -1030,7 +1027,7 @@
       ExRaiseStatus (STATUS_DATATYPE_MISALIGNMENT);
    }
 
-   Last = (PCHAR)((ULONG_PTR)Address + Length - 1);
+   Last = (CHAR*)((ULONG_PTR)Address + Length - 1);
    if ((ULONG_PTR)Last < (ULONG_PTR)Address ||
        (ULONG_PTR)Last >= (ULONG_PTR)MmUserProbeAddress)
    {
@@ -1039,11 +1036,13 @@
 
    /* Check for accessible pages */
    Current = (CHAR*)Address;
-   do
+   *Current = *Current;
+   Current = (PCHAR)((ULONG_PTR)PAGE_ROUND_DOWN(Current) + PAGE_SIZE);
+   while (Current <= Last)
    {
      *Current = *Current;
      Current = (CHAR*)((ULONG_PTR)Current + PAGE_SIZE);
-   } while (Current <= Last);
+   } 
 }
 
 /* EOF */