On 12/16/21 12:43, Paul Gofman wrote:
Signed-off-by: Paul Gofman pgofman@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; }@@ -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; }
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;