From: Sebastian Lackner Subject: server: Avoid sending unexpected wakeup with uninitialized cookie value. Message-Id: <5470BCEE.9030603@fds-team.de> Date: Sat, 22 Nov 2014 17:42:22 +0100 While executing the kernel32/sync tests I noticed a couple of unexpected wakeup cookies, which looked like uninitialized wineserver memory. Here an excerpt from the log (added additional debug values): --- a/snip 0009: select( flags=2, cookie=0033faac, timeout=0, prev_apc=0000, result={}, data={SIGNAL_AND_WAIT,signal=0038,wait=0038} ) 0009: *wakeup* signaled=0 cookie=0x55555555 0009: select() = 0 { timeout=1d00663fadcfb0a (+0.0000000), call={APC_NONE}, apc_handle=0000 } [...] 0009: select( flags=2, cookie=0033faac, timeout=0, prev_apc=0000, result={}, data={SIGNAL_AND_WAIT,signal=0038,wait=0038} ) 0009: *wakeup* signaled=0 cookie=0x55555555 0009: select() = 0 { timeout=1d00663fadcfdf8 (+0.0000000), call={APC_NONE}, apc_handle=0000 } [...] 0009: select( flags=2, cookie=0033f8dc, timeout=+0.0100000, prev_apc=0000, result={}, data={WAIT,handles={0038}} ) 0009: select() = PENDING { timeout=1d00663fae61280 (+0.0100000), call={APC_NONE}, apc_handle=0000 } 0009:fixme:server:wait_select_reply cookie = 0x55555555 0009:fixme:server:wait_select_reply cookie = 0x55555555 --- a/snip Those unexpected wakeup cookies are never removed again, and could overflow the wakeup pipe sooner or later. The problem is caused when signalling and waiting on the same object. wait_on_handles(...) allocates the current->wait structure, but doesn't store a cookie yet. Afterwards the object is signalled, and wake_thread(...) is executed, which sends a wakeup cookie. This normally isn't necessary because we're still in the same wine server call, and the thread isn't waiting for any reply. --- server/thread.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) From 506534d28e981c06685ea977e1d80672d18b2a35 Mon Sep 17 00:00:00 2001 From: Sebastian Lackner Date: Sat, 22 Nov 2014 15:44:22 +0100 Subject: server: Avoid sending unexpected wakeup with uninitialized cookie value. --- server/thread.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/server/thread.c b/server/thread.c index ba3f1d5..bdd9ef7 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.1.3