From 97afac469fbe012e22acc1f1045c88b1004a241f Mon Sep 17 00:00:00 2001 From: Zebediah Figura Date: Fri, 21 May 2021 22:08:46 -0500 Subject: [PATCH] ntdll: Avoid accessing the I/O status block in wait_async(). Steam uses WSASend() with completion ports, reusing OVERLAPPED structures as soon as they are returned from GetQueuedCompletionStatus(). Since completion is queued during the select request in wait_async(), the I/O status block can be reused even before the call to NtDeviceIoControl exits. This works fine with current Wine, because WSASend() doesn't access the I/O status block after queuing completion. However, a patch that changes it to use wait_async() like other async requests causes NtDeviceIoControlFile to consistently return garbage status codes. Signed-off-by: Zebediah Figura Signed-off-by: Alexandre Julliard --- dlls/ntdll/unix/file.c | 15 +++++++-------- server/async.c | 4 ++-- server/thread.c | 8 ++++++++ server/thread.h | 1 + 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index c033bb06fa3..8313e3c3160 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4697,10 +4697,9 @@ static async_data_t server_async( HANDLE handle, struct async_fileio *user, HAND return async; } -static NTSTATUS wait_async( HANDLE handle, BOOL alertable, IO_STATUS_BLOCK *io ) +static NTSTATUS wait_async( HANDLE handle, BOOL alertable ) { - if (NtWaitForSingleObject( handle, alertable, NULL )) return STATUS_PENDING; - return io->u.Status; + return NtWaitForSingleObject( handle, alertable, NULL ); } /* callback for irp async I/O completion */ @@ -4861,7 +4860,7 @@ static NTSTATUS server_read_file( HANDLE handle, HANDLE event, PIO_APC_ROUTINE a if (status != STATUS_PENDING) free( async ); - if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT), io ); + if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT) ); return status; } @@ -4899,7 +4898,7 @@ static NTSTATUS server_write_file( HANDLE handle, HANDLE event, PIO_APC_ROUTINE if (status != STATUS_PENDING) free( async ); - if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT), io ); + if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT) ); return status; } @@ -4944,7 +4943,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle, HANDLE event, if (status != STATUS_PENDING) free( async ); - if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT), io ); + if (wait_handle) status = wait_async( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT) ); return status; } @@ -5937,7 +5936,7 @@ NTSTATUS WINAPI NtFlushBuffersFile( HANDLE handle, IO_STATUS_BLOCK *io ) if (ret != STATUS_PENDING) free( async ); - if (wait_handle) ret = wait_async( wait_handle, FALSE, io ); + if (wait_handle) ret = wait_async( wait_handle, FALSE ); } if (needs_close) close( fd ); @@ -6436,7 +6435,7 @@ NTSTATUS WINAPI NtQueryVolumeInformationFile( HANDLE handle, IO_STATUS_BLOCK *io } SERVER_END_REQ; if (status != STATUS_PENDING) free( async ); - if (wait_handle) status = wait_async( wait_handle, FALSE, io ); + if (wait_handle) status = wait_async( wait_handle, FALSE ); return status; } else if (io->u.Status) return io->u.Status; diff --git a/server/async.c b/server/async.c index a5fa61dc6e5..27af3be52ff 100644 --- a/server/async.c +++ b/server/async.c @@ -118,14 +118,14 @@ static void async_satisfied( struct object *obj, struct wait_queue_entry *entry async->direct_result = 0; } + set_wait_status( entry, async->status ); + /* close wait handle here to avoid extra server round trip */ if (async->wait_handle) { close_handle( async->thread->process, async->wait_handle ); async->wait_handle = 0; } - - if (async->status == STATUS_PENDING) make_wait_abandoned( entry ); } static void async_destroy( struct object *obj ) diff --git a/server/thread.c b/server/thread.c index eb8b0de84b1..4077a752e4a 100644 --- a/server/thread.c +++ b/server/thread.c @@ -67,6 +67,7 @@ struct thread_wait client_ptr_t cookie; /* magic cookie to return to client */ abstime_t when; struct timeout_user *user; + int status; /* status to return (unless STATUS_PENDING) */ struct wait_queue_entry queues[1]; }; @@ -727,6 +728,11 @@ void make_wait_abandoned( struct wait_queue_entry *entry ) entry->wait->abandoned = 1; } +void set_wait_status( struct wait_queue_entry *entry, int status ) +{ + entry->wait->status = status; +} + /* finish waiting */ static unsigned int end_wait( struct thread *thread, unsigned int status ) { @@ -739,6 +745,7 @@ static unsigned int end_wait( struct thread *thread, unsigned int status ) if (status < wait->count) /* wait satisfied, tell it to the objects */ { + wait->status = status; if (wait->select == SELECT_WAIT_ALL) { for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) @@ -749,6 +756,7 @@ static unsigned int end_wait( struct thread *thread, unsigned int status ) entry = wait->queues + status; entry->obj->ops->satisfied( entry->obj, entry ); } + status = wait->status; if (wait->abandoned) status += STATUS_ABANDONED_WAIT_0; } for (i = 0, entry = wait->queues; i < wait->count; i++, entry++) diff --git a/server/thread.h b/server/thread.h index 74b828f768b..8dcf966a90a 100644 --- a/server/thread.h +++ b/server/thread.h @@ -106,6 +106,7 @@ extern struct thread *get_wait_queue_thread( struct wait_queue_entry *entry ); extern enum select_op get_wait_queue_select_op( struct wait_queue_entry *entry ); extern client_ptr_t get_wait_queue_key( struct wait_queue_entry *entry ); extern void make_wait_abandoned( struct wait_queue_entry *entry ); +extern void set_wait_status( struct wait_queue_entry *entry, int status ); extern void stop_thread( struct thread *thread ); extern int wake_thread( struct thread *thread ); extern int wake_thread_queue_entry( struct wait_queue_entry *entry );