From 458a26ab76232b035a4a5989e17c431727c6ca5b Mon Sep 17 00:00:00 2001 From: Andrew Boyarshin Date: Mon, 26 Nov 2018 19:49:15 +0700 Subject: [PATCH] [WIN32SS:NTUSER] Use the 2nd parameter of NtUserGetThreadDesktop() as fallback. (#1065) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - The 2nd parameter is already passed in user-mode by GetThreadDesktop(). It is then used by NtUserGetThreadDesktop() as a fallback for console threads. - Lookup and validate the thread by using the IntTID2PTI() helper. - Don't reference the desktop with too many access rights. - Get rid of the old-school DECLARE_RETURN() & co. macros. Co-authored-by: Hermès Bélusca-Maïto --- win32ss/include/ntuser.h | 2 +- win32ss/user/ntuser/desktop.c | 114 ++++++++++++++++++----------- win32ss/user/user32/misc/desktop.c | 2 +- 3 files changed, 74 insertions(+), 44 deletions(-) diff --git a/win32ss/include/ntuser.h b/win32ss/include/ntuser.h index 68f37ebff8f..81a0b914409 100644 --- a/win32ss/include/ntuser.h +++ b/win32ss/include/ntuser.h @@ -2439,7 +2439,7 @@ HDESK NTAPI NtUserGetThreadDesktop( DWORD dwThreadId, - DWORD Unknown1); + HDESK hConsoleDesktop); enum ThreadStateRoutines { diff --git a/win32ss/user/ntuser/desktop.c b/win32ss/user/ntuser/desktop.c index 875cc4b69c5..f1aec1b5af7 100644 --- a/win32ss/user/ntuser/desktop.c +++ b/win32ss/user/ntuser/desktop.c @@ -2981,14 +2981,14 @@ CLEANUP: */ HDESK APIENTRY -NtUserGetThreadDesktop(DWORD dwThreadId, DWORD Unknown1) +NtUserGetThreadDesktop(DWORD dwThreadId, HDESK hConsoleDesktop) { + HDESK hDesk; NTSTATUS Status; - PETHREAD Thread; + PTHREADINFO pti; + PEPROCESS Process; PDESKTOP DesktopObject; - HDESK hDesk, hThreadDesktop; OBJECT_HANDLE_INFORMATION HandleInformation; - DECLARE_RETURN(HDESK); UserEnterExclusive(); TRACE("Enter NtUserGetThreadDesktop\n"); @@ -2996,67 +2996,97 @@ NtUserGetThreadDesktop(DWORD dwThreadId, DWORD Unknown1) if (!dwThreadId) { EngSetLastError(ERROR_INVALID_PARAMETER); - RETURN(0); + hDesk = NULL; + goto Quit; } - Status = PsLookupThreadByThreadId((HANDLE)(DWORD_PTR)dwThreadId, &Thread); - if (!NT_SUCCESS(Status)) + /* Validate the Win32 thread and retrieve its information */ + pti = IntTID2PTI(UlongToHandle(dwThreadId)); + if (pti) + { + /* Get the desktop handle of the thread */ + hDesk = pti->hdesk; + Process = pti->ppi->peProcess; + } + else if (hConsoleDesktop) + { + /* + * The thread may belong to a console, so attempt to use the provided + * console desktop handle as a fallback. Otherwise this means that the + * thread is either not Win32 or invalid. + */ + hDesk = hConsoleDesktop; + Process = gpepCSRSS; + } + else { EngSetLastError(ERROR_INVALID_PARAMETER); - RETURN(0); + hDesk = NULL; + goto Quit; } - if (Thread->ThreadsProcess == PsGetCurrentProcess()) + if (!hDesk) { - /* Just return the handle, we queried the desktop handle of a thread running - in the same context */ - hDesk = ((PTHREADINFO)Thread->Tcb.Win32Thread)->hdesk; - ObDereferenceObject(Thread); - RETURN(hDesk); + ERR("Desktop information of thread 0x%x broken!?\n", dwThreadId); + goto Quit; } - /* Get the desktop handle and the desktop of the thread */ - if (!(hThreadDesktop = ((PTHREADINFO)Thread->Tcb.Win32Thread)->hdesk) || - !(DesktopObject = ((PTHREADINFO)Thread->Tcb.Win32Thread)->rpdesk)) + if (Process == PsGetCurrentProcess()) { - ObDereferenceObject(Thread); - ERR("Desktop information of thread 0x%x broken!?\n", dwThreadId); - RETURN(NULL); + /* + * Just return the handle, since we queried the desktop handle + * of a thread running in the same context. + */ + goto Quit; } - /* We could just use DesktopObject instead of looking up the handle, but latter - may be a bit safer (e.g. when the desktop is being destroyed */ - /* Switch into the context of the thread we're trying to get the desktop from, - so we can use the handle */ - KeAttachProcess(&Thread->ThreadsProcess->Pcb); - Status = ObReferenceObjectByHandle(hThreadDesktop, - GENERIC_ALL, + /* + * We could just use the cached rpdesk instead of looking up the handle, + * but it may actually be safer to validate the desktop and get a temporary + * reference to it so that it does not disappear under us (e.g. when the + * desktop is being destroyed) during the operation. + */ + /* + * Switch into the context of the thread we are trying to get + * the desktop from, so we can use the handle. + */ + KeAttachProcess(&Process->Pcb); + Status = ObReferenceObjectByHandle(hDesk, + 0, ExDesktopObjectType, UserMode, (PVOID*)&DesktopObject, &HandleInformation); KeDetachProcess(); - /* The handle couldn't be found, there's nothing to get... */ - if (!NT_SUCCESS(Status)) + if (NT_SUCCESS(Status)) { - ObDereferenceObject(Thread); - RETURN(NULL); - } + /* + * Lookup our handle table if we can find a handle to the desktop object. + * If not, create one. + * QUESTION: Do we really need to create a handle in case it doesn't exist?? + */ + hDesk = IntGetDesktopObjectHandle(DesktopObject); - /* Lookup our handle table if we can find a handle to the desktop object, - if not, create one */ - hDesk = IntGetDesktopObjectHandle(DesktopObject); + /* All done, we got a valid handle to the desktop */ + ObDereferenceObject(DesktopObject); + } + else + { + /* The handle could not be found, there is nothing to get... */ + hDesk = NULL; + } - /* All done, we got a valid handle to the desktop */ - ObDereferenceObject(DesktopObject); - ObDereferenceObject(Thread); - RETURN(hDesk); + if (!hDesk) + { + ERR("Could not retrieve or access desktop for thread 0x%x\n", dwThreadId); + EngSetLastError(ERROR_ACCESS_DENIED); + } -CLEANUP: - TRACE("Leave NtUserGetThreadDesktop, ret=%p\n",_ret_); +Quit: + TRACE("Leave NtUserGetThreadDesktop, hDesk = 0x%p\n", hDesk); UserLeave(); - END_CLEANUP; + return hDesk; } static NTSTATUS diff --git a/win32ss/user/user32/misc/desktop.c b/win32ss/user/user32/misc/desktop.c index bd3ca788263..862ff165034 100644 --- a/win32ss/user/user32/misc/desktop.c +++ b/win32ss/user/user32/misc/desktop.c @@ -563,7 +563,7 @@ GetThreadDesktop( } return NtUserGetThreadDesktop(dwThreadId, - (DWORD_PTR)GetThreadConsoleDesktopRequest->ConsoleDesktop); + GetThreadConsoleDesktopRequest->ConsoleDesktop); } -- 2.17.1