[PATCH v34 0/1] 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.* -- v34: server: Move getpeername() implementation from ntdll/unix. 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. This commit also removes todo_wine from multiple now-successful tests. --- dlls/ntdll/unix/socket.c | 24 ++-------------- dlls/ws2_32/socket.c | 13 +++++---- dlls/ws2_32/tests/sock.c | 28 ++++++++---------- server/sock.c | 61 ++++++++++++++++++++++++++++++++++++++-- 4 files changed, 78 insertions(+), 48 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index e09bf34b07d..32eba1af088 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1939,30 +1939,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/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 8ef7920e0fc..9da3ccb285f 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1514,6 +1514,7 @@ int WINAPI getpeername( SOCKET s, struct sockaddr *addr, int *len ) { IO_STATUS_BLOCK io; NTSTATUS status; + int safe_len = 0; TRACE( "socket %#Ix, addr %p, len %d\n", s, addr, len ? *len : 0 ); @@ -1523,14 +1524,14 @@ int WINAPI getpeername( SOCKET s, struct sockaddr *addr, int *len ) return -1; } - if (!len) - { - SetLastError( WSAEFAULT ); - return -1; - } + /* Windows checks the validity of the socket before checking len, so + * let wineserver do the same. Since len being NULL and *len being 0 + * yield the same error, we can substitute in 0 if len is NULL. */ + if (len) + safe_len = *len; status = NtDeviceIoControlFile( (HANDLE)s, NULL, NULL, NULL, &io, - IOCTL_AFD_WINE_GETPEERNAME, NULL, 0, addr, *len ); + IOCTL_AFD_WINE_GETPEERNAME, NULL, 0, addr, safe_len ); if (!status) *len = io.Information; SetLastError( NtStatusToWSAError( status ) ); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index eddef24048c..dece4208cec 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -9329,16 +9329,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)); @@ -9397,16 +9397,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); @@ -9913,7 +9913,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); - todo_wine ok(WSAGetLastError() == WSAENOTCONN, + ok(WSAGetLastError() == WSAENOTCONN, "Expected WSAGetLastError() to return WSAENOTCONN, got %d\n", WSAGetLastError()); memset(&sa, 0, sizeof(sa)); @@ -9928,7 +9928,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); - todo_wine ok(WSAGetLastError() == WSAENOTCONN, + ok(WSAGetLastError() == WSAENOTCONN, "Expected WSAGetLastError() to return WSAENOTCONN, got %d\n", WSAGetLastError()); ret = connect(sock, (struct sockaddr*)&sa, sizeof(sa)); @@ -12629,13 +12629,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()); - if (!ret) - { - ok(addr.sin_family == AF_INET, "got family %u\n", addr.sin_family); - ok(addr.sin_addr.s_addr == inet_addr("192.0.2.0"), "got address %#08lx\n", addr.sin_addr.s_addr); - ok(addr.sin_port == 255, "expected nonzero port\n"); - } + ok(!ret, "got error %u\n", WSAGetLastError()); ret = recv(client, buffer, sizeof(buffer), 0); ok(ret == -1, "got %d\n", ret); diff --git a/server/sock.c b/server/sock.c index 52175b08987..48fb8fc0a65 100644 --- a/server/sock.c +++ b/server/sock.c @@ -242,6 +242,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 */ @@ -1725,6 +1727,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; @@ -2034,8 +2038,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; @@ -2090,7 +2101,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; @@ -2565,7 +2582,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; @@ -2627,6 +2644,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) { @@ -2649,8 +2667,10 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) allow_fd_caching( sock->fd ); 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) ); + 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) @@ -2985,6 +3005,41 @@ 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 ConnectEx() hasn't finished connecting (or failing to connect) the provided + * socket, getpeername() can't be called on it. This seems to be undocumented + * and is *not* the case for connect(), but we do test for it in ws2_32. + * connect_req is non-NULL iff ConnectEx() was used and has not finished, + * so we can use it as a check for ConnectEx() usage here. */ + if (sock->connect_req) + { + 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
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=137805 Your paranoid android. === w1064_2qxl (64 bit report) === ws2_32: sock.c:13015: Test failed: wait timed out sock.c:13031: Test failed: wait timed out sock.c:13044: Test failed: got size 5 sock.c:13045: Test failed: got "datad" sock.c:12879: Test failed: got size 0 sock.c:12879: Test failed: got size 0 sock.c:12880: Test failed: got "" sock.c:12880: Test failed: got "" === w10pro64_ar (64 bit report) === ws2_32: sock.c:6687: Test failed: wait timed out sock.c:9303: Test failed: got "\x08\xfa\x7f"
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3074
participants (4)
-
Ally Sommers -
Ally Sommers (＠dropbear)
-
Marvin -
Zebediah Figura (＠zfigura)