[PATCH v2 0/1] MR2100: ntdll: Do not send data to port 0.
On Windows it seems sending to port 0 does nothing and does not error. Presently sendmsg errors with EINVAL. This works around it, by checking if it's port 0 then skipping the data. -- v2: ntdll: Do not send data to port 0. https://gitlab.winehq.org/wine/wine/-/merge_requests/2100
From: Jason Beetham <beefers331(a)gmail.com> On Windows it seems sending to port 0 does nothing and does not error. Presently sendmsg errors with EINVAL. This works around it, by checking if it's port 0 then skipping the data. --- dlls/ntdll/unix/socket.c | 67 ++++++++++++++++++++++++++++++++++------ dlls/ws2_32/tests/sock.c | 25 +++++++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index ac351a17a70..93774188495 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -980,6 +980,42 @@ NTSTATUS sock_read( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE apc, } +static int isPort0( const union unix_sockaddr *uaddr) +{ + switch (uaddr->addr.sa_family) + { + case AF_INET: + { + return uaddr->in.sin_port == 0; + } + + case AF_INET6: + { + return uaddr->in6.sin6_port == 0; + } + + #ifdef HAS_IPX + case AF_IPX: + { + return uaddr->ipx.sipx_port == 0; + } + #endif + + #ifdef HAS_IRDA + case AF_IRDA: + { + return FALSE + } + #endif + + case AF_UNSPEC: + return FALSE; + + default: + return FALSE; + } +} + static NTSTATUS try_send( int fd, struct async_send_ioctl *async ) { union unix_sockaddr unix_addr; @@ -1018,17 +1054,30 @@ static NTSTATUS try_send( int fd, struct async_send_ioctl *async ) hdr.msg_iov = async->iov + async->iov_cursor; hdr.msg_iovlen = async->count - async->iov_cursor; - while ((ret = sendmsg( fd, &hdr, async->unix_flags )) == -1) - { - if (errno == EISCONN) - { - hdr.msg_name = NULL; - hdr.msg_namelen = 0; + if(isPort0(&unix_addr)){ + /* Some Windows applications(Spellforce 3 is known to) send to port 0. + * This causes 'sendmsg' to throw a EINVAL error on Windows this does nothing but consume the data. + * This workaround says we successfully sent data even though we didnt send anything. + * Matching the Windows behaviour, making the program work(in theory). + */ + ret = 0; + for(int i = 0; i < hdr.msg_iovlen; i++){ + ret += hdr.msg_iov[i].iov_len; } - else if (errno != EINTR) + WARN("Attempting to send to port 0, skipping over %zd bytes\n", ret); + }else{ + while ((ret = sendmsg( fd, &hdr, async->unix_flags )) == -1) { - if (errno != EWOULDBLOCK) WARN( "sendmsg: %s\n", strerror( errno ) ); - return sock_errno_to_status( errno ); + if (errno == EISCONN) + { + hdr.msg_name = NULL; + hdr.msg_namelen = 0; + } + else if (errno != EINTR) + { + if (errno != EWOULDBLOCK) WARN( "sendmsg: %s\n", strerror( errno ) ); + return sock_errno_to_status( errno ); + } } } diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index d29e1c768dc..74b4cf1a30f 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -13659,6 +13659,30 @@ static void test_connect_udp(void) closesocket(client); } +static void test_WSAsendto_port0(void) +{ + SOCKET s; + struct sockaddr_in addr; + char buf[12] = "hello world"; + WSABUF data_buf; + DWORD bytesSent; + int ret; + + addr.sin_family = AF_INET; + addr.sin_port = htons(139); + addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + data_buf.len = sizeof(buf); + data_buf.buf = buf; + + s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + ok(s != INVALID_SOCKET, "failed to create socket, error %u\n", WSAGetLastError()); + + ret = WSASendTo(s, &data_buf, 1, &bytesSent, 0, (struct sockaddr *)&addr, sizeof(addr), NULL, NULL); + ok(!ret, "failed to send data to port 0, error: %lu\n", GetLastError()); + ok(bytesSent == sizeof(buf), "Failed to send full data to port 0, error: %lu\n", GetLastError()); +} + + START_TEST( sock ) { int i; @@ -13740,6 +13764,7 @@ START_TEST( sock ) test_tcp_reset(); test_icmp(); test_connect_udp(); + test_WSAsendto_port0(); /* this is an io heavy test, do it at the end so the kernel doesn't start dropping packets */ test_send(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2100
I'm not sure if it's required, but I recommend you to separate the tests and implementations in two commits. So you would write a commit writting the tests to make sure they fail in wine and pass in windows (remember to mark the failing tests with todo_wine), and then fix them with another commit, removing the todo_wine marks. This way it's more clear what is broken and what you are fixing, and also makes it easier for you to make sure that your tests work fine. You can check other tests to see how the todo_wine macro is used. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23031
Santino Mazza (@tati1454) commented about dlls/ws2_32/tests/sock.c:
+static void test_WSAsendto_port0(void) +{ + SOCKET s; + struct sockaddr_in addr; + char buf[12] = "hello world"; + WSABUF data_buf; + DWORD bytesSent; + int ret; + + addr.sin_family = AF_INET; + addr.sin_port = htons(139); + addr.sin_addr.s_addr = inet_addr("127.0.0.1"); + data_buf.len = sizeof(buf); + data_buf.buf = buf; + + s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); Remember to close the socket
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23032
Santino Mazza (@tati1454) commented about dlls/ws2_32/tests/sock.c:
closesocket(client); }
+static void test_WSAsendto_port0(void) +{ + SOCKET s; + struct sockaddr_in addr; + char buf[12] = "hello world"; + WSABUF data_buf; + DWORD bytesSent; + int ret; + + addr.sin_family = AF_INET; + addr.sin_port = htons(139);
Shouldn't this be 0, instead of 139? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23033
On Mon Feb 6 01:12:00 2023 +0000, Santino Mazza wrote:
Shouldn't this be 0, instead of 139? Of course, I'm just copy pasting myself around for a bit, hence changing this to a draft.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23034
participants (3)
-
Jason Beetham -
Jason Beetham (＠beef331)
-
Santino Mazza (＠tati1454)