[1/2] server: Avoid sending unexpected wakeup with uninitialized cookie value. (resend)

[Sebastian Lackner]

The code for SELECT_SIGNAL_AND_WAIT in select_on() calls signal_object(). This might
wake up the same thread which is currently in the wineserver call. The value for
current->wait->cookie is initialized at the end of the function, and not defined yet
at this point. This has the effect that an unexpected wakeup cookie is queued in the
client thread, which will never be processed. Here a backtrace of what is happening:

#0  0x0807ed3a in send_thread_wakeup (thread=0x83b5460, cookie=6148914691236517205, signaled=0) at thread.c:697
#1  0x0807ee8c in wake_thread (thread=0x83b5460) at thread.c:727
#2  0x0807f465 in wake_up (obj=0x83e78a0, max=1) at thread.c:887
#3  0x0805644d in set_event (event=0x83e78a0) at event.c:144
#4  0x08056628 in event_signal (obj=0x83e78a0, access=2031619) at event.c:203
#5  0x0807f0f2 in signal_object (handle=56) at thread.c:790
#6  0x0807f252 in select_on (select_op=0xffffcf6c, op_size=12, cookie=3406508, flags=2, timeout=130814054469202480) at thread.c:829
#7  0x08080aa0 in req_select (req=0x83b554c, reply=0xffffd0bc) at thread.c:1467
#8  0x08077bda in call_req_handler (thread=0x83b5460) at request.c:247
#9  0x08077ddf in read_request (thread=0x83b5460) at request.c:302
#10 0x0807dcb4 in thread_poll_event (fd=0x83b5600, event=1) at thread.c:267
#11 0x080572c0 in fd_poll_event (fd=0x83b5600, event=1) at fd.c:446
#12 0x0805756b in main_loop_epoll () at fd.c:541
#13 0x0805795f in main_loop () at fd.c:886
#14 0x080602e4 in main (argc=3, argv=0xffffd8a4) at main.c:148

Please note that hex(6148914691236517205) == 0x5555555555555555. To solve this issue,
this patch initializes the ->cookie value with zero, which should not be used in select()
anyway. A value of zero means that no cookie is needed, and send_thread_wakeup() is
skipped. The patch has been tested since some time, and no other side effects were found
so far. To reproduce this issue, it is sufficient to run the kernel32/sync tests, and to
enable a debug channel to take a look at the cookie values.

Note: As soon as this is fixed, and only expected wakeup cookies arrive in the target thread,
we can theoretically also get rid of the ugly "put back in the pipe" mechanism, and instead
directly use the memory at *reply.cookie to store the wait result.

---
 server/thread.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/server/thread.c b/server/thread.c
index 6c5d12d..8471651 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -601,6 +601,7 @@ static int wait_on( const select_op_t *select_op, unsigned int count, struct obj
     wait->count   = count;
     wait->flags   = flags;
     wait->select  = select_op->op;
+    wait->cookie  = 0;
     wait->user    = NULL;
     wait->timeout = timeout;
     wait->abandoned = 0;
@@ -719,7 +720,7 @@ int wake_thread( struct thread *thread )
         cookie = thread->wait->cookie;
         if (debug_level) fprintf( stderr, "%04x: *wakeup* signaled=%d\n", thread->id, signaled );
         end_wait( thread );
-        if (send_thread_wakeup( thread, cookie, signaled ) == -1) /* error */
+        if (cookie && send_thread_wakeup( thread, cookie, signaled ) == -1) /* error */
         {
             if (!count) count = -1;
             break;
@@ -749,7 +750,7 @@ int wake_thread_queue_entry( struct wait_queue_entry *entry )
     if (debug_level) fprintf( stderr, "%04x: *wakeup* signaled=%d\n", thread->id, signaled );
     end_wait( thread );
 
-    if (send_thread_wakeup( thread, cookie, signaled ) != -1)
+    if (!cookie || send_thread_wakeup( thread, cookie, signaled ) != -1)
         wake_thread( thread );  /* check other waits too */
 
     return 1;
@@ -768,6 +769,8 @@ static void thread_timeout( void *ptr )
 
     if (debug_level) fprintf( stderr, "%04x: *wakeup* signaled=TIMEOUT\n", thread->id );
     end_wait( thread );
+
+    assert( cookie );
     if (send_thread_wakeup( thread, cookie, STATUS_TIMEOUT ) == -1) return;
     /* check if other objects have become signaled in the meantime */
     wake_thread( thread );
@@ -1429,6 +1432,12 @@ DECL_HANDLER(select)
         set_error( STATUS_INVALID_PARAMETER );
         return;
     }
+    if (!req->cookie)
+    {
+        set_error( STATUS_INVALID_PARAMETER );
+        return;
+    }
+
     op_size = min( get_req_data_size() - sizeof(*result), sizeof(select_op) );
     memset( &select_op, 0, sizeof(select_op) );
     memcpy( &select_op, result + 1, op_size );
-- 
2.4.5