From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> Ensure proper atomic update, so that the application do not read stale information from IO_STATUS_BLOCK. Also, ensure IOSB Status loads are not reordered with Information loads from the consumer side as well, specifically in the following functions: - GetOverlappedResultEx - WSAGetOverlappedResult --- dlls/kernelbase/file.c | 2 +- dlls/ntdll/unix/unix_private.h | 25 ++++++++++++++++++++++--- dlls/ws2_32/socket.c | 2 +- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index 5ba7e0be419..3d4989ebeed 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -3133,7 +3133,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH GetOverlappedResultEx( HANDLE file, OVERLAPPED *ov TRACE( "(%p %p %p %lu %d)\n", file, overlapped, result, timeout, alertable ); - status = overlapped->Internal; + status = ReadAcquire( (volatile LONG *)&overlapped->Internal ); if (status == STATUS_PENDING) { if (!timeout) diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..3939186ad05 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -361,6 +361,8 @@ static inline BOOL in_wow64_call(void) static inline void set_async_iosb( client_ptr_t iosb, NTSTATUS status, ULONG_PTR info ) { + NTSTATUS *status_ptr; + if (!iosb) return; if (in_wow64_call()) @@ -370,19 +372,36 @@ static inline void set_async_iosb( client_ptr_t iosb, NTSTATUS status, ULONG_PTR NTSTATUS Status; ULONG Information; } *io = wine_server_get_ptr( iosb ); - io->Status = status; + status_ptr = &io->Status; io->Information = info; } else { IO_STATUS_BLOCK *io = wine_server_get_ptr( iosb ); #ifdef NONAMELESSUNION - io->u.Status = status; + status_ptr = &io->u.Status; #else - io->Status = status; + status_ptr = &io->Status; #endif io->Information = info; } + + /* GetOverlappedResultEx() skips waiting for the event/file handle if the + * Status field indicates completion, and returns the result from the + * Information field. + * + * Hence, we have to ensure that writes to the Information field are seen + * from the other threads *before* writes to the Status field. + * + * Otherwise, a race condition results: if writing the Status field has + * been completed but the subsequent update to the Information field has + * not, the application may end up reading stale value from it. + * + * The race condition can be especially problematic with applications that + * use the HasOverlappedIoCompleted() macro in a tight loop to poll for + * completion of overlapped I/O. + */ + WriteRelease( (volatile LONG *)status_ptr, status ); } static inline client_ptr_t iosb_client_ptr( IO_STATUS_BLOCK *io ) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 2f115f02e48..3489c437dac 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3702,7 +3702,7 @@ BOOL WINAPI WSAGetOverlappedResult( SOCKET s, LPWSAOVERLAPPED lpOverlapped, return FALSE; } - status = lpOverlapped->Internal; + status = ReadAcquire( (volatile LONG *)&lpOverlapped->Internal ); if (status == STATUS_PENDING) { if (!fWait) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/155