[ros-diffs] [hbelusca] 63841: [KERNEL32][CONSRV] For WriteConsoleOutput API only!! Diagnosed with ApiMonitor from Rohitab. If one tries to write a too large buffer (>= 80*198 CHAR_INFO cells), the CsrAllocateC...

Author: hbelusca
Date: Fri Aug  8 17:06:28 2014
New Revision: 63841

URL: http://svn.reactos.org/svn/reactos?rev=63841&view=rev
Log:
[KERNEL32][CONSRV]
For WriteConsoleOutput API only!! Diagnosed with ApiMonitor from Rohitab.

If one tries to write a too large buffer (>= 80*198 CHAR_INFO cells), the
CsrAllocateCaptureBuffer API helper fails, because it uses the non-growable CSR port heap
of fixed size 64kB.
Since many applications use the API with large buffers (for example, Far Manager <=
1.70, the Windows XTree app http://textmode.netne.net/Extreme.html , ...) and maybe
NTVDM,
Windows (and hence ReactOS for compatibility reasons) allocates a buffer in the
process' heap (and not in the CSR port heap via the CSR capture API), so that the big
buffer allocation
should work. Then, to be able to access it, CSR needs to call NtReadVirtualMemory for
capturing the buffer.

CORE-5006 CORE-6397 CORE-8424 #resolve

Modified:
    branches/condrv_restructure/dll/win32/kernel32/client/console/readwrite.c
    branches/condrv_restructure/include/reactos/subsys/win/conmsg.h
    branches/condrv_restructure/win32ss/user/winsrv/consrv/conoutput.c

Modified: branches/condrv_restructure/dll/win32/kernel32/client/console/readwrite.c
URL:
http://svn.reactos.org/svn/reactos/branches/condrv_restructure/dll/win32/ke…
==============================================================================
--- branches/condrv_restructure/dll/win32/kernel32/client/console/readwrite.c	[iso-8859-1]
(original)
+++ branches/condrv_restructure/dll/win32/kernel32/client/console/readwrite.c	[iso-8859-1]
Fri Aug  8 17:06:28 2014
@@ -881,6 +881,7 @@
     {
         WriteOutputRequest->CharInfo = &WriteOutputRequest->StaticBuffer;
         // CaptureBuffer = NULL;
+        WriteOutputRequest->UseVirtualMemory = FALSE;
     }
     else
     {
@@ -888,17 +889,36 @@
 
         /* Allocate a Capture Buffer */
         CaptureBuffer = CsrAllocateCaptureBuffer(1, Size);
-        if (CaptureBuffer == NULL)
-        {
-            DPRINT1("CsrAllocateCaptureBuffer failed with size %ld!\n", Size);
-            SetLastError(ERROR_NOT_ENOUGH_MEMORY);
-            return FALSE;
-        }
-
-        /* Allocate space in the Buffer */
-        CsrAllocateMessagePointer(CaptureBuffer,
-                                  Size,
-                                  (PVOID*)&WriteOutputRequest->CharInfo);
+        if (CaptureBuffer)
+        {
+            /* Allocate space in the Buffer */
+            CsrAllocateMessagePointer(CaptureBuffer,
+                                      Size,
+                                      (PVOID*)&WriteOutputRequest->CharInfo);
+            WriteOutputRequest->UseVirtualMemory = FALSE;
+        }
+        else
+        {
+            /*
+             * CsrAllocateCaptureBuffer failed because we tried to allocate
+             * a too large (>= 64 kB, size of the CSR heap) data buffer.
+             * To circumvent this, Windows uses a trick (that we reproduce for
+             * compatibility reasons): we allocate a heap buffer in the process'
+             * memory, and CSR will read it via NtReadVirtualMemory.
+             */
+            DPRINT1("CsrAllocateCaptureBuffer failed with size %ld, let's use
local heap buffer...\n", Size);
+
+            WriteOutputRequest->CharInfo = RtlAllocateHeap(RtlGetProcessHeap(), 0,
Size);
+            WriteOutputRequest->UseVirtualMemory = TRUE;
+
+            /* Bail out if we still cannot allocate memory */
+            if (WriteOutputRequest->CharInfo == NULL)
+            {
+                DPRINT1("Failed to allocate heap buffer with size %ld!\n",
Size);
+                SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+                return FALSE;
+            }
+        }
     }
 
     /* Capture the user buffer contents */
@@ -945,7 +965,16 @@
     Success = NT_SUCCESS(ApiMessage.Status);
 
     /* Release the capture buffer if needed */
-    if (CaptureBuffer) CsrFreeCaptureBuffer(CaptureBuffer);
+    if (CaptureBuffer)
+    {
+        CsrFreeCaptureBuffer(CaptureBuffer);
+    }
+    else
+    {
+        /* If we used a heap buffer, free it */
+        if (WriteOutputRequest->UseVirtualMemory)
+            RtlFreeHeap(RtlGetProcessHeap(), 0, WriteOutputRequest->CharInfo);
+    }
 
     /* Retrieve the results */
     _SEH2_TRY

Modified: branches/condrv_restructure/include/reactos/subsys/win/conmsg.h
URL:
http://svn.reactos.org/svn/reactos/branches/condrv_restructure/include/reac…
==============================================================================
--- branches/condrv_restructure/include/reactos/subsys/win/conmsg.h	[iso-8859-1]
(original)
+++ branches/condrv_restructure/include/reactos/subsys/win/conmsg.h	[iso-8859-1] Fri Aug 
8 17:06:28 2014
@@ -544,7 +544,12 @@
     SMALL_RECT WriteRegion;
     BOOLEAN Unicode;
 
-    ULONG Unknown;
+    /*
+     * If we are going to write too large (>= 64 kB, size of the CSR heap)
+     * data buffers, we allocate a heap buffer in the process' memory, and
+     * CSR will read it via NtReadVirtualMemory.
+     */
+    BOOLEAN UseVirtualMemory;
 } CONSOLE_WRITEOUTPUT, *PCONSOLE_WRITEOUTPUT;
 
 typedef struct

Modified: branches/condrv_restructure/win32ss/user/winsrv/consrv/conoutput.c
URL:
http://svn.reactos.org/svn/reactos/branches/condrv_restructure/win32ss/user…
==============================================================================
--- branches/condrv_restructure/win32ss/user/winsrv/consrv/conoutput.c	[iso-8859-1]
(original)
+++ branches/condrv_restructure/win32ss/user/winsrv/consrv/conoutput.c	[iso-8859-1] Fri
Aug  8 17:06:28 2014
@@ -463,15 +463,15 @@
 
     DPRINT("SrvReadConsoleOutput\n");
 
+    NumCells = (ReadOutputRequest->ReadRegion.Right -
ReadOutputRequest->ReadRegion.Left + 1) *
+               (ReadOutputRequest->ReadRegion.Bottom -
ReadOutputRequest->ReadRegion.Top + 1);
+
     /*
      * For optimization purposes, Windows (and hence ReactOS, too, for
      * compatibility reasons) uses a static buffer if no more than one
      * cell is read. Otherwise a new buffer is used.
      * The client-side expects that we know this behaviour.
      */
-    NumCells = (ReadOutputRequest->ReadRegion.Right -
ReadOutputRequest->ReadRegion.Left + 1) *
-               (ReadOutputRequest->ReadRegion.Bottom -
ReadOutputRequest->ReadRegion.Top + 1);
-
     if (NumCells <= 1)
     {
         /*
@@ -520,47 +520,83 @@
     NTSTATUS Status;
     PCONSOLE_WRITEOUTPUT WriteOutputRequest =
&((PCONSOLE_API_MESSAGE)ApiMessage)->Data.WriteOutputRequest;
     PTEXTMODE_SCREEN_BUFFER Buffer;
+    PCSR_PROCESS Process = CsrGetClientThread()->Process;
 
     ULONG NumCells;
     PCHAR_INFO CharInfo;
 
     DPRINT("SrvWriteConsoleOutput\n");
 
-    /*
-     * For optimization purposes, Windows (and hence ReactOS, too, for
-     * compatibility reasons) uses a static buffer if no more than one
-     * cell is written. Otherwise a new buffer is used.
-     * The client-side expects that we know this behaviour.
-     */
     NumCells = (WriteOutputRequest->WriteRegion.Right -
WriteOutputRequest->WriteRegion.Left + 1) *
                (WriteOutputRequest->WriteRegion.Bottom -
WriteOutputRequest->WriteRegion.Top + 1);
 
-    if (NumCells <= 1)
-    {
-        /*
-         * Adjust the internal pointer, because its old value points to
-         * the static buffer in the original ApiMessage structure.
-         */
-        // WriteOutputRequest->CharInfo = &WriteOutputRequest->StaticBuffer;
-        CharInfo = &WriteOutputRequest->StaticBuffer;
-    }
-    else
-    {
-        if (!CsrValidateMessageBuffer(ApiMessage,
-                                      (PVOID*)&WriteOutputRequest->CharInfo,
-                                      NumCells,
-                                      sizeof(CHAR_INFO)))
-        {
-            return STATUS_INVALID_PARAMETER;
-        }
-
-        CharInfo = WriteOutputRequest->CharInfo;
-    }
-
-    Status =
ConSrvGetTextModeBuffer(ConsoleGetPerProcessData(CsrGetClientThread()->Process),
+    Status = ConSrvGetTextModeBuffer(ConsoleGetPerProcessData(Process),
                                      WriteOutputRequest->OutputHandle,
                                      &Buffer, GENERIC_WRITE, TRUE);
     if (!NT_SUCCESS(Status)) return Status;
+
+    /*
+     * Validate the message buffer if we do not use a process' heap buffer
+     * (CsrAllocateCaptureBuffer succeeded because we haven't allocated
+     * a too large (>= 64 kB, size of the CSR heap) data buffer).
+     */
+    if (!WriteOutputRequest->UseVirtualMemory)
+    {
+        /*
+         * For optimization purposes, Windows (and hence ReactOS, too, for
+         * compatibility reasons) uses a static buffer if no more than one
+         * cell is written. Otherwise a new buffer is used.
+         * The client-side expects that we know this behaviour.
+         */
+        if (NumCells <= 1)
+        {
+            /*
+             * Adjust the internal pointer, because its old value points to
+             * the static buffer in the original ApiMessage structure.
+             */
+            // WriteOutputRequest->CharInfo =
&WriteOutputRequest->StaticBuffer;
+            CharInfo = &WriteOutputRequest->StaticBuffer;
+        }
+        else
+        {
+            if (!CsrValidateMessageBuffer(ApiMessage,
+                                          (PVOID*)&WriteOutputRequest->CharInfo,
+                                          NumCells,
+                                          sizeof(CHAR_INFO)))
+            {
+                Status = STATUS_INVALID_PARAMETER;
+                goto Quit;
+            }
+
+            CharInfo = WriteOutputRequest->CharInfo;
+        }
+    }
+    else
+    {
+        /*
+         * This was not the case: we use a heap buffer. Retrieve its contents.
+         */
+        ULONG Size = NumCells * sizeof(CHAR_INFO);
+
+        CharInfo = ConsoleAllocHeap(HEAP_ZERO_MEMORY, Size);
+        if (CharInfo == NULL)
+        {
+            Status = STATUS_NO_MEMORY;
+            goto Quit;
+        }
+
+        Status = NtReadVirtualMemory(Process->ProcessHandle,
+                                     WriteOutputRequest->CharInfo,
+                                     CharInfo,
+                                     Size,
+                                     NULL);
+        if (!NT_SUCCESS(Status))
+        {
+            ConsoleFreeHeap(CharInfo);
+            // Status = STATUS_NO_MEMORY;
+            goto Quit;
+        }
+    }
 
     Status = ConDrvWriteConsoleOutput(Buffer->Header.Console,
                                       Buffer,
@@ -568,6 +604,11 @@
                                       CharInfo,
                                       &WriteOutputRequest->WriteRegion);
 
+    /* Free the temporary buffer if we used the process' heap buffer */
+    if (WriteOutputRequest->UseVirtualMemory && CharInfo)
+        ConsoleFreeHeap(CharInfo);
+
+Quit:
     ConSrvReleaseScreenBuffer(Buffer, TRUE);
     return Status;
 }