[PATCH 1/2] ntdll: Set socket IO status before calling server if operation is completed.
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- If the operation is completed before the server call then the completion will be sent during the server call and another thread may receive it before the iosb status is set. It looks like we shouldn't normally have failure status from server once our operation already succeeded. dlls/ntdll/unix/socket.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 92374e39db7..6937a84df85 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -679,6 +679,7 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi struct WS_sockaddr *addr, int *addr_len, DWORD *ret_flags, int unix_flags, int force_async ) { struct async_recv_ioctl *async; + BOOL ioresult_set = FALSE; ULONG_PTR information; HANDLE wait_handle; DWORD async_size; @@ -736,8 +737,12 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } status = try_recv( fd, async, &information ); - - if (status != STATUS_SUCCESS && status != STATUS_BUFFER_OVERFLOW && status != STATUS_DEVICE_NOT_READY) + if (!status) + { + io->Status = 0; + io->Information = information; + ioresult_set = TRUE; + } else if (status != STATUS_BUFFER_OVERFLOW && status != STATUS_DEVICE_NOT_READY) { release_fileio( &async->io ); return status; @@ -755,7 +760,11 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi status = wine_server_call( req ); wait_handle = wine_server_ptr_handle( reply->wait ); options = reply->options; - if ((!NT_ERROR(status) || wait_handle) && status != STATUS_PENDING) + + if (ioresult_set && status) + FIXME( "Server status %#x after successful receive.\n", status ); + + if ((!ioresult_set || status) && (!NT_ERROR(status) || wait_handle) && status != STATUS_PENDING) { io->Status = status; io->Information = information; @@ -865,6 +874,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi const struct WS_sockaddr *addr, unsigned int addr_len, int unix_flags, int force_async ) { struct async_send_ioctl *async; + BOOL ioresult_set = FALSE; HANDLE wait_handle; DWORD async_size; NTSTATUS status; @@ -905,7 +915,13 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi status = try_send( fd, async ); - if (status != STATUS_SUCCESS && status != STATUS_DEVICE_NOT_READY) + if (!status) + { + io->Status = 0; + io->Information = async->sent_len; + ioresult_set = TRUE; + } + else if (status != STATUS_DEVICE_NOT_READY) { release_fileio( &async->io ); return status; @@ -922,7 +938,11 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi status = wine_server_call( req ); wait_handle = wine_server_ptr_handle( reply->wait ); options = reply->options; - if ((!NT_ERROR(status) || wait_handle) && status != STATUS_PENDING) + + if (ioresult_set && status) + FIXME( "Server status %#x after successful send.\n", status ); + + if ((!ioresult_set || status) && (!NT_ERROR(status) || wait_handle) && status != STATUS_PENDING) { io->Status = status; io->Information = async->sent_len; -- 2.33.1
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- Currently NtDeviceIoControlFile() always sets the io comletion status after sock_ioctl(). This may happen after the application has received and processed the completion already and rightfully freed or reused IOSB (OVERLAPPED) pointer and result in data corruption. Fixes random crashes in Forza Horizon 4 when connected to online. dlls/ntdll/unix/file.c | 1 + dlls/ntdll/unix/socket.c | 52 ++++++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 51c92df57e3..07bfa6c22c4 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -5712,6 +5712,7 @@ NTSTATUS WINAPI NtDeviceIoControlFile( HANDLE handle, HANDLE event, PIO_APC_ROUT case FILE_DEVICE_BEEP: case FILE_DEVICE_NETWORK: status = sock_ioctl( handle, event, apc, apc_context, io, code, in_buffer, in_size, out_buffer, out_size ); + if (status != STATUS_NOT_SUPPORTED && status != STATUS_BAD_DEVICE_TYPE) return status; break; case FILE_DEVICE_DISK: case FILE_DEVICE_CD_ROM: diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 6937a84df85..94e1e9efcfe 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1162,7 +1162,11 @@ static NTSTATUS do_getsockopt( HANDLE handle, IO_STATUS_BLOCK *io, int level, ret = getsockopt( fd, level, option, out_buffer, &len ); if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); - if (io) io->Information = len; + if (io) + { + io->Status = STATUS_SUCCESS; + io->Information = len; + } return STATUS_SUCCESS; } @@ -1179,7 +1183,9 @@ static NTSTATUS do_setsockopt( HANDLE handle, IO_STATUS_BLOCK *io, int level, ret = setsockopt( fd, level, option, optval, optlen ); if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + if (ret) return sock_errno_to_status( errno ); + if (io) io->Status = STATUS_SUCCESS; + return STATUS_SUCCESS; } @@ -1308,9 +1314,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (params.msg_flags & AFD_MSG_WAITALL) FIXME( "MSG_WAITALL is not supported\n" ); - status = sock_recv( handle, event, apc, apc_user, io, fd, params.buffers, params.count, NULL, + return sock_recv( handle, event, apc, apc_user, io, fd, params.buffers, params.count, NULL, NULL, NULL, NULL, unix_flags, !!(params.recv_flags & AFD_RECV_FORCE_ASYNC) ); - break; } case IOCTL_AFD_WINE_RECVMSG: @@ -1335,11 +1340,10 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (*ws_flags & WS_MSG_WAITALL) FIXME( "MSG_WAITALL is not supported\n" ); - status = sock_recv( handle, event, apc, apc_user, io, fd, u64_to_user_ptr(params->buffers_ptr), + return sock_recv( handle, event, apc, apc_user, io, fd, u64_to_user_ptr(params->buffers_ptr), params->count, u64_to_user_ptr(params->control_ptr), u64_to_user_ptr(params->addr_ptr), u64_to_user_ptr(params->addr_len_ptr), ws_flags, unix_flags, params->force_async ); - break; } case IOCTL_AFD_WINE_SENDMSG: @@ -1363,9 +1367,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (params->ws_flags & ~(WS_MSG_OOB | WS_MSG_PARTIAL)) FIXME( "unknown flags %#x\n", params->ws_flags ); - status = sock_send( handle, event, apc, apc_user, io, fd, u64_to_user_ptr( params->buffers_ptr ), params->count, + return sock_send( handle, event, apc, apc_user, io, fd, u64_to_user_ptr( params->buffers_ptr ), params->count, u64_to_user_ptr( params->addr_ptr ), params->addr_len, unix_flags, params->force_async ); - break; } case IOCTL_AFD_WINE_TRANSMIT: @@ -1666,7 +1669,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc io->Information = sizeof(*ws_linger); } - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_SO_LINGER: @@ -1695,7 +1699,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc case IOCTL_AFD_WINE_SET_SO_REUSEADDR: { int fd, needs_close = FALSE; - NTSTATUS status; int ret; if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) @@ -1706,7 +1709,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (!ret) ret = setsockopt( fd, SOL_SOCKET, SO_REUSEPORT, in_buffer, in_size ); #endif if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_IP_ADD_MEMBERSHIP: @@ -1748,7 +1752,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); io->Information = len; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_IP_DONTFRAGMENT: @@ -1766,7 +1771,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (!once++) FIXME( "IP_DONTFRAGMENT is not supported on this platform\n" ); - return STATUS_SUCCESS; /* fake success */ + status = STATUS_SUCCESS; /* fake success */ + break; } #endif @@ -1954,7 +1960,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); io->Information = len; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_IPV6_DONTFRAG: @@ -2093,7 +2100,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc int fd, needs_close = FALSE; union unix_sockaddr addr; socklen_t len = sizeof(addr); - NTSTATUS status; int ret; if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) @@ -2103,12 +2109,14 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc { /* changing IPV6_V6ONLY succeeds on an unbound IPv4 socket */ WARN( "ignoring IPV6_V6ONLY on an unbound IPv4 socket\n" ); - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } ret = setsockopt( fd, IPPROTO_IPV6, IPV6_V6ONLY, in_buffer, in_size ); if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; } #ifdef SOL_IPX @@ -2132,7 +2140,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); *(DWORD *)out_buffer = value.ipx_pt; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_IPX_PTYPE: @@ -2154,7 +2163,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc socklen_t len = sizeof(buffer); DEVICELIST *ws_list = out_buffer; int fd, needs_close = FALSE; - NTSTATUS status; unsigned int i; int ret; @@ -2184,7 +2192,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc ws_dev->irdaDeviceHints2 = unix_dev->hints[1]; ws_dev->irdaCharSet = unix_dev->charset; } - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } #endif @@ -2212,5 +2221,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc } if (needs_close) close( fd ); + + if (status != STATUS_PENDING && !NT_ERROR(status)) io->Status = status; + return status; } -- 2.33.1
On 12/16/21 12:43, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- Currently NtDeviceIoControlFile() always sets the io comletion status after sock_ioctl(). This may happen after the application has received and processed the completion already and rightfully freed or reused IOSB (OVERLAPPED) pointer and result in data corruption. Fixes random crashes in Forza Horizon 4 when connected to online.
dlls/ntdll/unix/file.c | 1 + dlls/ntdll/unix/socket.c | 52 ++++++++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 51c92df57e3..07bfa6c22c4 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -5712,6 +5712,7 @@ NTSTATUS WINAPI NtDeviceIoControlFile( HANDLE handle, HANDLE event, PIO_APC_ROUT case FILE_DEVICE_BEEP: case FILE_DEVICE_NETWORK: status = sock_ioctl( handle, event, apc, apc_context, io, code, in_buffer, in_size, out_buffer, out_size ); + if (status != STATUS_NOT_SUPPORTED && status != STATUS_BAD_DEVICE_TYPE) return status; break; case FILE_DEVICE_DISK: case FILE_DEVICE_CD_ROM:
I think other types of ioctls deserve a similar treatment (although perhaps that should wait until after code freeze?) This patch also misses the following socket ioctls: * IOCTL_AFD_WINE_TRANSMIT * IOCTL_AFD_WINE_COMPLETE_ASYNC * IOCTL_AFD_WINE_FIONREAD * IOCTL_AFD_WINE_SIOCATMARK * IOCTL_AFD_WINE_GET_INTERFACE_LIST * IOCTL_AFD_WINE_KEEPALIVE_VALS
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 6937a84df85..94e1e9efcfe 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1162,7 +1162,11 @@ static NTSTATUS do_getsockopt( HANDLE handle, IO_STATUS_BLOCK *io, int level, ret = getsockopt( fd, level, option, out_buffer, &len ); if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); - if (io) io->Information = len; + if (io) + { + io->Status = STATUS_SUCCESS; + io->Information = len; + } return STATUS_SUCCESS; }
@@ -1179,7 +1183,9 @@ static NTSTATUS do_setsockopt( HANDLE handle, IO_STATUS_BLOCK *io, int level,
ret = setsockopt( fd, level, option, optval, optlen ); if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + if (ret) return sock_errno_to_status( errno ); + if (io) io->Status = STATUS_SUCCESS; + return STATUS_SUCCESS; }
@@ -1308,9 +1314,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (params.msg_flags & AFD_MSG_WAITALL) FIXME( "MSG_WAITALL is not supported\n" );
- status = sock_recv( handle, event, apc, apc_user, io, fd, params.buffers, params.count, NULL, + return sock_recv( handle, event, apc, apc_user, io, fd, params.buffers, params.count, NULL, NULL, NULL, NULL, unix_flags, !!(params.recv_flags & AFD_RECV_FORCE_ASYNC) ); - break; }
case IOCTL_AFD_WINE_RECVMSG:
This leaks "fd".
@@ -1335,11 +1340,10 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (*ws_flags & WS_MSG_WAITALL) FIXME( "MSG_WAITALL is not supported\n" );
- status = sock_recv( handle, event, apc, apc_user, io, fd, u64_to_user_ptr(params->buffers_ptr), + return sock_recv( handle, event, apc, apc_user, io, fd, u64_to_user_ptr(params->buffers_ptr), params->count, u64_to_user_ptr(params->control_ptr), u64_to_user_ptr(params->addr_ptr), u64_to_user_ptr(params->addr_len_ptr), ws_flags, unix_flags, params->force_async ); - break; }
case IOCTL_AFD_WINE_SENDMSG:
Same here.
@@ -1363,9 +1367,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (params->ws_flags & ~(WS_MSG_OOB | WS_MSG_PARTIAL)) FIXME( "unknown flags %#x\n", params->ws_flags );
- status = sock_send( handle, event, apc, apc_user, io, fd, u64_to_user_ptr( params->buffers_ptr ), params->count, + return sock_send( handle, event, apc, apc_user, io, fd, u64_to_user_ptr( params->buffers_ptr ), params->count, u64_to_user_ptr( params->addr_ptr ), params->addr_len, unix_flags, params->force_async ); - break; }
case IOCTL_AFD_WINE_TRANSMIT:
Same here.
@@ -1666,7 +1669,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc io->Information = sizeof(*ws_linger); }
- return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; }
case IOCTL_AFD_WINE_SET_SO_LINGER:
And this might close "fd" twice.
@@ -1695,7 +1699,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc case IOCTL_AFD_WINE_SET_SO_REUSEADDR: { int fd, needs_close = FALSE; - NTSTATUS status; int ret;
if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) @@ -1706,7 +1709,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (!ret) ret = setsockopt( fd, SOL_SOCKET, SO_REUSEPORT, in_buffer, in_size ); #endif if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; }
case IOCTL_AFD_WINE_SET_IP_ADD_MEMBERSHIP:
Same here.
@@ -1748,7 +1752,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); io->Information = len; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; }
case IOCTL_AFD_WINE_SET_IP_DONTFRAGMENT:
Same here.
@@ -1766,7 +1771,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc
if (!once++) FIXME( "IP_DONTFRAGMENT is not supported on this platform\n" ); - return STATUS_SUCCESS; /* fake success */ + status = STATUS_SUCCESS; /* fake success */ + break; } #endif
@@ -1954,7 +1960,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); io->Information = len; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; }
case IOCTL_AFD_WINE_SET_IPV6_DONTFRAG:
Same here.
@@ -2093,7 +2100,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc int fd, needs_close = FALSE; union unix_sockaddr addr; socklen_t len = sizeof(addr); - NTSTATUS status; int ret;
if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) @@ -2103,12 +2109,14 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc { /* changing IPV6_V6ONLY succeeds on an unbound IPv4 socket */ WARN( "ignoring IPV6_V6ONLY on an unbound IPv4 socket\n" ); - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; }
ret = setsockopt( fd, IPPROTO_IPV6, IPV6_V6ONLY, in_buffer, in_size ); if (needs_close) close( fd ); - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; }
#ifdef SOL_IPX
Same here.
@@ -2132,7 +2140,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc if (needs_close) close( fd ); if (ret) return sock_errno_to_status( errno ); *(DWORD *)out_buffer = value.ipx_pt; - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; }
case IOCTL_AFD_WINE_SET_IPX_PTYPE:
Same here.
@@ -2154,7 +2163,6 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc socklen_t len = sizeof(buffer); DEVICELIST *ws_list = out_buffer; int fd, needs_close = FALSE; - NTSTATUS status; unsigned int i; int ret;
@@ -2184,7 +2192,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc ws_dev->irdaDeviceHints2 = unix_dev->hints[1]; ws_dev->irdaCharSet = unix_dev->charset; } - return STATUS_SUCCESS; + status = STATUS_SUCCESS; + break; } #endif
Same here.
@@ -2212,5 +2221,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc }
if (needs_close) close( fd ); + + if (status != STATUS_PENDING && !NT_ERROR(status)) io->Status = status; + return status; }
Thanks for spotting all that. On 12/16/21 22:43, Zebediah Figura (she/her) wrote:
@@ -1666,7 +1669,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc io->Information = sizeof(*ws_linger); } - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_SO_LINGER:
And this might close "fd" twice.
I must be missing something but not sure how? IOCTL_AFD_WINE_SET_SO_LINGER and other similar cases return do_setsockopt() and fd is opened and closed inside do_setsockopt()?
On 12/16/21 22:56, Paul Gofman wrote:
Thanks for spotting all that.
On 12/16/21 22:43, Zebediah Figura (she/her) wrote:
@@ -1666,7 +1669,8 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc io->Information = sizeof(*ws_linger); } - return ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + status = ret ? sock_errno_to_status( errno ) : STATUS_SUCCESS; + break; } case IOCTL_AFD_WINE_SET_SO_LINGER:
And this might close "fd" twice.
I must be missing something but not sure how? IOCTL_AFD_WINE_SET_SO_LINGER and other similar cases return do_setsockopt() and fd is opened and closed inside do_setsockopt()?
Sorry, I was looking into the wrong case, I see now.
On 12/16/21 12:43, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- If the operation is completed before the server call then the completion will be sent during the server call and another thread may receive it before the iosb status is set. It looks like we shouldn't normally have failure status from server once our operation already succeeded.
That doesn't sound right. The server shouldn't queue completion during the recv_socket server call, but rather during the subsequent wait on the async object [i.e. wait_async()]. In that case we should have already filled the IOSB.
On 12/16/21 21:50, Zebediah Figura (she/her) wrote:
On 12/16/21 12:43, Paul Gofman wrote:
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- If the operation is completed before the server call then the completion will be sent during the server call and another thread may receive it before the iosb status is set. It looks like we shouldn't normally have failure status from server once our operation already succeeded.
That doesn't sound right. The server shouldn't queue completion during the recv_socket server call, but rather during the subsequent wait on the async object [i.e. wait_async()]. In that case we should have already filled the IOSB.
Oh, yeah, it is probably triggered by async_satisfied(). I briefly checked and it looks like it is the second patch alone does the job here. So I suggest to just ignore this one, the second patch applies cleanly without this one. Just for better understanding, why a separate wait on async is needed even if the socket operation already succeeded before the server call?
participants (2)
-
Paul Gofman -
Zebediah Figura (she/her)