[PATCH 1/5] server: Correctly access the sir_lsap_sel field.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- server/sock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/sock.c b/server/sock.c index 6b0c4af6b11..23c750243a6 100644 --- a/server/sock.c +++ b/server/sock.c @@ -427,10 +427,10 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl memcpy( &win, wsaddr, sizeof(win) ); uaddr->irda.sir_family = AF_IRDA; if (sscanf( win.irdaServiceName, "LSAP-SEL%u", &lsap_sel ) == 1) - uaddr->sir_lsap_sel = lsap_sel; + uaddr->irda.sir_lsap_sel = lsap_sel; else { - uaddr->sir_lsap_sel = LSAP_ANY; + uaddr->irda.sir_lsap_sel = LSAP_ANY; memcpy( uaddr->irda.sir_name, win.irdaServiceName, sizeof(win.irdaServiceName) ); } memcpy( &uaddr->irda.sir_addr, win.irdaDeviceID, sizeof(win.irdaDeviceID) ); -- 2.30.2
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ws2_32/socket.c | 11 ----------- server/sock.c | 6 ++++++ 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 63c26a9c769..c8403808d8f 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -3282,20 +3282,9 @@ int WINAPI WS_listen( SOCKET s, int backlog ) struct afd_listen_params params = {.backlog = backlog}; IO_STATUS_BLOCK io; NTSTATUS status; - int fd, bound; TRACE( "socket %#lx, backlog %d\n", s, backlog ); - if ((fd = get_sock_fd( s, FILE_READ_DATA, NULL )) == -1) - return -1; - bound = is_fd_bound( fd, NULL, NULL ); - release_sock_fd( s, fd ); - if (bound <= 0) - { - SetLastError( bound ? wsaErrno() : WSAEINVAL ); - return -1; - } - status = NtDeviceIoControlFile( SOCKET2HANDLE(s), NULL, NULL, NULL, &io, IOCTL_AFD_LISTEN, ¶ms, sizeof(params), NULL, 0 ); SetLastError( NtStatusToWSAError( status ) ); diff --git a/server/sock.c b/server/sock.c index 23c750243a6..39575b22f76 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2084,6 +2084,12 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return 0; } + if (!sock->bound) + { + set_error( STATUS_INVALID_PARAMETER ); + return 0; + } + if (listen( unix_fd, params->backlog ) < 0) { set_error( sock_get_ntstatus( errno ) ); -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=92649 Your paranoid android. === debiant2 (32 bit Arabic:Egypt report) === ws2_32: sock.c:4767: Test failed: expected timeout sock.c:6973: Test failed: The wrong first client was accepted by acceptex: 2 != 1 sock.c:8253: Test succeeded inside todo block: GetQueuedCompletionStatus returned 0 sock.c:8254: Test succeeded inside todo block: Last error was 64
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ws2_32/socket.c | 54 ++++++++------------------------------------ include/wine/afd.h | 2 +- server/sock.c | 19 ++++++++++++---- 3 files changed, 24 insertions(+), 51 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index c8403808d8f..845a0e68986 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1959,8 +1959,6 @@ int WINAPI WS_closesocket(SOCKET s) */ int WINAPI WS_connect( SOCKET s, const struct WS_sockaddr *addr, int len ) { - union generic_unix_sockaddr uaddr; - unsigned int uaddrlen = ws_sockaddr_ws2u( addr, len, &uaddr ); struct afd_connect_params *params; IO_STATUS_BLOCK io; HANDLE sync_event; @@ -1968,35 +1966,19 @@ int WINAPI WS_connect( SOCKET s, const struct WS_sockaddr *addr, int len ) TRACE( "socket %#lx, addr %s, len %d\n", s, debugstr_sockaddr(addr), len ); - if (!uaddrlen) - { - SetLastError( WSAEFAULT ); - return -1; - } - - if (addr->sa_family == WS_AF_INET) - { - struct sockaddr_in *in4 = (struct sockaddr_in *)&uaddr; - if (!memcmp(&in4->sin_addr, magic_loopback_addr, sizeof(magic_loopback_addr))) - { - TRACE("Replacing magic address 127.12.34.56 with INADDR_LOOPBACK.\n"); - in4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); - } - } - if (!(sync_event = get_sync_event())) return -1; - if (!(params = HeapAlloc( GetProcessHeap(), 0, sizeof(*params) + uaddrlen ))) + if (!(params = HeapAlloc( GetProcessHeap(), 0, sizeof(*params) + len ))) { SetLastError( ERROR_NOT_ENOUGH_MEMORY ); return -1; } - params->addr_len = uaddrlen; + params->addr_len = len; params->synchronous = TRUE; - memcpy(params + 1, &uaddr, uaddrlen); + memcpy( params + 1, addr, len ); status = NtDeviceIoControlFile( (HANDLE)s, sync_event, NULL, NULL, &io, IOCTL_AFD_WINE_CONNECT, - params, sizeof(*params) + uaddrlen, NULL, 0); + params, sizeof(*params) + len, NULL, 0 ); HeapFree( GetProcessHeap(), 0, params ); if (status == STATUS_PENDING) { @@ -2029,8 +2011,6 @@ int WINAPI WSAConnect( SOCKET s, const struct WS_sockaddr* name, int namelen, static BOOL WINAPI WS2_ConnectEx( SOCKET s, const struct WS_sockaddr *name, int namelen, void *send_buffer, DWORD send_len, DWORD *ret_len, OVERLAPPED *overlapped ) { - union generic_unix_sockaddr uaddr; - unsigned int uaddrlen = ws_sockaddr_ws2u(name, namelen, &uaddr); struct afd_connect_params *params; void *cvalue = NULL; NTSTATUS status; @@ -2060,35 +2040,19 @@ static BOOL WINAPI WS2_ConnectEx( SOCKET s, const struct WS_sockaddr *name, int overlapped->Internal = STATUS_PENDING; overlapped->InternalHigh = 0; - if (!uaddrlen) - { - SetLastError( WSAEFAULT ); - return SOCKET_ERROR; - } - - if (name->sa_family == WS_AF_INET) - { - struct sockaddr_in *in4 = (struct sockaddr_in *)&uaddr; - if (!memcmp( &in4->sin_addr, magic_loopback_addr, sizeof(magic_loopback_addr) )) - { - TRACE("Replacing magic address 127.12.34.56 with INADDR_LOOPBACK.\n"); - in4->sin_addr.s_addr = htonl(INADDR_LOOPBACK); - } - } - - if (!(params = HeapAlloc( GetProcessHeap(), 0, sizeof(*params) + uaddrlen + send_len ))) + if (!(params = HeapAlloc( GetProcessHeap(), 0, sizeof(*params) + namelen + send_len ))) { SetLastError( ERROR_NOT_ENOUGH_MEMORY ); return SOCKET_ERROR; } - params->addr_len = uaddrlen; + params->addr_len = namelen; params->synchronous = FALSE; - memcpy( params + 1, &uaddr, uaddrlen ); - memcpy( (char *)(params + 1) + uaddrlen, send_buffer, send_len ); + memcpy( params + 1, name, namelen ); + memcpy( (char *)(params + 1) + namelen, send_buffer, send_len ); status = NtDeviceIoControlFile( SOCKET2HANDLE(s), overlapped->hEvent, NULL, cvalue, (IO_STATUS_BLOCK *)overlapped, IOCTL_AFD_WINE_CONNECT, - params, sizeof(*params) + uaddrlen + send_len, NULL, 0 ); + params, sizeof(*params) + namelen + send_len, NULL, 0 ); HeapFree( GetProcessHeap(), 0, params ); if (ret_len) *ret_len = overlapped->InternalHigh; SetLastError( NtStatusToWSAError( status ) ); diff --git a/include/wine/afd.h b/include/wine/afd.h index 04468363673..19c1e064aaf 100644 --- a/include/wine/afd.h +++ b/include/wine/afd.h @@ -173,7 +173,7 @@ struct afd_connect_params { int addr_len; int synchronous; - /* VARARG(addr, struct sockaddr, addr_len); */ + /* VARARG(addr, struct WS(sockaddr), addr_len); */ /* VARARG(data, bytes); */ }; diff --git a/server/sock.c b/server/sock.c index 39575b22f76..88f6e4ac9fd 100644 --- a/server/sock.c +++ b/server/sock.c @@ -109,6 +109,8 @@ #define IP_UNICAST_IF 50 #endif +static const char magic_loopback_addr[] = {127, 12, 34, 56}; + union win_sockaddr { struct WS_sockaddr addr; @@ -2109,8 +2111,8 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) case IOCTL_AFD_WINE_CONNECT: { const struct afd_connect_params *params = get_req_data(); + const struct WS_sockaddr *addr; union unix_sockaddr unix_addr; - const struct sockaddr *addr; struct connect_req *req; socklen_t unix_len; int send_len, ret; @@ -2122,7 +2124,7 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return 0; } send_len = get_req_data_size() - sizeof(*params) - params->addr_len; - addr = (const struct sockaddr *)(params + 1); + addr = (const struct WS_sockaddr *)(params + 1); if (sock->accept_recv_req) { @@ -2144,7 +2146,16 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) return 0; } - ret = connect( unix_fd, addr, params->addr_len ); + unix_len = sockaddr_to_unix( addr, params->addr_len, &unix_addr ); + if (!unix_len) + { + set_error( STATUS_INVALID_ADDRESS ); + return 0; + } + if (unix_addr.addr.sa_family == AF_INET && !memcmp( &unix_addr.in.sin_addr, magic_loopback_addr, 4 )) + unix_addr.in.sin_addr.s_addr = htonl( INADDR_LOOPBACK ); + + ret = connect( unix_fd, &unix_addr.addr, unix_len ); if (ret < 0 && errno != EINPROGRESS) { set_error( sock_get_ntstatus( errno ) ); @@ -2427,8 +2438,6 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) if (unix_addr.addr.sa_family == WS_AF_INET) { - static const char magic_loopback_addr[] = {127, 12, 34, 56}; - if (!memcmp( &unix_addr.in.sin_addr, magic_loopback_addr, 4 ) || bind_to_interface( sock, &unix_addr.in )) bind_addr.in.sin_addr.s_addr = htonl( INADDR_ANY ); -- 2.30.2
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ws2_32/socket.c | 12 ------------ server/sock.c | 6 ++++++ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 845a0e68986..1a484da1527 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2014,22 +2014,10 @@ static BOOL WINAPI WS2_ConnectEx( SOCKET s, const struct WS_sockaddr *name, int struct afd_connect_params *params; void *cvalue = NULL; NTSTATUS status; - int fd, ret; TRACE( "socket %#lx, ptr %p %s, length %d, send_buffer %p, send_len %u, overlapped %p\n", s, name, debugstr_sockaddr(name), namelen, send_buffer, send_len, overlapped ); - if ((fd = get_sock_fd( s, FILE_READ_DATA, NULL )) == -1) - return FALSE; - - if ((ret = is_fd_bound( fd, NULL, NULL )) <= 0) - { - SetLastError( ret ? wsaErrno() : WSAEINVAL ); - release_sock_fd( s, fd ); - return FALSE; - } - release_sock_fd( s, fd ); - if (!overlapped) { SetLastError( WSA_INVALID_PARAMETER ); diff --git a/server/sock.c b/server/sock.c index 88f6e4ac9fd..bb0d13d9e6c 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2126,6 +2126,12 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) send_len = get_req_data_size() - sizeof(*params) - params->addr_len; addr = (const struct WS_sockaddr *)(params + 1); + if (!params->synchronous && !sock->bound) + { + set_error( STATUS_INVALID_PARAMETER ); + return 0; + } + if (sock->accept_recv_req) { set_error( STATUS_INVALID_PARAMETER ); -- 2.30.2
This adds todo_wine to a couple of tests; however, these are overly zealous invalid parameter tests, and fixing them is nontrivial. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/ntdll/unix/socket.c | 26 ++++++++++++++++++++ dlls/ws2_32/socket.c | 51 +++++++++++++++++----------------------- dlls/ws2_32/tests/sock.c | 4 ++-- include/wine/afd.h | 1 + 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 02700d37d6a..04db120dcbd 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1540,6 +1540,32 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc break; } + case IOCTL_AFD_WINE_GETPEERNAME: + { + union unix_sockaddr unix_addr; + socklen_t unix_len = sizeof(unix_addr); + int len; + + if ((status = server_get_unix_fd( handle, 0, &fd, &needs_close, NULL, NULL ))) + return status; + + if (getpeername( fd, &unix_addr.addr, &unix_len ) < 0) + { + status = sock_errno_to_status( errno ); + break; + } + + len = sockaddr_from_unix( &unix_addr, out_buffer, out_size ); + if (out_size < len) + { + status = STATUS_BUFFER_TOO_SMALL; + break; + } + io->Information = len; + status = STATUS_SUCCESS; + break; + } + default: { if ((code >> 16) == FILE_DEVICE_NETWORK) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 1a484da1527..a4d89e728b8 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -2077,42 +2077,35 @@ static BOOL WINAPI WS2_DisconnectEx( SOCKET s, OVERLAPPED *overlapped, DWORD fla return !status; } + /*********************************************************************** - * getpeername (WS2_32.5) + * getpeername (ws2_32.5) */ -int WINAPI WS_getpeername(SOCKET s, struct WS_sockaddr *name, int *namelen) +int WINAPI WS_getpeername( SOCKET s, struct WS_sockaddr *addr, int *len ) { - int fd; - int res; + IO_STATUS_BLOCK io; + NTSTATUS status; - TRACE("socket %04lx, ptr %p, len %08x\n", s, name, namelen ? *namelen : 0); + TRACE( "socket %#lx, addr %p, len %d\n", s, addr, len ? *len : 0 ); - fd = get_sock_fd( s, 0, NULL ); - res = SOCKET_ERROR; - - if (fd != -1) + if (!socket_list_find( s )) { - union generic_unix_sockaddr uaddr; - socklen_t uaddrlen = sizeof(uaddr); - - if (getpeername(fd, &uaddr.addr, &uaddrlen) == 0) - { - if (!name || !namelen) - SetLastError(WSAEFAULT); - else if (ws_sockaddr_u2ws(&uaddr.addr, name, namelen) != 0) - /* The buffer was too small */ - SetLastError(WSAEFAULT); - else - { - res = 0; - TRACE("=> %s\n", debugstr_sockaddr(name)); - } - } - else - SetLastError(wsaErrno()); - release_sock_fd( s, fd ); + WSASetLastError( WSAENOTSOCK ); + return -1; } - return res; + + if (!len) + { + SetLastError( WSAEFAULT ); + return -1; + } + + status = NtDeviceIoControlFile( (HANDLE)s, NULL, NULL, NULL, &io, + IOCTL_AFD_WINE_GETPEERNAME, NULL, 0, addr, *len ); + if (!status) + *len = io.Information; + SetLastError( NtStatusToWSAError( status ) ); + return status ? -1 : 0; } diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 3e6461ad994..f67bc4efae5 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -7752,7 +7752,7 @@ static void test_getpeername(void) ret = getpeername(sock, NULL, NULL); ok(ret == SOCKET_ERROR, "Expected getpeername to return SOCKET_ERROR, got %d\n", ret); - ok(WSAGetLastError() == WSAENOTCONN, + todo_wine ok(WSAGetLastError() == WSAENOTCONN, "Expected WSAGetLastError() to return WSAENOTCONN, got %d\n", WSAGetLastError()); memset(&sa, 0, sizeof(sa)); @@ -7767,7 +7767,7 @@ static void test_getpeername(void) ret = getpeername(sock, NULL, NULL); ok(ret == SOCKET_ERROR, "Expected getpeername to return SOCKET_ERROR, got %d\n", ret); - ok(WSAGetLastError() == WSAENOTCONN, + todo_wine ok(WSAGetLastError() == WSAENOTCONN, "Expected WSAGetLastError() to return WSAENOTCONN, got %d\n", WSAGetLastError()); ret = connect(sock, (struct sockaddr*)&sa, sizeof(sa)); diff --git a/include/wine/afd.h b/include/wine/afd.h index 19c1e064aaf..ad8e3abf5ba 100644 --- a/include/wine/afd.h +++ b/include/wine/afd.h @@ -156,6 +156,7 @@ struct afd_get_events_params #define IOCTL_AFD_WINE_GET_INTERFACE_LIST CTL_CODE(FILE_DEVICE_NETWORK, 213, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_AFD_WINE_KEEPALIVE_VALS CTL_CODE(FILE_DEVICE_NETWORK, 214, METHOD_BUFFERED, FILE_ANY_ACCESS) #define IOCTL_AFD_WINE_MESSAGE_SELECT CTL_CODE(FILE_DEVICE_NETWORK, 215, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_AFD_WINE_GETPEERNAME CTL_CODE(FILE_DEVICE_NETWORK, 216, METHOD_BUFFERED, FILE_ANY_ACCESS) struct afd_create_params { -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=92652 Your paranoid android. === debiant2 (32 bit Arabic:Egypt report) === ntdll: om.c:2307: Test failed: got 85 ws2_32: sock.c:5720: Test succeeded inside todo block: expected failure sock.c:5721: Test succeeded inside todo block: got error 64 sock.c:4767: Test failed: expected timeout === debiant2 (32 bit Hebrew:Israel report) === ws2_32: sock.c:5720: Test succeeded inside todo block: expected failure sock.c:5721: Test succeeded inside todo block: got error 64 === debiant2 (32 bit Japanese:Japan report) === ntdll: om.c:2307: Test failed: got 88
On 6/17/21 12:27 AM, Marvin wrote:
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=92652
Your paranoid android.
=== debiant2 (32 bit Arabic:Egypt report) ===
ntdll: om.c:2307: Test failed: got 85
ws2_32: sock.c:5720: Test succeeded inside todo block: expected failure sock.c:5721: Test succeeded inside todo block: got error 64 sock.c:4767: Test failed: expected timeout
=== debiant2 (32 bit Hebrew:Israel report) ===
ws2_32: sock.c:5720: Test succeeded inside todo block: expected failure sock.c:5721: Test succeeded inside todo block: got error 64
I haven't seen this failure before, but unfortunately I think it is spurious.
=== debiant2 (32 bit Japanese:Japan report) ===
ntdll: om.c:2307: Test failed: got 88
participants (3)
-
Marvin -
Zebediah Figura -
Zebediah Figura (she/her)