[PATCH 0/1] MR5375: server: Avoid removing thread twice from its desktop thread list.

[Rémi Bernon]

When a process is made a system process, `release_thread_desktop( thread, 0 )` is called and we remove the process threads from their desktop thread list, as well as decrementing the desktop user count. Later, when the process gets killed on shutdown, `release_thread_desktop( thread, 1 )` is called, and we remove the thread list node again.

This is 1) wrong, we only want to decrement the desktop user count in this case, and 2) causes an invalid write and corrupts the list the second time we remove the entry.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5375

[PATCH 1/1] server: Avoid removing thread twice from its desktop thread list.

[Rémi Bernon]

From: Rémi Bernon <rbernon(a)codeweavers.com>

---
 server/winstation.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/server/winstation.c b/server/winstation.c
index 3af4c365ec3..80126ad5d60 100644
--- a/server/winstation.c
+++ b/server/winstation.c
@@ -403,21 +403,24 @@ static void add_desktop_thread( struct desktop *desktop, struct thread *thread )
 }
 
 /* remove a user of the desktop and start the close timeout if necessary */
-static void remove_desktop_thread( struct desktop *desktop, struct thread *thread )
+static void remove_desktop_user( struct desktop *desktop, struct thread *thread )
 {
     struct process *process;
 
-    list_remove( &thread->desktop_entry );
+    assert( desktop->users > 0 );
+    desktop->users--;
 
-    if (!thread->process->is_system)
-    {
-        assert( desktop->users > 0 );
-        desktop->users--;
+    /* if we have one remaining user, it has to be the manager of the desktop window */
+    if ((process = get_top_window_owner( desktop )) && desktop->users == process->running_threads && !desktop->close_timeout)
+        desktop->close_timeout = add_timeout_user( -TICKS_PER_SEC, close_desktop_timeout, desktop );
+}
 
-        /* if we have one remaining user, it has to be the manager of the desktop window */
-        if ((process = get_top_window_owner( desktop )) && desktop->users == process->running_threads && !desktop->close_timeout)
-            desktop->close_timeout = add_timeout_user( -TICKS_PER_SEC, close_desktop_timeout, desktop );
-    }
+/* remove a thread from the list of threads attached to a desktop */
+static void remove_desktop_thread( struct desktop *desktop, struct thread *thread )
+{
+    list_remove( &thread->desktop_entry );
+
+    if (!thread->process->is_system) remove_desktop_user( desktop, thread );
 
     if (desktop == desktop->winstation->input_desktop)
     {
@@ -546,7 +549,8 @@ void release_thread_desktop( struct thread *thread, int close )
     if (!(desktop = get_desktop_obj( thread->process, handle, 0 ))) clear_error();  /* ignore errors */
     else
     {
-        remove_desktop_thread( desktop, thread );
+        if (close) remove_desktop_thread( desktop, thread );
+        else remove_desktop_user( desktop, thread );
         release_object( desktop );
     }
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5375