[PATCH v7 0/3] MR3074: Move getpeername() implementation to wineserver

[Ally Sommers (＠dropbear)]

`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

[PATCH v7 1/3] server: Move getpeername() implementation from ntdll/unix.

[Ally Sommers]

From: Ally Sommers

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();

[PATCH v7 2/3] ws2_32/tests: Remove todo_wine from now-successful tests.

[Ally Sommers]

From: Ally Sommers

---
 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);

[PATCH v7 3/3] ws2_32/tests: Fix incorrect test (bug #47560)

[Ally Sommers]

From: Ally Sommers

---
 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);

[PATCH v7 3/3] ws2_32/tests: Fix incorrect test (bug #47560)

[Marvin]

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

[Ally Sommers (＠dropbear)]

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.