[PATCH v18 0/2] 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. -- v18: ntdll: Do not send data to port 0. https://gitlab.winehq.org/wine/wine/-/merge_requests/2100
From: Jason Beetham <beefers331(a)gmail.com> --- dlls/ws2_32/tests/sock.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index d29e1c768dc..1b6fcdd69cc 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -13659,6 +13659,31 @@ static void test_connect_udp(void) closesocket(client); } +static void test_WSASendto_port0(void) +{ + SOCKET s; + struct sockaddr_in addr; + char buf[] = "hello world"; + WSABUF data_buf; + DWORD bytes_sent = 0; + int ret; + + addr.sin_family = AF_INET; + addr.sin_port = htons(0); + 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, &bytes_sent, 0, (struct sockaddr *)&addr, sizeof(addr), NULL, NULL); + todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); + todo_wine ok(bytes_sent == sizeof(buf), "Failed to send full data(%Iu) only sent(%lu)\n", sizeof(buf), bytes_sent); + closesocket(s); +} + + START_TEST( sock ) { int i; @@ -13740,6 +13765,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
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 | 59 ++++++++++++++++++++++++++++++++-- dlls/ws2_32/tests/sock.c | 4 +-- include/wine/server_protocol.h | 5 +-- server/protocol.def | 1 + server/request.h | 1 + server/sock.c | 1 + server/trace.c | 1 + 7 files changed, 66 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index ac351a17a70..e65503d3612 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -295,6 +295,32 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl } } +static int sockaddr_get_port( const struct WS_sockaddr *wsaddr, int wsaddrlen ) +{ + switch (wsaddr->sa_family) + { + case WS_AF_INET: + { + struct WS_sockaddr_in win = {0}; + if (wsaddrlen < sizeof(win)) return 0; + memcpy( &win, wsaddr, sizeof(win) ); + return win.sin_port; + } + + case WS_AF_INET6: + { + struct WS_sockaddr_in6 win = {0}; + + if (wsaddrlen < sizeof(win)) return 0; + memcpy( &win, wsaddr, sizeof(win) ); + return win.sin6_port; + } + + default: + return 0; + } +} + static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_sockaddr *wsaddr, socklen_t wsaddrlen ) { memset( wsaddr, 0, wsaddrlen ); @@ -979,7 +1005,6 @@ NTSTATUS sock_read( HANDLE handle, int fd, HANDLE event, PIO_APC_ROUTINE apc, return sock_recv( handle, event, apc, apc_user, io, fd, async, 1 ); } - static NTSTATUS try_send( int fd, struct async_send_ioctl *async ) { union unix_sockaddr unix_addr; @@ -1102,6 +1127,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi BOOL nonblocking; unsigned int status; ULONG options; + unsigned short protocol; SERVER_START_REQ( send_socket ) { @@ -1111,6 +1137,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi wait_handle = wine_server_ptr_handle( reply->wait ); options = reply->options; nonblocking = reply->nonblocking; + protocol = reply->protocol; } SERVER_END_REQ; @@ -1124,7 +1151,35 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi { ULONG_PTR information; - status = try_send( fd, async ); + if (protocol == IPPROTO_UDP && async->addr && sockaddr_get_port(async->addr, async->addr_len) == 0 ) + { + /* 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 didn't send anything. + * Matching the Windows behaviour, making the program work(in theory). + */ + + size_t skipping = 0; + struct iovec *start = async->iov + async->iov_cursor; + + for(ssize_t i = async->iov_cursor; i < async->count; i++) + { + skipping += start[i].iov_len; + } + + WARN("Attempting to send to port 0, skipping over %zd bytes\n", skipping); + async->sent_len += skipping; + + while (async->iov_cursor < async->count && skipping >= async->iov[async->iov_cursor].iov_len) + skipping -= async->iov[async->iov_cursor++].iov_len; + + status = STATUS_SUCCESS; + } + else + { + status = try_send( fd, async ); + } + if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 1b6fcdd69cc..7d1ff7f7aa9 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -13678,8 +13678,8 @@ static void test_WSASendto_port0(void) ok(s != INVALID_SOCKET, "failed to create socket, error %u\n", WSAGetLastError()); ret = WSASendTo(s, &data_buf, 1, &bytes_sent, 0, (struct sockaddr *)&addr, sizeof(addr), NULL, NULL); - todo_wine ok(!ret, "got error %u\n", WSAGetLastError()); - todo_wine ok(bytes_sent == sizeof(buf), "Failed to send full data(%Iu) only sent(%lu)\n", sizeof(buf), bytes_sent); + ok(!ret, "got error %u\n", WSAGetLastError()); + ok(bytes_sent == sizeof(buf), "Failed to send full data(%Iu) only sent(%lu)\n", sizeof(buf), bytes_sent); closesocket(s); } diff --git a/include/wine/server_protocol.h b/include/wine/server_protocol.h index 472c0ea709d..d63a2c3d57f 100644 --- a/include/wine/server_protocol.h +++ b/include/wine/server_protocol.h @@ -1810,7 +1810,8 @@ struct send_socket_reply obj_handle_t wait; unsigned int options; int nonblocking; - char __pad_20[4]; + unsigned short protocol; + char __pad_22[2]; }; @@ -6356,7 +6357,7 @@ union generic_reply /* ### protocol_version begin ### */ -#define SERVER_PROTOCOL_VERSION 758 +#define SERVER_PROTOCOL_VERSION 760 /* ### protocol_version end ### */ diff --git a/server/protocol.def b/server/protocol.def index 8c2fbeb4afe..7986a852b62 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1485,6 +1485,7 @@ enum server_fd_type obj_handle_t wait; /* handle to wait on for blocking send */ unsigned int options; /* device open options */ int nonblocking; /* is socket non-blocking? */ + unsigned short protocol; /* the protocol the socket is presently using */ @END diff --git a/server/request.h b/server/request.h index 089af79e199..11529d92b55 100644 --- a/server/request.h +++ b/server/request.h @@ -1062,6 +1062,7 @@ C_ASSERT( sizeof(struct send_socket_request) == 64 ); C_ASSERT( FIELD_OFFSET(struct send_socket_reply, wait) == 8 ); C_ASSERT( FIELD_OFFSET(struct send_socket_reply, options) == 12 ); C_ASSERT( FIELD_OFFSET(struct send_socket_reply, nonblocking) == 16 ); +C_ASSERT( FIELD_OFFSET(struct send_socket_reply, protocol) == 20 ); C_ASSERT( sizeof(struct send_socket_reply) == 24 ); C_ASSERT( FIELD_OFFSET(struct socket_send_icmp_id_request, handle) == 12 ); C_ASSERT( FIELD_OFFSET(struct socket_send_icmp_id_request, icmp_id) == 16 ); diff --git a/server/sock.c b/server/sock.c index a64cb22404e..ab35ec22a02 100644 --- a/server/sock.c +++ b/server/sock.c @@ -3917,6 +3917,7 @@ DECL_HANDLER(send_socket) reply->wait = async_handoff( async, NULL, 0 ); reply->options = get_fd_options( fd ); reply->nonblocking = sock->nonblocking; + reply->protocol = sock->proto; release_object( async ); } release_object( sock ); diff --git a/server/trace.c b/server/trace.c index a0076d5449b..443e0bf993c 100644 --- a/server/trace.c +++ b/server/trace.c @@ -2060,6 +2060,7 @@ static void dump_send_socket_reply( const struct send_socket_reply *req ) fprintf( stderr, " wait=%04x", req->wait ); fprintf( stderr, ", options=%08x", req->options ); fprintf( stderr, ", nonblocking=%d", req->nonblocking ); + fprintf( stderr, ", protocol=%04x", req->protocol ); } static void dump_socket_send_icmp_id_request( const struct socket_send_icmp_id_request *req ) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2100
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
} }
+static int sockaddr_get_port( const struct WS_sockaddr *wsaddr, int wsaddrlen ) +{ + switch (wsaddr->sa_family) + { + case WS_AF_INET: + { + struct WS_sockaddr_in win = {0}; + if (wsaddrlen < sizeof(win)) return 0;
This will end up hitting the zero-port case if the address is invalid, which doesn't seem right. Probably "is port zero" is a better semantics for this function, as an earlier iteration. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23775
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
} }
+static int sockaddr_get_port( const struct WS_sockaddr *wsaddr, int wsaddrlen ) +{ + switch (wsaddr->sa_family) + { + case WS_AF_INET: + { + struct WS_sockaddr_in win = {0}; + if (wsaddrlen < sizeof(win)) return 0; + memcpy( &win, wsaddr, sizeof(win) );
No need to memcpy; you can just cast "wsaddr" to the right type. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23776
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
return sock_recv( handle, event, apc, apc_user, io, fd, async, 1 ); }
-
Stray whitespace change here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23777
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
{ ULONG_PTR information;
- status = try_send( fd, async ); + if (protocol == IPPROTO_UDP && async->addr && sockaddr_get_port(async->addr, async->addr_len) == 0 )
The protocol that the server returned is in WS form, so this should be WS_IPPROTO_UDP. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23778
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
- status = try_send( fd, async ); + if (protocol == IPPROTO_UDP && async->addr && sockaddr_get_port(async->addr, async->addr_len) == 0 ) + { + /* 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 didn't send anything. + * Matching the Windows behaviour, making the program work(in theory). + */ + + size_t skipping = 0; + struct iovec *start = async->iov + async->iov_cursor; + + for(ssize_t i = async->iov_cursor; i < async->count; i++) + { + skipping += start[i].iov_len;
You can just use "async->sent_len += async->iov[i].iov_len", and get rid of both local variables. iov_cursor is always zero at this point. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23781
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
{ ULONG_PTR information;
- status = try_send( fd, async ); + if (protocol == IPPROTO_UDP && async->addr && sockaddr_get_port(async->addr, async->addr_len) == 0 ) + { + /* 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 didn't send anything. + * Matching the Windows behaviour, making the program work(in theory). + */ + + size_t skipping = 0; + struct iovec *start = async->iov + async->iov_cursor; + + for(ssize_t i = async->iov_cursor; i < async->count; i++)
Some compilers can't handle the variable declaration here; you'll need to move it outside the loop. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23780
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
{ ULONG_PTR information;
- status = try_send( fd, async ); + if (protocol == IPPROTO_UDP && async->addr && sockaddr_get_port(async->addr, async->addr_len) == 0 ) + { + /* 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 didn't send anything. + * Matching the Windows behaviour, making the program work(in theory).
I think you can just delete the last two sentences. I'd also just simplify and say "Spellforce 3 sends data to port 0", no need to generalize that to "some applications". -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23779
Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c:
+ * Matching the Windows behaviour, making the program work(in theory). + */ + + size_t skipping = 0; + struct iovec *start = async->iov + async->iov_cursor; + + for(ssize_t i = async->iov_cursor; i < async->count; i++) + { + skipping += start[i].iov_len; + } + + WARN("Attempting to send to port 0, skipping over %zd bytes\n", skipping); + async->sent_len += skipping; + + while (async->iov_cursor < async->count && skipping >= async->iov[async->iov_cursor].iov_len) + skipping -= async->iov[async->iov_cursor++].iov_len; What's this for? You're not using this variable later...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23782
participants (3)
-
Jason Beetham -
Jason Beetham (＠beef331)
-
Zebediah Figura (＠zfigura)