Author: hbelusca Date: Sun May 12 00:20:15 2013 New Revision: 59001 URL: http://svn.reactos.org/svn/reactos?rev=59001&view=rev Log: [CONSRV] - Use new helpers ConsoleAllocHeap and ConsoleFreeHeap instead of RtlAllocateHeap and RtlFreeHeap, to be able to use also debug heaps etc... - Initialize completely CONSOLE and CONSOLE_SCREEN_BUFFER structures to avoid using unitializing fields, that can be prone to bugs. - Do not unnecessarily lock the per-process IO handles table. - Validate the console pointers when calling SrvCloseHandle / SrvVerifyConsoleIoHandle / SrvDuplicateHandle (so that we are sure to do valid operations for console applications, and fail properly for GUI ones).
- Fix a bug caught by [TheFlash] which unveiled only on livecds (why??) as random CSR heap corruptions. It was caused by corrupting the heap by freeing invalid IO handles when freeing the handles table inherited by default when starting a console application. I reworked the initialization code a bit and the freeing code to fix this problem. This bug was diagnosed with the help of Thomas Faber's debug heap (see http://www.reactos.org/wiki/User:ThFabba/Debug_Heap ), that I've augmented a bit (with dumping functions when a heap block was found invalid). - Now reenable the CSR heap usage. CORE-7157 #resolve #comment Resolved in revision 59001. Thanks for your help :D Added: trunk/reactos/win32ss/user/consrv/heap.h (with props) Modified: trunk/reactos/win32ss/user/consrv/alias.c trunk/reactos/win32ss/user/consrv/coninput.c trunk/reactos/win32ss/user/consrv/conoutput.c trunk/reactos/win32ss/user/consrv/console.c trunk/reactos/win32ss/user/consrv/consrv.h trunk/reactos/win32ss/user/consrv/frontends/gui/guiterm.c trunk/reactos/win32ss/user/consrv/frontends/tui/tuiterm.c trunk/reactos/win32ss/user/consrv/handle.c trunk/reactos/win32ss/user/consrv/handle.h trunk/reactos/win32ss/user/consrv/include/conio.h trunk/reactos/win32ss/user/consrv/init.c trunk/reactos/win32ss/user/consrv/lineinput.c trunk/reactos/win32ss/user/consrv/settings.c Modified: trunk/reactos/win32ss/user/consrv/alias.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/alias.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/alias.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/alias.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -57,7 +57,7 @@ PALIAS_HEADER Entry; UINT dwLength = wcslen(lpExeName) + 1; - Entry = RtlAllocateHeap(ConSrvHeap, 0, sizeof(ALIAS_HEADER) + sizeof(WCHAR) * dwLength); + Entry = ConsoleAllocHeap(0, sizeof(ALIAS_HEADER) + sizeof(WCHAR) * dwLength); if (!Entry) return Entry; Entry->lpExeName = (LPCWSTR)(Entry + 1); @@ -134,7 +134,7 @@ dwSource = wcslen(lpSource) + 1; dwTarget = wcslen(lpTarget) + 1; - Entry = RtlAllocateHeap(ConSrvHeap, 0, sizeof(ALIAS_ENTRY) + sizeof(WCHAR) * (dwSource + dwTarget)); + Entry = ConsoleAllocHeap(0, sizeof(ALIAS_ENTRY) + sizeof(WCHAR) * (dwSource + dwTarget)); if (!Entry) return Entry; Entry->lpSource = (LPCWSTR)(Entry + 1); @@ -247,7 +247,7 @@ if (CurEntry == Entry) { *LastLink = Entry->Next; - RtlFreeHeap(ConSrvHeap, 0, Entry); + ConsoleFreeHeap(Entry); return; } LastLink = &CurEntry->Next; @@ -266,9 +266,9 @@ for (Entry = Header->Data; Entry; Entry = NextEntry) { NextEntry = Entry->Next; - RtlFreeHeap(ConSrvHeap, 0, Entry); - } - RtlFreeHeap(ConSrvHeap, 0, Header); + ConsoleFreeHeap(Entry); + } + ConsoleFreeHeap(Header); } } Modified: trunk/reactos/win32ss/user/consrv/coninput.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/coninput.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/coninput.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/coninput.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -100,7 +100,7 @@ } /* add event to the queue */ - ConInRec = RtlAllocateHeap(ConSrvHeap, 0, sizeof(ConsoleInput)); + ConInRec = ConsoleAllocHeap(0, sizeof(ConsoleInput)); if (ConInRec == NULL) return STATUS_INSUFFICIENT_RESOURCES; ConInRec->InputEvent = *InputEvent; @@ -129,7 +129,7 @@ { CurrentEntry = RemoveHeadList(&Console->InputBuffer.InputEvents); Event = CONTAINING_RECORD(CurrentEntry, ConsoleInput, ListEntry); - RtlFreeHeap(ConSrvHeap, 0, Event); + ConsoleFreeHeap(Event); } CloseHandle(Console->InputBuffer.ActiveEvent); @@ -324,7 +324,7 @@ { PGET_INPUT_INFO CapturedInputInfo; - CapturedInputInfo = RtlAllocateHeap(ConSrvHeap, 0, sizeof(GET_INPUT_INFO)); + CapturedInputInfo = ConsoleAllocHeap(0, sizeof(GET_INPUT_INFO)); if (!CapturedInputInfo) return STATUS_NO_MEMORY; RtlMoveMemory(CapturedInputInfo, InputInfo, sizeof(GET_INPUT_INFO)); @@ -336,7 +336,7 @@ CapturedInputInfo, NULL)) { - RtlFreeHeap(ConSrvHeap, 0, CapturedInputInfo); + ConsoleFreeHeap(CapturedInputInfo); return STATUS_NO_MEMORY; } } @@ -407,7 +407,7 @@ if (Status != STATUS_PENDING) { WaitApiMessage->Status = Status; - RtlFreeHeap(ConSrvHeap, 0, InputInfo); + ConsoleFreeHeap(InputInfo); } return (Status == STATUS_PENDING ? FALSE : TRUE); @@ -466,7 +466,7 @@ if (Wait) // TRUE --> Read, we remove inputs from the buffer ; FALSE --> Peek, we keep inputs. { RemoveEntryList(&Input->ListEntry); - RtlFreeHeap(ConSrvHeap, 0, Input); + ConsoleFreeHeap(Input); } } @@ -539,7 +539,7 @@ if (Status != STATUS_PENDING) { WaitApiMessage->Status = Status; - RtlFreeHeap(ConSrvHeap, 0, InputInfo); + ConsoleFreeHeap(InputInfo); } return (Status == STATUS_PENDING ? FALSE : TRUE); @@ -569,7 +569,7 @@ { /* Starting a new line */ Console->LineMaxSize = (WORD)max(256, nNumberOfCharsToRead); - Console->LineBuffer = RtlAllocateHeap(ConSrvHeap, 0, Console->LineMaxSize * sizeof(WCHAR)); + Console->LineBuffer = ConsoleAllocHeap(0, Console->LineMaxSize * sizeof(WCHAR)); if (Console->LineBuffer == NULL) { return STATUS_NO_MEMORY; @@ -612,7 +612,7 @@ LineInputKeyDown(Console, &Input->InputEvent.Event.KeyEvent); ReadConsoleRequest->ControlKeyState = Input->InputEvent.Event.KeyEvent.dwControlKeyState; } - RtlFreeHeap(ConSrvHeap, 0, Input); + ConsoleFreeHeap(Input); } /* Check if we have a complete line to read from */ @@ -640,7 +640,7 @@ if (Console->LinePos == Console->LineSize) { /* Entire line has been read */ - RtlFreeHeap(ConSrvHeap, 0, Console->LineBuffer); + ConsoleFreeHeap(Console->LineBuffer); Console->LineBuffer = NULL; } @@ -684,7 +684,7 @@ /* Did read something */ WaitForMoreToRead = FALSE; } - RtlFreeHeap(ConSrvHeap, 0, Input); + ConsoleFreeHeap(Input); } } @@ -862,7 +862,7 @@ { CurrentEntry = RemoveHeadList(&InputBuffer->InputEvents); Event = CONTAINING_RECORD(CurrentEntry, ConsoleInput, ListEntry); - RtlFreeHeap(ConSrvHeap, 0, Event); + ConsoleFreeHeap(Event); } ResetEvent(InputBuffer->ActiveEvent); Modified: trunk/reactos/win32ss/user/consrv/conoutput.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/conoutput.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/conoutput.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/conoutput.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -83,21 +83,19 @@ if (Console == NULL || Buffer == NULL) return STATUS_INVALID_PARAMETER; - *Buffer = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, sizeof(CONSOLE_SCREEN_BUFFER)); + *Buffer = ConsoleAllocHeap(HEAP_ZERO_MEMORY, sizeof(CONSOLE_SCREEN_BUFFER)); if (NULL == *Buffer) { return STATUS_INSUFFICIENT_RESOURCES; } - (*Buffer)->Header.Type = SCREEN_BUFFER; - (*Buffer)->Header.Console = Console; - (*Buffer)->Header.HandleCount = 0; + ConSrvInitObject(&(*Buffer)->Header, SCREEN_BUFFER, Console); (*Buffer)->ScreenBufferSize = ScreenBufferSize; - (*Buffer)->Buffer = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, (*Buffer)->ScreenBufferSize.X * (*Buffer)->ScreenBufferSize.Y * 2); + (*Buffer)->Buffer = ConsoleAllocHeap(HEAP_ZERO_MEMORY, (*Buffer)->ScreenBufferSize.X * (*Buffer)->ScreenBufferSize.Y * 2); if (NULL == (*Buffer)->Buffer) { - RtlFreeHeap(ConSrvHeap, 0, *Buffer); + ConsoleFreeHeap(*Buffer); return STATUS_INSUFFICIENT_RESOURCES; } @@ -105,6 +103,7 @@ (*Buffer)->ShowY = 0; (*Buffer)->VirtualY = 0; + (*Buffer)->CursorBlinkOn = (*Buffer)->ForceCursorOff = FALSE; (*Buffer)->CursorInfo.bVisible = (IsCursorVisible && (CursorSize != 0)); (*Buffer)->CursorInfo.dwSize = min(max(CursorSize, 0), 100); @@ -437,7 +436,7 @@ if (!ConioIsBufferResizeSupported(Console)) return STATUS_NOT_SUPPORTED; - Buffer = RtlAllocateHeap(ConSrvHeap, 0, Size.X * Size.Y * 2); + Buffer = ConsoleAllocHeap(0, Size.X * Size.Y * 2); if (!Buffer) return STATUS_NO_MEMORY; DPRINT1("Resizing (%d,%d) to (%d,%d)\n", ScreenBuffer->ScreenBufferSize.X, ScreenBuffer->ScreenBufferSize.Y, Size.X, Size.Y); @@ -487,7 +486,7 @@ } (void)InterlockedExchangePointer((PVOID volatile*)&ScreenBuffer->Buffer, Buffer); - RtlFreeHeap(ConSrvHeap, 0, OldBuffer); + ConsoleFreeHeap(OldBuffer); ScreenBuffer->ScreenBufferSize = Size; ScreenBuffer->VirtualY = 0; @@ -538,8 +537,8 @@ } } - RtlFreeHeap(ConSrvHeap, 0, Buffer->Buffer); - RtlFreeHeap(ConSrvHeap, 0, Buffer); + ConsoleFreeHeap(Buffer->Buffer); + ConsoleFreeHeap(Buffer); } VOID FASTCALL @@ -986,7 +985,7 @@ * TODO: Do NOT loop up to NumCodesToRead, but stop before * if we are going to overflow... */ - for (i = 0; i < ReadOutputCodeRequest->NumCodesToRead; ++i) + for (i = 0; i < min(ReadOutputCodeRequest->NumCodesToRead, Buff->ScreenBufferSize.X * Buff->ScreenBufferSize.Y * 2); ++i) { Code = Buff->Buffer[2 * (Xpos + Ypos * Buff->ScreenBufferSize.X) + (CodeType == CODE_ATTRIBUTE ? 1 : 0)]; Modified: trunk/reactos/win32ss/user/consrv/console.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/console.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/console.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/console.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -59,7 +59,7 @@ SIZE_T Size = (wcslen(Source) + 1) * sizeof(WCHAR); if (Size > MAXUSHORT) return FALSE; - UniDest->Buffer = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, Size); + UniDest->Buffer = ConsoleAllocHeap(HEAP_ZERO_MEMORY, Size); if (UniDest->Buffer == NULL) return FALSE; RtlCopyMemory(UniDest->Buffer, Source, Size); @@ -75,7 +75,7 @@ { if (UnicodeString->Buffer) { - RtlFreeHeap(ConSrvHeap, 0, UnicodeString->Buffer); + ConsoleFreeHeap(UnicodeString->Buffer); RtlZeroMemory(UnicodeString, sizeof(UNICODE_STRING)); } } @@ -310,7 +310,7 @@ NTSTATUS Status = STATUS_SUCCESS; PCONSOLE ProcessConsole; - RtlEnterCriticalSection(&ProcessData->HandleTableLock); + // RtlEnterCriticalSection(&ProcessData->HandleTableLock); ProcessConsole = ProcessData->Console; if (ConSrvValidateConsole(ProcessConsole, CONSOLE_RUNNING, LockConsole)) @@ -324,7 +324,7 @@ Status = STATUS_INVALID_HANDLE; } - RtlLeaveCriticalSection(&ProcessData->HandleTableLock); + // RtlLeaveCriticalSection(&ProcessData->HandleTableLock); return Status; } @@ -485,7 +485,7 @@ PCONSOLE Console; PCONSOLE_SCREEN_BUFFER NewBuffer; BOOL GuiMode; - WCHAR Title[128]; + WCHAR DefaultTitle[128]; WCHAR IconPath[MAX_PATH + 1] = L""; INT iIcon = 0; @@ -495,7 +495,7 @@ /* * Allocate a console structure */ - Console = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, sizeof(CONSOLE)); + Console = ConsoleAllocHeap(HEAP_ZERO_MEMORY, sizeof(CONSOLE)); if (NULL == Console) { DPRINT1("Not enough memory for console creation.\n"); @@ -576,17 +576,18 @@ * Initialize the console */ Console->State = CONSOLE_INITIALIZING; + Console->ReferenceCount = 0; InitializeCriticalSection(&Console->Lock); - Console->ReferenceCount = 0; InitializeListHead(&Console->ProcessList); + RtlZeroMemory(&Console->TermIFace, sizeof(Console->TermIFace)); + memcpy(Console->Colors, ConsoleInfo.Colors, sizeof(ConsoleInfo.Colors)); Console->ConsoleSize = ConsoleInfo.ConsoleSize; /* * Initialize the input buffer */ - Console->InputBuffer.Header.Type = INPUT_BUFFER; - Console->InputBuffer.Header.Console = Console; + ConSrvInitObject(&Console->InputBuffer.Header, INPUT_BUFFER, Console); SecurityAttributes.nLength = sizeof(SECURITY_ATTRIBUTES); SecurityAttributes.lpSecurityDescriptor = NULL; @@ -595,17 +596,25 @@ if (NULL == Console->InputBuffer.ActiveEvent) { DeleteCriticalSection(&Console->Lock); - RtlFreeHeap(ConSrvHeap, 0, Console); + ConsoleFreeHeap(Console); return STATUS_UNSUCCESSFUL; } + Console->InputBuffer.InputBufferSize = 0; // FIXME! + InitializeListHead(&Console->InputBuffer.InputEvents); + InitializeListHead(&Console->InputBuffer.ReadWaitQueue); Console->InputBuffer.Mode = ENABLE_PROCESSED_INPUT | ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_MOUSE_INPUT; + Console->QuickEdit = ConsoleInfo.QuickEdit; Console->InsertMode = ConsoleInfo.InsertMode; - InitializeListHead(&Console->InputBuffer.ReadWaitQueue); - InitializeListHead(&Console->InputBuffer.InputEvents); Console->LineBuffer = NULL; + Console->LineMaxSize = Console->LineSize = Console->LinePos = 0; + Console->LineComplete = Console->LineUpPressed = Console->LineInsertToggle = FALSE; + // LineWakeupMask + // Selection + // dwSelectionCursor + Console->CodePage = GetOEMCP(); Console->OutputCodePage = GetOEMCP(); @@ -625,16 +634,20 @@ DPRINT1("ConSrvCreateScreenBuffer: failed, Status = 0x%08lx\n", Status); CloseHandle(Console->InputBuffer.ActiveEvent); DeleteCriticalSection(&Console->Lock); - RtlFreeHeap(ConSrvHeap, 0, Console); + ConsoleFreeHeap(Console); return Status; } /* Make the new screen buffer active */ Console->ActiveBuffer = NewBuffer; InitializeListHead(&Console->WriteWaitQueue); - - /* - * Initialize the history buffers - */ + Console->PauseFlags = 0; + Console->UnpauseEvent = NULL; + // HardwareState + + /* + * Initialize the alias and history buffers + */ + Console->Aliases = NULL; InitializeListHead(&Console->HistoryBuffers); Console->HistoryBufferSize = ConsoleInfo.HistoryBufferSize; Console->NumberOfHistoryBuffers = ConsoleInfo.NumberOfHistoryBuffers; @@ -644,9 +657,9 @@ ConsoleCreateUnicodeString(&Console->OriginalTitle, ConsoleInfo.ConsoleTitle); if (ConsoleInfo.ConsoleTitle[0] == L'\0') { - if (LoadStringW(ConSrvDllInstance, IDS_CONSOLE_TITLE, Title, sizeof(Title) / sizeof(Title[0]))) - { - ConsoleCreateUnicodeString(&Console->Title, Title); + if (LoadStringW(ConSrvDllInstance, IDS_CONSOLE_TITLE, DefaultTitle, sizeof(DefaultTitle) / sizeof(DefaultTitle[0]))) + { + ConsoleCreateUnicodeString(&Console->Title, DefaultTitle); } else { @@ -709,7 +722,7 @@ CloseHandle(Console->InputBuffer.ActiveEvent); // LeaveCriticalSection(&Console->Lock); DeleteCriticalSection(&Console->Lock); - RtlFreeHeap(ConSrvHeap, 0, Console); + ConsoleFreeHeap(Console); return Status; } } @@ -827,7 +840,7 @@ /* Discard all entries in the input event queue */ PurgeInputBuffer(Console); - if (Console->LineBuffer) RtlFreeHeap(ConSrvHeap, 0, Console->LineBuffer); + if (Console->LineBuffer) ConsoleFreeHeap(Console->LineBuffer); IntDeleteAllAliases(Console); HistoryDeleteBuffers(Console); @@ -850,7 +863,7 @@ DeleteCriticalSection(&Console->Lock); DPRINT("ConSrvDeleteConsole - Lock destroyed ; freeing console\n"); - RtlFreeHeap(ConSrvHeap, 0, Console); + ConsoleFreeHeap(Console); DPRINT("ConSrvDeleteConsole - Console freed\n"); /* Unlock the console list and return */ @@ -880,18 +893,6 @@ { return STATUS_INVALID_PARAMETER; } - - /* - * We are about to create a new console. However when ConSrvNewProcess - * was called, we didn't know that we wanted to create a new console and - * therefore, we by default inherited the handles table from our parent - * process. It's only now that we notice that in fact we do not need - * them, because we've created a new console and thus we must use it. - * - * Therefore, free the console we can have and our handles table, - * and recreate a new one later on. - */ - ConSrvRemoveConsole(ProcessData); /* Initialize a new Console owned by the Console Leader Process */ Status = ConSrvAllocateConsole(ProcessData, @@ -971,18 +972,6 @@ } /* - * We are about to create a new console. However when ConSrvNewProcess - * was called, we didn't know that we wanted to create a new console and - * therefore, we by default inherited the handles table from our parent - * process. It's only now that we notice that in fact we do not need - * them, because we've created a new console and thus we must use it. - * - * Therefore, free the console we can have and our handles table, - * and recreate a new one later on. - */ - ConSrvRemoveConsole(TargetProcessData); - - /* * Inherit the console from the parent, * if any, otherwise return an error. */ @@ -1212,7 +1201,7 @@ } /* Allocate a new buffer to hold the new title (NULL-terminated) */ - Buffer = RtlAllocateHeap(ConSrvHeap, 0, TitleRequest->Length + sizeof(WCHAR)); + Buffer = ConsoleAllocHeap(0, TitleRequest->Length + sizeof(WCHAR)); if (Buffer) { /* Free the old title */ Modified: trunk/reactos/win32ss/user/consrv/consrv.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/consrv.h?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/consrv.h [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/consrv.h [iso-8859-1] Sun May 12 00:20:15 2013 @@ -45,9 +45,11 @@ #include <win/conmsg.h> +/* Heap Helpers */ +#include "heap.h" + /* Globals */ extern HINSTANCE ConSrvDllInstance; -extern HANDLE ConSrvHeap; #define ConsoleGetPerProcessData(Process) \ ((PCONSOLE_PROCESS_DATA)((Process)->ServerData[CONSRV_SERVERDLL_INDEX])) Modified: trunk/reactos/win32ss/user/consrv/frontends/gui/guiterm.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/frontends/gui/guiterm.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/frontends/gui/guiterm.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/frontends/gui/guiterm.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -233,11 +233,11 @@ GuiConsoleUpdateSelection(Console, &Console->Selection.dwSelectionAnchor); Length = Console->Title.Length + sizeof(L"Mark - ")/sizeof(WCHAR) + 1; - WindowTitle = RtlAllocateHeap(ConSrvHeap, 0, Length * sizeof(WCHAR)); + WindowTitle = ConsoleAllocHeap(0, Length * sizeof(WCHAR)); wcscpy(WindowTitle, L"Mark - "); wcscat(WindowTitle, Console->Title.Buffer); SetWindowText(GuiData->hWindow, WindowTitle); - RtlFreeHeap(ConSrvHeap, 0, WindowTitle); + ConsoleFreeHeap(WindowTitle); break; } @@ -1013,11 +1013,11 @@ GuiConsoleUpdateSelection(Console, &Console->Selection.dwSelectionAnchor); Length = Console->Title.Length + sizeof(L"Selection - ")/sizeof(WCHAR) + 1; - WindowTitle = RtlAllocateHeap(ConSrvHeap, 0, Length * sizeof(WCHAR)); + WindowTitle = ConsoleAllocHeap(0, Length * sizeof(WCHAR)); wcscpy(WindowTitle, L"Selection - "); wcscat(WindowTitle, Console->Title.Buffer); SetWindowText(GuiData->hWindow, WindowTitle); - RtlFreeHeap(ConSrvHeap, 0, WindowTitle); + ConsoleFreeHeap(WindowTitle); break; } @@ -2020,7 +2020,7 @@ Console->TermIFace.Data = NULL; DeleteCriticalSection(&GuiData->Lock); - RtlFreeHeap(ConSrvHeap, 0, GuiData); + ConsoleFreeHeap(GuiData); DPRINT("Quit GuiCleanupConsole\n"); } @@ -2290,8 +2290,7 @@ /* Initialize the console */ Console->TermIFace.Vtbl = &GuiVtbl; - GuiData = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, - sizeof(GUI_CONSOLE_DATA)); + GuiData = ConsoleAllocHeap(HEAP_ZERO_MEMORY, sizeof(GUI_CONSOLE_DATA)); if (!GuiData) { DPRINT1("CONSRV: Failed to create GUI_CONSOLE_DATA\n"); Modified: trunk/reactos/win32ss/user/consrv/frontends/tui/tuiterm.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/frontends/tui/tuiterm.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/frontends/tui/tuiterm.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/frontends/tui/tuiterm.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -68,9 +68,8 @@ /* Build the driver path */ /* 52 = wcslen(L"\\Registry\\Machine\\System\\CurrentControlSet\\Services\\") */ - pszDriverPath = RtlAllocateHeap(ConSrvHeap, - HEAP_ZERO_MEMORY, - (52 + wcslen(lpServiceName) + 1) * sizeof(WCHAR)); + pszDriverPath = ConsoleAllocHeap(HEAP_ZERO_MEMORY, + (52 + wcslen(lpServiceName) + 1) * sizeof(WCHAR)); if (pszDriverPath == NULL) return ERROR_NOT_ENOUGH_MEMORY; @@ -105,7 +104,7 @@ &WasPrivilegeEnabled); done: - RtlFreeHeap(ConSrvHeap, 0, pszDriverPath); + ConsoleFreeHeap(pszDriverPath); return RtlNtStatusToDosError(Status); } @@ -120,9 +119,8 @@ /* Build the driver path */ /* 52 = wcslen(L"\\Registry\\Machine\\System\\CurrentControlSet\\Services\\") */ - pszDriverPath = RtlAllocateHeap(ConSrvHeap, - HEAP_ZERO_MEMORY, - (52 + wcslen(lpServiceName) + 1) * sizeof(WCHAR)); + pszDriverPath = ConsoleAllocHeap(HEAP_ZERO_MEMORY, + (52 + wcslen(lpServiceName) + 1) * sizeof(WCHAR)); if (pszDriverPath == NULL) return ERROR_NOT_ENOUGH_MEMORY; @@ -157,7 +155,7 @@ &WasPrivilegeEnabled); done: - RtlFreeHeap(ConSrvHeap, 0, pszDriverPath); + ConsoleFreeHeap(pszDriverPath); return RtlNtStatusToDosError(Status); } #endif @@ -185,8 +183,7 @@ SwapConsole = (0 < Next ? GetNextConsole(SwapConsole) : GetPrevConsole(SwapConsole)); Title.MaximumLength = RtlUnicodeStringToAnsiSize(&SwapConsole->Console->Title); Title.Length = 0; - Buffer = RtlAllocateHeap(ConSrvHeap, 0, - sizeof(COORD) + Title.MaximumLength); + Buffer = ConsoleAllocHeap(0, sizeof(COORD) + Title.MaximumLength); pos = (PCOORD)Buffer; Title.Buffer = (PVOID)((ULONG_PTR)Buffer + sizeof(COORD)); @@ -201,7 +198,7 @@ { DPRINT1( "Error writing to console\n" ); } - RtlFreeHeap(ConSrvHeap, 0, Buffer); + ConsoleFreeHeap(Buffer); LeaveCriticalSection(&ActiveVirtConsLock); return TRUE; @@ -476,7 +473,7 @@ Console->TermIFace.Data = NULL; DeleteCriticalSection(&TuiData->Lock); - RtlFreeHeap(ConSrvHeap, 0, TuiData); + ConsoleFreeHeap(TuiData); } static VOID WINAPI @@ -506,10 +503,10 @@ ConsoleDrawSize = sizeof(CONSOLE_DRAW) + (ConioRectWidth(Region) * ConioRectHeight(Region)) * 2; - ConsoleDraw = RtlAllocateHeap(ConSrvHeap, 0, ConsoleDrawSize); + ConsoleDraw = ConsoleAllocHeap(0, ConsoleDrawSize); if (NULL == ConsoleDraw) { - DPRINT1("RtlAllocateHeap failed\n"); + DPRINT1("ConsoleAllocHeap failed\n"); return; } ConsoleDraw->X = Region->Left; @@ -525,11 +522,11 @@ NULL, 0, ConsoleDraw, ConsoleDrawSize, &BytesReturned, NULL)) { DPRINT1("Failed to draw console\n"); - RtlFreeHeap(ConSrvHeap, 0, ConsoleDraw); + ConsoleFreeHeap(ConsoleDraw); return; } - RtlFreeHeap(ConSrvHeap, 0, ConsoleDraw); + ConsoleFreeHeap(ConsoleDraw); } static BOOL WINAPI @@ -679,8 +676,7 @@ /* Initialize the console */ Console->TermIFace.Vtbl = &TuiVtbl; - TuiData = RtlAllocateHeap(ConSrvHeap, HEAP_ZERO_MEMORY, - sizeof(TUI_CONSOLE_DATA)); + TuiData = ConsoleAllocHeap(HEAP_ZERO_MEMORY, sizeof(TUI_CONSOLE_DATA)); if (!TuiData) { DPRINT1("CONSRV: Failed to create TUI_CONSOLE_DATA\n"); Modified: trunk/reactos/win32ss/user/consrv/handle.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/handle.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/handle.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/handle.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -40,7 +40,8 @@ { PCONSOLE_IO_OBJECT Object = Entry->Object; - DPRINT("AdjustHandleCounts(0x%p, %d), Object = 0x%p, Object->HandleCount = %d, Object->Type = %lu\n", Entry, Change, Object, Object->HandleCount, Object->Type); + DPRINT("AdjustHandleCounts(0x%p, %d), Object = 0x%p\n", Entry, Change, Object); + DPRINT("\tAdjustHandleCounts(0x%p, %d), Object = 0x%p, Object->HandleCount = %d, Object->Type = %lu\n", Entry, Change, Object, Object->HandleCount, Object->Type); if (Entry->Access & GENERIC_READ) Object->AccessRead += Change; if (Entry->Access & GENERIC_WRITE) Object->AccessWrite += Change; @@ -116,8 +117,12 @@ } /// LOCK /// LeaveCriticalSection(&Console->Lock); - Entry->Object = NULL; - } + + /* Invalidate (zero-out) this handle entry */ + // Entry->Object = NULL; + // RtlZeroMemory(Entry, sizeof(*Entry)); + } + RtlZeroMemory(Entry, sizeof(*Entry)); // Be sure the whole entry is invalidated. } @@ -204,22 +209,21 @@ IN PCONSOLE_PROCESS_DATA TargetProcessData) { NTSTATUS Status = STATUS_SUCCESS; - ULONG i; + ULONG i, j; RtlEnterCriticalSection(&SourceProcessData->HandleTableLock); /* Inherit a handles table only if there is no already */ if (TargetProcessData->HandleTable != NULL /* || TargetProcessData->HandleTableSize != 0 */) { - Status = STATUS_UNSUCCESSFUL; /* STATUS_INVALID_PARAMETER */ + Status = STATUS_UNSUCCESSFUL; goto Quit; } /* Allocate a new handle table for the child process */ - TargetProcessData->HandleTable = RtlAllocateHeap(ConSrvHeap, - HEAP_ZERO_MEMORY, - SourceProcessData->HandleTableSize - * sizeof(CONSOLE_IO_HANDLE)); + TargetProcessData->HandleTable = ConsoleAllocHeap(HEAP_ZERO_MEMORY, + SourceProcessData->HandleTableSize + * sizeof(CONSOLE_IO_HANDLE)); if (TargetProcessData->HandleTable == NULL) { Status = STATUS_NO_MEMORY; @@ -232,7 +236,7 @@ * Parse the parent process' handles table and, for each handle, * do a copy of it and reference it, if the handle is inheritable. */ - for (i = 0; i < SourceProcessData->HandleTableSize; i++) + for (i = 0, j = 0; i < SourceProcessData->HandleTableSize; i++) { if (SourceProcessData->HandleTable[i].Object != NULL && SourceProcessData->HandleTable[i].Inheritable) @@ -241,8 +245,9 @@ * Copy the handle data and increment the reference count of the * pointed object (via the call to ConSrvCreateHandleEntry). */ - TargetProcessData->HandleTable[i] = SourceProcessData->HandleTable[i]; - ConSrvCreateHandleEntry(&TargetProcessData->HandleTable[i]); + TargetProcessData->HandleTable[j] = SourceProcessData->HandleTable[i]; + ConSrvCreateHandleEntry(&TargetProcessData->HandleTable[j]); + ++j; } } @@ -260,18 +265,44 @@ { ULONG i; - /* Close all console handles and free the handle table memory */ - for (i = 0; i < ProcessData->HandleTableSize; i++) + /* + * ProcessData->Console is NULL (and the assertion fails) when + * ConSrvFreeHandlesTable is called in ConSrvConnect during the + * allocation of a new console. + */ + // ASSERT(ProcessData->Console); + if (ProcessData->Console != NULL) { - ConSrvCloseHandleEntry(&ProcessData->HandleTable[i]); + /* Close all the console handles */ + for (i = 0; i < ProcessData->HandleTableSize; i++) + { + ConSrvCloseHandleEntry(&ProcessData->HandleTable[i]); + } } - RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable); + /* Free the handles table memory */ + ConsoleFreeHeap(ProcessData->HandleTable); ProcessData->HandleTable = NULL; } ProcessData->HandleTableSize = 0; RtlLeaveCriticalSection(&ProcessData->HandleTableLock); +} + +VOID +FASTCALL +ConSrvInitObject(IN OUT PCONSOLE_IO_OBJECT Object, + IN CONSOLE_IO_OBJECT_TYPE Type, + IN PCONSOLE Console) +{ + ASSERT(Object); + // if (!Object) return; + + Object->Type = Type; + Object->Console = Console; + Object->AccessRead = Object->AccessWrite = 0; + Object->ExclusiveRead = Object->ExclusiveWrite = 0; + Object->HandleCount = 0; } NTSTATUS @@ -285,34 +316,45 @@ { #define IO_HANDLES_INCREMENT 2 * 3 - ULONG i; + ULONG i = 0; PCONSOLE_IO_HANDLE Block; // NOTE: Commented out because calling code always lock HandleTableLock before. // RtlEnterCriticalSection(&ProcessData->HandleTableLock); - for (i = 0; i < ProcessData->HandleTableSize; i++) - { - if (ProcessData->HandleTable[i].Object == NULL) + ASSERT( (ProcessData->HandleTable == NULL && ProcessData->HandleTableSize == 0) || + (ProcessData->HandleTable != NULL && ProcessData->HandleTableSize != 0) ); + + if (ProcessData->HandleTable) + { + for (i = 0; i < ProcessData->HandleTableSize; i++) { - break; + if (ProcessData->HandleTable[i].Object == NULL) + break; } } + if (i >= ProcessData->HandleTableSize) { - Block = RtlAllocateHeap(ConSrvHeap, - HEAP_ZERO_MEMORY, - (ProcessData->HandleTableSize + + /* Allocate a new handles table */ + Block = ConsoleAllocHeap(HEAP_ZERO_MEMORY, + (ProcessData->HandleTableSize + IO_HANDLES_INCREMENT) * sizeof(CONSOLE_IO_HANDLE)); if (Block == NULL) { // RtlLeaveCriticalSection(&ProcessData->HandleTableLock); return STATUS_UNSUCCESSFUL; } - RtlCopyMemory(Block, - ProcessData->HandleTable, - ProcessData->HandleTableSize * sizeof(CONSOLE_IO_HANDLE)); - RtlFreeHeap(ConSrvHeap, 0, ProcessData->HandleTable); + + /* If we previously had a handles table, free it and use the new one */ + if (ProcessData->HandleTable) + { + /* Copy the handles from the old table to the new one */ + RtlCopyMemory(Block, + ProcessData->HandleTable, + ProcessData->HandleTableSize * sizeof(CONSOLE_IO_HANDLE)); + ConsoleFreeHeap(ProcessData->HandleTable); + } ProcessData->HandleTable = Block; ProcessData->HandleTableSize += IO_HANDLES_INCREMENT; } @@ -339,6 +381,8 @@ RtlEnterCriticalSection(&ProcessData->HandleTableLock); + ASSERT(ProcessData->HandleTable); + if (h >= ProcessData->HandleTableSize || (Object = ProcessData->HandleTable[h].Object) == NULL) { @@ -346,6 +390,7 @@ return STATUS_INVALID_HANDLE; } + ASSERT(ProcessData->Console); ConSrvCloseHandleEntry(&ProcessData->HandleTable[h]); RtlLeaveCriticalSection(&ProcessData->HandleTableLock); @@ -429,6 +474,18 @@ { NTSTATUS Status = STATUS_SUCCESS; + /* + * We are about to create a new console. However when ConSrvNewProcess + * was called, we didn't know that we wanted to create a new console and + * therefore, we by default inherited the handles table from our parent + * process. It's only now that we notice that in fact we do not need + * them, because we've created a new console and thus we must use it. + * + * Therefore, free the handles table so that we can recreate + * a new one later on. + */ + ConSrvFreeHandlesTable(ProcessData); + /* Initialize a new Console owned by this process */ Status = ConSrvInitConsole(&ProcessData->Console, ConsoleStartInfo, ProcessData->Process); if (!NT_SUCCESS(Status)) @@ -500,6 +557,18 @@ if (CreateNewHandlesTable) { + /* + * We are about to create a new console. However when ConSrvNewProcess + * was called, we didn't know that we wanted to create a new console and + * therefore, we by default inherited the handles table from our parent + * process. It's only now that we notice that in fact we do not need + * them, because we've created a new console and thus we must use it. + * + * Therefore, free the handles table so that we can recreate + * a new one later on. + */ + ConSrvFreeHandlesTable(ProcessData); + /* Initialize the handles table */ Status = ConSrvInitHandlesTable(ProcessData, pInputHandle, @@ -552,7 +621,7 @@ DPRINT("ConSrvRemoveConsole\n"); - RtlEnterCriticalSection(&ProcessData->HandleTableLock); + // RtlEnterCriticalSection(&ProcessData->HandleTableLock); /* Validate and lock the console */ if (ConSrvValidateConsole(Console, CONSOLE_RUNNING, TRUE)) @@ -578,7 +647,7 @@ //ProcessData->ConsoleEvent = NULL; } - RtlLeaveCriticalSection(&ProcessData->HandleTableLock); + // RtlLeaveCriticalSection(&ProcessData->HandleTableLock); } @@ -649,19 +718,40 @@ CSR_API(SrvCloseHandle) { + NTSTATUS Status; PCONSOLE_CLOSEHANDLE CloseHandleRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.CloseHandleRequest; - - return ConSrvRemoveObject(ConsoleGetPerProcessData(CsrGetClientThread()->Process), - CloseHandleRequest->ConsoleHandle); + PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(CsrGetClientThread()->Process); + PCONSOLE Console; + + Status = ConSrvGetConsole(ProcessData, &Console, TRUE); + if (!NT_SUCCESS(Status)) + { + DPRINT1("Can't get console\n"); + return Status; + } + + Status = ConSrvRemoveObject(ProcessData, CloseHandleRequest->ConsoleHandle); + + ConSrvReleaseConsole(Console, TRUE); + return Status; } CSR_API(SrvVerifyConsoleIoHandle) { - NTSTATUS Status = STATUS_SUCCESS; + NTSTATUS Status; PCONSOLE_VERIFYHANDLE VerifyHandleRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.VerifyHandleRequest; PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(CsrGetClientThread()->Process); + PCONSOLE Console; + HANDLE ConsoleHandle = VerifyHandleRequest->ConsoleHandle; ULONG_PTR Index = (ULONG_PTR)ConsoleHandle >> 2; + + Status = ConSrvGetConsole(ProcessData, &Console, TRUE); + if (!NT_SUCCESS(Status)) + { + DPRINT1("Can't get console\n"); + return Status; + } RtlEnterCriticalSection(&ProcessData->HandleTableLock); @@ -675,17 +765,28 @@ RtlLeaveCriticalSection(&ProcessData->HandleTableLock); + ConSrvReleaseConsole(Console, TRUE); return Status; } CSR_API(SrvDuplicateHandle) { + NTSTATUS Status; + PCONSOLE_DUPLICATEHANDLE DuplicateHandleRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.DuplicateHandleRequest; + PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(CsrGetClientThread()->Process); + PCONSOLE Console; + + HANDLE ConsoleHandle = DuplicateHandleRequest->ConsoleHandle; + ULONG_PTR Index = (ULONG_PTR)ConsoleHandle >> 2; PCONSOLE_IO_HANDLE Entry; DWORD DesiredAccess; - PCONSOLE_DUPLICATEHANDLE DuplicateHandleRequest = &((PCONSOLE_API_MESSAGE)ApiMessage)->Data.DuplicateHandleRequest; - PCONSOLE_PROCESS_DATA ProcessData = ConsoleGetPerProcessData(CsrGetClientThread()->Process); - HANDLE ConsoleHandle = DuplicateHandleRequest->ConsoleHandle; - ULONG_PTR Index = (ULONG_PTR)ConsoleHandle >> 2; + + Status = ConSrvGetConsole(ProcessData, &Console, TRUE); + if (!NT_SUCCESS(Status)) + { + DPRINT1("Can't get console\n"); + return Status; + } RtlEnterCriticalSection(&ProcessData->HandleTableLock); @@ -694,8 +795,8 @@ (Entry = &ProcessData->HandleTable[Index])->Object == NULL) { DPRINT1("Couldn't duplicate invalid handle %p\n", ConsoleHandle); - RtlLeaveCriticalSection(&ProcessData->HandleTableLock); - return STATUS_INVALID_HANDLE; + Status = STATUS_INVALID_HANDLE; + goto Quit; } if (DuplicateHandleRequest->Options & DUPLICATE_SAME_ACCESS) @@ -709,27 +810,31 @@ if ((Entry->Access & DesiredAccess) == 0) { DPRINT1("Handle %p only has access %X; requested %X\n", - ConsoleHandle, Entry->Access, DesiredAccess); - RtlLeaveCriticalSection(&ProcessData->HandleTableLock); - return STATUS_INVALID_PARAMETER; + ConsoleHandle, Entry->Access, DesiredAccess); + Status = STATUS_INVALID_PARAMETER; + goto Quit; } } /* Insert the new handle inside the process handles table */ - ApiMessage->Status = ConSrvInsertObject(ProcessData, - &DuplicateHandleRequest->ConsoleHandle, // Use the new handle value! - Entry->Object, - DesiredAccess, - DuplicateHandleRequest->Inheritable, - Entry->ShareMode); - if (NT_SUCCESS(ApiMessage->Status) && - DuplicateHandleRequest->Options & DUPLICATE_CLOSE_SOURCE) - { + Status = ConSrvInsertObject(ProcessData, + &DuplicateHandleRequest->ConsoleHandle, // Use the new handle value! + Entry->Object, + DesiredAccess, + DuplicateHandleRequest->Inheritable, + Entry->ShareMode); + if (NT_SUCCESS(Status) && + (DuplicateHandleRequest->Options & DUPLICATE_CLOSE_SOURCE)) + { + /* Close the original handle if needed */ ConSrvCloseHandleEntry(Entry); } +Quit: RtlLeaveCriticalSection(&ProcessData->HandleTableLock); - return ApiMessage->Status; + + ConSrvReleaseConsole(Console, TRUE); + return Status; } /* EOF */ Modified: trunk/reactos/win32ss/user/consrv/handle.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/handle.h?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/handle.h [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/handle.h [iso-8859-1] Sun May 12 00:20:15 2013 @@ -10,6 +10,9 @@ #pragma once +VOID FASTCALL ConSrvInitObject(IN OUT PCONSOLE_IO_OBJECT Object, + IN CONSOLE_IO_OBJECT_TYPE Type, + IN PCONSOLE Console); NTSTATUS FASTCALL ConSrvInsertObject(PCONSOLE_PROCESS_DATA ProcessData, PHANDLE Handle, PCONSOLE_IO_OBJECT Object, Added: trunk/reactos/win32ss/user/consrv/heap.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/heap.h?rev=59001&view=auto ============================================================================== --- trunk/reactos/win32ss/user/consrv/heap.h (added) +++ trunk/reactos/win32ss/user/consrv/heap.h [iso-8859-1] Sun May 12 00:20:15 2013 @@ -0,0 +1,17 @@ +/* + * COPYRIGHT: See COPYING in the top level directory + * PROJECT: ReactOS Console Server DLL + * FILE: win32ss/user/consrv/heap.h + * PURPOSE: Heap Helpers + * PROGRAMMERS: Hermes Belusca-Maito (hermes.belu...@sfr.fr) + */ + +#pragma once + +/* See init.c */ +extern HANDLE ConSrvHeap; + +#define ConsoleAllocHeap(Flags, Size) RtlAllocateHeap(ConSrvHeap, Flags, Size) +#define ConsoleFreeHeap(HeapBase) RtlFreeHeap(ConSrvHeap, 0, HeapBase) + +/* EOF */ Propchange: trunk/reactos/win32ss/user/consrv/heap.h ------------------------------------------------------------------------------ svn:eol-style = native Modified: trunk/reactos/win32ss/user/consrv/include/conio.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/include/conio.h?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/include/conio.h [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/include/conio.h [iso-8859-1] Sun May 12 00:20:15 2013 @@ -64,7 +64,7 @@ BOOLEAN CursorBlinkOn; BOOLEAN ForceCursorOff; - ULONG CursorSize; + // ULONG CursorSize; CONSOLE_CURSOR_INFO CursorInfo; // FIXME: Keep this member or not ?? WORD ScreenDefaultAttrib; /* Default screen char attribute */ Modified: trunk/reactos/win32ss/user/consrv/init.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/init.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/init.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/init.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -11,6 +11,8 @@ #include "consrv.h" #include "api.h" #include "procinit.h" +#include "include/conio.h" +#include "include/console.h" #include "console.h" #define NDEBUG @@ -301,6 +303,7 @@ /* FUNCTIONS ******************************************************************/ +/* See handle.c */ NTSTATUS ConSrvInheritHandlesTable(IN PCONSOLE_PROCESS_DATA SourceProcessData, IN PCONSOLE_PROCESS_DATA TargetProcessData); @@ -322,22 +325,25 @@ * ConSrvConnect we don't have any reference to the parent process anymore. **************************************************************************/ - PCONSOLE_PROCESS_DATA SourceProcessData, TargetProcessData; + NTSTATUS Status = STATUS_SUCCESS; + PCONSOLE_PROCESS_DATA /* SourceProcessData, */ TargetProcessData; /* An empty target process is invalid */ if (!TargetProcess) return STATUS_INVALID_PARAMETER; TargetProcessData = ConsoleGetPerProcessData(TargetProcess); - /**** HACK !!!! ****/ RtlZeroMemory(TargetProcessData, sizeof(*TargetProcessData)); - /* Initialize the new (target) process */ + RtlZeroMemory(TargetProcessData, sizeof(*TargetProcessData)); TargetProcessData->Process = TargetProcess; TargetProcessData->ConsoleEvent = NULL; TargetProcessData->Console = TargetProcessData->ParentConsole = NULL; TargetProcessData->ConsoleApp = ((TargetProcess->Flags & CsrProcessIsConsoleApp) ? TRUE : FALSE); - // Testing + /* + * The handles table gets initialized either when inheriting from + * another console process, or when creating a new console. + */ TargetProcessData->HandleTableSize = 0; TargetProcessData->HandleTable = NULL; @@ -346,25 +352,35 @@ /* Do nothing if the source process is NULL */ if (!SourceProcess) return STATUS_SUCCESS; - SourceProcessData = ConsoleGetPerProcessData(SourceProcess); + // SourceProcessData = ConsoleGetPerProcessData(SourceProcess); /* - * If both of the processes (parent and new child) are console applications, - * then try to inherit handles from the parent process. + * If the child process is a console application and the parent process is + * either a console application or just has a valid console (with a valid + * handles table: this can happen if it is a GUI application having called + * AllocConsole), then try to inherit handles from the parent process. */ - if ( SourceProcessData->Console != NULL && /* SourceProcessData->ConsoleApp */ - TargetProcessData->ConsoleApp ) + if (TargetProcessData->ConsoleApp /* && SourceProcessData->ConsoleApp */) { - NTSTATUS Status; - - Status = ConSrvInheritHandlesTable(SourceProcessData, TargetProcessData); - if (!NT_SUCCESS(Status)) return Status; - - /* Temporary save the parent's console */ - TargetProcessData->ParentConsole = SourceProcessData->Console; + PCONSOLE_PROCESS_DATA SourceProcessData = ConsoleGetPerProcessData(SourceProcess); + + /* Validate and lock the parent's console */ + if (ConSrvValidateConsole(SourceProcessData->Console, CONSOLE_RUNNING, TRUE)) + { + /* Inherit the parent's handles table */ + Status = ConSrvInheritHandlesTable(SourceProcessData, TargetProcessData); + if (NT_SUCCESS(Status)) + { + /* Temporary save the parent's console too */ + TargetProcessData->ParentConsole = SourceProcessData->Console; + } + + /* Unlock the parent's console */ + LeaveCriticalSection(&SourceProcessData->Console->Lock); + } } - return STATUS_SUCCESS; + return Status; } NTSTATUS @@ -408,10 +424,9 @@ * process. It's only now that we notice that in fact we do not need * them, because we've created a new console and thus we must use it. * - * Therefore, free the console we can have and our handles table, - * and recreate a new one later on. + * ConSrvAllocateConsole will free our old handles table + * and recreate a new valid one. */ - ConSrvRemoveConsole(ProcessData); /* Initialize a new Console owned by the Console Leader Process */ Status = ConSrvAllocateConsole(ProcessData, @@ -481,9 +496,9 @@ CSR_SERVER_DLL_INIT(ConServerDllInitialization) { /* Initialize the memory */ - // HACK: To try to uncover a heap corruption in CONSRV, use our own heap - // instead of the CSR heap, so that we won't corrupt it. - // ConSrvHeap = RtlGetProcessHeap(); + ConSrvHeap = RtlGetProcessHeap(); +/* + // We can use our own heap instead of the CSR heap to investigate heap corruptions :) ConSrvHeap = RtlCreateHeap(HEAP_GROWABLE | HEAP_PROTECTION_ENABLED | HEAP_FREE_CHECKING_ENABLED | @@ -491,6 +506,7 @@ HEAP_VALIDATE_ALL_ENABLED, NULL, 0, 0, NULL, NULL); if (!ConSrvHeap) return STATUS_NO_MEMORY; +*/ ConSrvInitConsoleSupport(); Modified: trunk/reactos/win32ss/user/consrv/lineinput.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/lineinput.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/lineinput.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/lineinput.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -45,15 +45,15 @@ } /* Couldn't find the buffer, create a new one */ - Hist = RtlAllocateHeap(ConSrvHeap, 0, sizeof(HISTORY_BUFFER) + ExeName.Length); + Hist = ConsoleAllocHeap(0, sizeof(HISTORY_BUFFER) + ExeName.Length); if (!Hist) return NULL; Hist->MaxEntries = Console->HistoryBufferSize; Hist->NumEntries = 0; - Hist->Entries = RtlAllocateHeap(ConSrvHeap, 0, Hist->MaxEntries * sizeof(UNICODE_STRING)); + Hist->Entries = ConsoleAllocHeap(0, Hist->MaxEntries * sizeof(UNICODE_STRING)); if (!Hist->Entries) { - RtlFreeHeap(ConSrvHeap, 0, Hist); + ConsoleFreeHeap(Hist); return NULL; } Hist->ExeName.Length = Hist->ExeName.MaximumLength = ExeName.Length; @@ -148,9 +148,9 @@ while (Hist->NumEntries != 0) RtlFreeUnicodeString(&Hist->Entries[--Hist->NumEntries]); - RtlFreeHeap(ConSrvHeap, 0, Hist->Entries); + ConsoleFreeHeap(Hist->Entries); RemoveEntryList(&Hist->ListEntry); - RtlFreeHeap(ConSrvHeap, 0, Hist); + ConsoleFreeHeap(Hist); } VOID FASTCALL @@ -577,8 +577,7 @@ if (Hist) { OldEntryList = Hist->Entries; - NewEntryList = RtlAllocateHeap(ConSrvHeap, 0, - MaxEntries * sizeof(UNICODE_STRING)); + NewEntryList = ConsoleAllocHeap(0, MaxEntries * sizeof(UNICODE_STRING)); if (!NewEntryList) { Status = STATUS_NO_MEMORY; @@ -595,7 +594,7 @@ Hist->MaxEntries = MaxEntries; Hist->Entries = memcpy(NewEntryList, Hist->Entries, Hist->NumEntries * sizeof(UNICODE_STRING)); - RtlFreeHeap(ConSrvHeap, 0, OldEntryList); + ConsoleFreeHeap(OldEntryList); } } ConSrvReleaseConsole(Console, TRUE); Modified: trunk/reactos/win32ss/user/consrv/settings.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/consrv/settings.c?rev=59001&r1=59000&r2=59001&view=diff ============================================================================== --- trunk/reactos/win32ss/user/consrv/settings.c [iso-8859-1] (original) +++ trunk/reactos/win32ss/user/consrv/settings.c [iso-8859-1] Sun May 12 00:20:15 2013 @@ -74,20 +74,20 @@ hProcess = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | READ_CONTROL, FALSE, ProcessId); if (!hProcess) { - DPRINT("Error: OpenProcess failed(0x%x)\n", GetLastError()); + DPRINT1("Error: OpenProcess failed(0x%x)\n", GetLastError()); return FALSE; } if (!OpenProcessToken(hProcess, TOKEN_QUERY, &hProcessToken)) { - DPRINT("Error: OpenProcessToken failed(0x%x)\n", GetLastError()); + DPRINT1("Error: OpenProcessToken failed(0x%x)\n", GetLastError()); CloseHandle(hProcess); return FALSE; } if (!GetTokenInformation(hProcessToken, TokenUser, (PVOID)Buffer, sizeof(Buffer), &Length)) { - DPRINT("Error: GetTokenInformation failed(0x%x)\n",GetLastError()); + DPRINT1("Error: GetTokenInformation failed(0x%x)\n",GetLastError()); CloseHandle(hProcessToken); CloseHandle(hProcess); return FALSE; @@ -96,12 +96,15 @@ TokUser = ((PTOKEN_USER)Buffer)->User.Sid; if (!NT_SUCCESS(RtlConvertSidToUnicodeString(&SidName, TokUser, TRUE))) { - DPRINT("Error: RtlConvertSidToUnicodeString failed(0x%x)\n", GetLastError()); + DPRINT1("Error: RtlConvertSidToUnicodeString failed(0x%x)\n", GetLastError()); CloseHandle(hProcessToken); CloseHandle(hProcess); return FALSE; } + /* + * Might fail for LiveCD... Why ? Because only HKU\.DEFAULT exists. + */ bRet = (RegOpenKeyExW(HKEY_USERS, SidName.Buffer, 0,
