[PATCH v7 0/3] MR3074: Move getpeername() implementation to wineserver
`getpeername()` is currently handled in ntdll. This merge request changes ntdll to forward `getpeername()` to wineserver. The implementation utilises new `peer_addr` and `peer_addr_len` fields in wineserver's `struct sock`. This fixes multiple `todo_wine`s in `ws2_32/tests` and allows for more accurate peer names to be provided in case the address the socket is bound to on the Unix side does not match what `bind()` was called with on the Windows side. *This merge request was originally intended to be included in !2786 (Add support for AF_UNIX sockets) but was split into its own merge request because this is not an insignificant change.* -- v7: ws2_32/tests: Fix incorrect test (bug #47560) ws2_32/tests: Remove todo_wine from now-successful tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/3074
From: Ally Sommers <dropbear.sh(a)gmail.com> This brings getpeername() in line with getsockname(), which is also implemented in wineserver. It also allows getpeername() to return a possibly-more-accurate peer name, as in the case of AF_UNIX sockets. --- dlls/ntdll/unix/socket.c | 24 ++------------------ server/sock.c | 47 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 4e706323a0a..8fcd366879b 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1912,30 +1912,10 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc } 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; - } + if (in_size) FIXME( "unexpected input size %u\n", in_size ); - 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; + status = STATUS_BAD_DEVICE_TYPE; break; - } case IOCTL_AFD_WINE_GET_SO_BROADCAST: return do_getsockopt( handle, io, SOL_SOCKET, SO_BROADCAST, out_buffer, out_size ); diff --git a/server/sock.c b/server/sock.c index 088e6d63079..f81f096d66d 100644 --- a/server/sock.c +++ b/server/sock.c @@ -239,6 +239,8 @@ struct sock struct poll_req *main_poll; /* main poll */ union win_sockaddr addr; /* socket name */ int addr_len; /* socket name length */ + union win_sockaddr peer_addr; /* peer name */ + int peer_addr_len; /* peer name length */ unsigned int rcvbuf; /* advisory recv buffer size */ unsigned int sndbuf; /* advisory send buffer size */ unsigned int rcvtimeo; /* receive timeout in ms */ @@ -1718,6 +1720,8 @@ static struct sock *create_socket(void) sock->main_poll = NULL; memset( &sock->addr, 0, sizeof(sock->addr) ); sock->addr_len = 0; + memset( &sock->peer_addr, 0, sizeof(sock->peer_addr) ); + sock->peer_addr_len = 0; sock->rd_shutdown = 0; sock->wr_shutdown = 0; sock->wr_shutdown_pending = 0; @@ -2020,8 +2024,15 @@ static struct sock *accept_socket( struct sock *sock ) } unix_len = sizeof(unix_addr); if (!getsockname( acceptfd, &unix_addr.addr, &unix_len )) + { acceptsock->addr_len = sockaddr_from_unix( &unix_addr, &acceptsock->addr.addr, sizeof(acceptsock->addr) ); + if (!getpeername( acceptfd, &unix_addr.addr, &unix_len )) + acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->peer_addr.addr, + sizeof(acceptsock->peer_addr) ); + } } + clear_error(); sock->pending_events &= ~AFD_POLL_ACCEPT; sock->reported_events &= ~AFD_POLL_ACCEPT; @@ -2076,7 +2087,13 @@ static int accept_into_socket( struct sock *sock, struct sock *acceptsock ) unix_len = sizeof(unix_addr); if (!getsockname( get_unix_fd( newfd ), &unix_addr.addr, &unix_len )) + { acceptsock->addr_len = sockaddr_from_unix( &unix_addr, &acceptsock->addr.addr, sizeof(acceptsock->addr) ); + if (!getpeername( get_unix_fd( newfd ), &unix_addr.addr, &unix_len )) + acceptsock->peer_addr_len = sockaddr_from_unix( &unix_addr, + &acceptsock->peer_addr.addr, + sizeof(acceptsock->peer_addr) ); + } clear_error(); sock->pending_events &= ~AFD_POLL_ACCEPT; @@ -2545,7 +2562,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) { const struct afd_connect_params *params = get_req_data(); const struct WS_sockaddr *addr; - union unix_sockaddr unix_addr; + union unix_sockaddr unix_addr, peer_addr; struct connect_req *req; socklen_t unix_len; int send_len, ret; @@ -2607,6 +2624,7 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) 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 ); + memcpy( &peer_addr, &unix_addr, sizeof(unix_addr) ); ret = connect( unix_fd, &unix_addr.addr, unix_len ); if (ret < 0 && errno == ECONNABORTED) { @@ -2630,7 +2648,10 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) unix_len = sizeof(unix_addr); if (!getsockname( unix_fd, &unix_addr.addr, &unix_len )) + { sock->addr_len = sockaddr_from_unix( &unix_addr, &sock->addr.addr, sizeof(sock->addr) ); + sock->peer_addr_len = sockaddr_from_unix( &peer_addr, &sock->peer_addr.addr, sizeof(sock->peer_addr)); + } sock->bound = 1; if (!ret) @@ -2987,6 +3008,30 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) set_reply_data( &sock->addr, sock->addr_len ); return; + case IOCTL_AFD_WINE_GETPEERNAME: + if (sock->state != SOCK_CONNECTED && + sock->state != SOCK_CONNECTING && + sock->state != SOCK_CONNECTIONLESS) + { + set_error( STATUS_INVALID_CONNECTION ); + return; + } + + if (!sock->peer_addr_len && sock->type == WS_SOCK_DGRAM) + { + set_error( STATUS_INVALID_CONNECTION ); + return; + } + + if (get_reply_max_size() < sock->peer_addr_len) + { + set_error( STATUS_BUFFER_TOO_SMALL ); + return; + } + + set_reply_data( &sock->peer_addr, sock->peer_addr_len); + return; + case IOCTL_AFD_WINE_DEFER: { const obj_handle_t *handle = get_req_data(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3074
From: Ally Sommers <dropbear.sh(a)gmail.com> --- dlls/ws2_32/tests/sock.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index cff4acb0b3c..757e879e961 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -9158,16 +9158,16 @@ static void test_shutdown(void) addrlen = sizeof(addr); ret = getpeername(client, (struct sockaddr *)&addr, &addrlen); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); - todo_wine ok(!memcmp(&addr, &server_addr, sizeof(server_addr)), "address didn't match\n"); + ok(!ret, "got error %u\n", WSAGetLastError()); + ok(!memcmp(&addr, &server_addr, sizeof(server_addr)), "address didn't match\n"); addrlen = sizeof(client_addr); ret = getsockname(client, (struct sockaddr *)&client_addr, &addrlen); ok(!ret, "got error %u\n", WSAGetLastError()); addrlen = sizeof(addr); ret = getpeername(server, (struct sockaddr *)&addr, &addrlen); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); - todo_wine ok(!memcmp(&addr, &client_addr, sizeof(addr)), "address didn't match\n"); + ok(!ret, "got error %u\n", WSAGetLastError()); + ok(!memcmp(&addr, &client_addr, sizeof(addr)), "address didn't match\n"); WSASetLastError(0xdeadbeef); ret = connect(client, (struct sockaddr *)&server_addr, sizeof(server_addr)); @@ -9226,16 +9226,16 @@ static void test_shutdown(void) addrlen = sizeof(addr); ret = getpeername(client, (struct sockaddr *)&addr, &addrlen); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); - todo_wine ok(!memcmp(&addr, &server_addr, sizeof(server_addr)), "address didn't match\n"); + ok(!ret, "got error %u\n", WSAGetLastError()); + ok(!memcmp(&addr, &server_addr, sizeof(server_addr)), "address didn't match\n"); addrlen = sizeof(client_addr); ret = getsockname(client, (struct sockaddr *)&client_addr, &addrlen); ok(!ret, "got error %u\n", WSAGetLastError()); addrlen = sizeof(addr); ret = getpeername(server, (struct sockaddr *)&addr, &addrlen); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); - todo_wine ok(!memcmp(&addr, &client_addr, sizeof(addr)), "address didn't match\n"); + ok(!ret, "got error %u\n", WSAGetLastError()); + ok(!memcmp(&addr, &client_addr, sizeof(addr)), "address didn't match\n"); closesocket(client); closesocket(server); @@ -12444,7 +12444,7 @@ static void test_connecting_socket(void) len = sizeof(addr); ret = getpeername(client, (struct sockaddr *)&addr, &len); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); + ok(!ret, "got error %u\n", WSAGetLastError()); if (!ret) { ok(addr.sin_family == AF_INET, "got family %u\n", addr.sin_family); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3074
From: Ally Sommers <dropbear.sh(a)gmail.com> --- dlls/ws2_32/tests/sock.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 757e879e961..090fb370d1d 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -12496,8 +12496,7 @@ static void test_connecting_socket(void) len = sizeof(addr); ret = getpeername(client, (struct sockaddr *)&addr, &len); - ok(ret == -1, "got %d\n", ret); - ok(WSAGetLastError() == WSAENOTCONN, "got %u\n", WSAGetLastError()); + ok(!ret, "got %d: %u\n", ret, WSAGetLastError()); ret = recv(client, buffer, sizeof(buffer), 0); ok(ret == -1, "got %d\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3074
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=134004 Your paranoid android. === w7u_2qxl (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w7u_adm (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w7u_el (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w8 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w8adm (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w864 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064v1507 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064v1809 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064_tsign (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w11pro64 (32 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w7pro64 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w864 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064v1507 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064v1809 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064_2qxl (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064_adm (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w1064_tsign (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64_en_AE_u8 (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64_ar (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64_ja (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w10pro64_zh_CN (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057 === w11pro64_amd (64 bit report) === ws2_32: sock.c:12499: Test failed: got -1: 10057
This might not be reliably testable. It appears to depend on how fast the nonblocking `connect()` call finishes, i.e. if `getpeername()` is called before `connect()` finishes failing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3074#note_36403
participants (3)
-
Ally Sommers -
Ally Sommers (＠dropbear)
-
Marvin