[PATCH v15 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. -- v15: 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 | 53 +++++++++++++++++++++++++++------- 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, 51 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index ac351a17a70..fd4b933d11f 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -979,13 +979,26 @@ 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 BOOL is_port0( 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; + default: + return FALSE; + } +} -static NTSTATUS try_send( int fd, struct async_send_ioctl *async ) +static NTSTATUS try_send( int fd, struct async_send_ioctl *async, BOOL known_udp_socket) { union unix_sockaddr unix_addr; struct msghdr hdr; ssize_t ret; + memset( &hdr, 0, sizeof(hdr) ); if (async->addr) { @@ -1018,17 +1031,34 @@ 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 (known_udp_socket && is_port0(&unix_addr)) { - if (errno == EISCONN) + /* 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(ssize_t i = 0; i < hdr.msg_iovlen; i++) { - hdr.msg_name = NULL; - hdr.msg_namelen = 0; + 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 ); + } } } @@ -1057,7 +1087,7 @@ static BOOL async_send_proc( void *user, ULONG_PTR *info, unsigned int *status ) if ((*status = server_get_unix_fd( async->io.handle, 0, &fd, &needs_close, NULL, NULL ))) return TRUE; - *status = try_send( fd, async ); + *status = try_send( fd, async, FALSE ); TRACE( "got status %#x\n", *status ); if (needs_close) close( fd ); @@ -1102,6 +1132,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 +1142,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; @@ -1123,8 +1155,7 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi if (status == STATUS_ALERTED) { ULONG_PTR information; - - status = try_send( fd, async ); + status = try_send( fd, async, protocol == IPPROTO_UDP ); 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..ca0527ae438 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(%lu) only sent(%lu)\n", (unsigned long) sizeof(buf), (unsigned long) 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..b75ef81be9a 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 = (int) 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
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=129246 Your paranoid android. === w1064_2qxl (64 bit report) === ws2_32: sock.c:6661: Test failed: wait timed out sock.c:248: Test failed: WSAPoll() returned -1
"known_udp_socket" seems wrong; we shouldn't have to guess. This may suggest it's better to handle this logic inside of sock_send() rather than try_send(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2100#note_23550
participants (4)
-
Jason Beetham -
Jason Beetham (＠beef331)
-
Marvin -
Zebediah Figura (＠zfigura)