[reactos] 01/01: [WIN32SS:NTUSER] Use the 2nd parameter of NtUserGetThreadDesktop() as fallback. (#1065)

[Andrew Boyarshin]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=458a26ab76232b035a4a59...

commit 458a26ab76232b035a4a5989e17c431727c6ca5b
Author:     Andrew Boyarshin [e-mail]
AuthorDate: Mon Nov 26 19:49:15 2018 +0700
Commit:     Hermès Bélusca-Maïto [e-mail]
CommitDate: Sun Apr 28 19:27:45 2019 +0200

[WIN32SS:NTUSER] Use the 2nd parameter of NtUserGetThreadDesktop() as fallback. (#1065)

- 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 [e-mail]
---
 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 68f37ebff8..81a0b91440 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 875cc4b69c..f1aec1b5af 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 bd3ca78826..862ff16503 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);
 }