[PATCH v7 0/9] MR10954: nsiproxy.sys: Reuse icmp sockets for multiple requests and support receiving error packets with Linux ping sockets.

[Paul Gofman (＠gofman)]

--
  v7: nsiproxy.sys: Use compatible ICMP request id and seq numbers.
      nsiproxy.sys: Support ICMP error replies with ping sockets.
      nsiproxy.sys: Reuse icmp socket with matching parameters.
      nsiproxy.sys: Return ICMP ids from parse_icmp_hdr() instead of matching those.
      nsiproxy.sys: Poll all icmp sockets in a single thread.
      nsiproxy.sys: Manage ICMP listen threads on the Unix side.
      nsiproxy.sys: Contain all the reply data in struct icmp_reply_ctx.
      nsiproxy.sys: Rename icmp_listen into icmp_get_reply.
      nsiproxy.sys: Factor out icmp_socket structure.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 1/9] nsiproxy.sys: Factor out icmp_socket structure.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 188 +++++++++++++++++++---------------
 1 file changed, 106 insertions(+), 82 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index af1139e2d48..8d7502b2d85 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -126,20 +126,28 @@ struct icmp_reply_ctx
 };
 
 struct family_ops;
-struct icmp_data
+
+struct icmp_socket
 {
-    LARGE_INTEGER send_time;
+    LONG ref;
     int socket;
-    int cancel_pipe[2];
-    unsigned short id;
-    unsigned short seq;
     const struct family_ops *ops;
     struct sockaddr_storage src_storage;
-    struct sockaddr_storage dst_storage;
     int src_len;
-    int dst_len;
     int hop_limit;
+    BYTE ttl, tos;
     BOOL ping_socket;
+    unsigned short id;
+};
+
+struct icmp_data
+{
+    LARGE_INTEGER send_time;
+    int cancel_pipe[2];
+    unsigned short seq;
+    struct sockaddr_storage dst_storage;
+    int dst_len;
+    struct icmp_socket *s;
 };
 
 #define MAX_HANDLES 256 /* Max number of simultaneous pings - could become dynamic if need be */
@@ -207,7 +215,7 @@ static void ipv4_init_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp_hd
     icmp_hdr->type = ICMP4_ECHO_REQUEST;
     icmp_hdr->code = 0;
     icmp_hdr->checksum = 0;
-    icmp_hdr->un.echo.id = data->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
+    icmp_hdr->un.echo.id = data->s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
     icmp_hdr->un.echo.sequence = data->seq = InterlockedIncrement( &icmp_sequence ) & 0xffff;
 }
 
@@ -217,7 +225,7 @@ static unsigned short chksum( struct icmp_data *icmp_data, BYTE *data, unsigned
     unsigned int sum = 0, carry = 0;
     unsigned short check, s;
 
-    if (icmp_data->ping_socket) return 0;
+    if (icmp_data->s->ping_socket) return 0;
 
     while (count > 1)
     {
@@ -259,25 +267,25 @@ static int ipv4_set_reply_ip_status( IP_STATUS ip_status, unsigned int bits, voi
     }
 }
 
-static void ipv4_set_socket_opts( struct icmp_data *data, struct icmp_send_echo_params *params )
+static void ipv4_set_socket_opts( struct icmp_socket *s )
 {
     int val;
 
-    val = params->ttl;
-    if (val) setsockopt( data->socket, IPPROTO_IP, IP_TTL, &val, sizeof(val) );
-    val = params->tos;
-    if (val) setsockopt( data->socket, IPPROTO_IP, IP_TOS, &val, sizeof(val) );
+    val = s->ttl;
+    if (val) setsockopt( s->socket, IPPROTO_IP, IP_TTL, &val, sizeof(val) );
+    val = s->tos;
+    if (val) setsockopt( s->socket, IPPROTO_IP, IP_TOS, &val, sizeof(val) );
 }
 
 #ifdef __linux__
-static void ipv4_linux_ping_set_socket_opts( struct icmp_data *data, struct icmp_send_echo_params *params )
+static void ipv4_linux_ping_set_socket_opts( struct icmp_socket *s )
 {
     static const int val = 1;
 
-    ipv4_set_socket_opts( data, params );
+    ipv4_set_socket_opts( s );
 
-    setsockopt( data->socket, IPPROTO_IP, IP_RECVTTL, &val, sizeof(val) );
-    setsockopt( data->socket, IPPROTO_IP, IP_RECVTOS, &val, sizeof(val) );
+    setsockopt( s->socket, IPPROTO_IP, IP_RECVTTL, &val, sizeof(val) );
+    setsockopt( s->socket, IPPROTO_IP, IP_RECVTOS, &val, sizeof(val) );
 }
 #endif
 
@@ -374,7 +382,7 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     switch (icmp->type)
     {
     case ICMP4_ECHO_REPLY:
-        if ((!data->ping_socket && icmp->un.echo.id != data->id) ||
+        if ((!data->s->ping_socket && icmp->un.echo.id != data->s->id) ||
             icmp->un.echo.sequence != data->seq) return -1;
 
         ctx->status = IP_SUCCESS;
@@ -406,7 +414,7 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
         return -1;
     }
 
-    if (data->ping_socket) return 0;
+    if (data->s->ping_socket) return 0;
 
     /* Check that the appended packet is really ours -
      * all handled icmp replies have an 8-byte header
@@ -419,7 +427,7 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + orig_ip_hdr_len);
     if (orig_icmp_hdr->type != ICMP4_ECHO_REQUEST ||
         orig_icmp_hdr->code != 0 ||
-        (!data->ping_socket && orig_icmp_hdr->un.echo.id != data->id) ||
+        (!data->s->ping_socket && orig_icmp_hdr->un.echo.id != data->s->id) ||
         orig_icmp_hdr->un.echo.sequence != data->seq) return -1;
 
     ctx->status = status;
@@ -480,7 +488,7 @@ struct family_ops
     void (*init_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp_hdr );
     unsigned short (*chksum)( struct icmp_data *icmp_data, BYTE *data, unsigned int count );
     int (*set_reply_ip_status)( IP_STATUS ip_status, unsigned int bits, void *out );
-    void (*set_socket_opts)( struct icmp_data *data, struct icmp_send_echo_params *params );
+    void (*set_socket_opts)( struct icmp_socket *s );
     int (*reply_buffer_len)( struct icmp_listen_params *params );
     BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
     int (*parse_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
@@ -523,7 +531,7 @@ static void ipv6_init_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp_hd
     icmp_hdr->type = ICMP6_ECHO_REQUEST;
     icmp_hdr->code = 0;
     icmp_hdr->checksum = 0;
-    icmp_hdr->un.echo.id = data->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
+    icmp_hdr->un.echo.id = data->s->id;
     icmp_hdr->un.echo.sequence = data->seq = InterlockedIncrement( &icmp_sequence ) & 0xffff;
 }
 
@@ -535,7 +543,7 @@ static unsigned short ipv6_chksum( struct icmp_data *icmp_data, BYTE *data, unsi
     socklen_t slen;
     int s, ret;
 
-    if (icmp_data->ping_socket) return 0;
+    if (icmp_data->s->ping_socket) return 0;
 
     /* Determine source address. Do it on a separate socket or raw socket won't receive ICMP replies
      * originating not from the destination address. */
@@ -550,7 +558,7 @@ static unsigned short ipv6_chksum( struct icmp_data *icmp_data, BYTE *data, unsi
             return 0;
         }
     }
-    if (bind( s, (void *)&icmp_data->src_storage, icmp_data->src_len ))
+    if (bind( s, (void *)&icmp_data->s->src_storage, icmp_data->s->src_len ))
     {
         close( s );
         return 0;
@@ -587,13 +595,13 @@ static int ipv6_set_reply_ip_status( IP_STATUS ip_status, unsigned int bits, voi
     return sizeof(*reply);
 }
 
-static void ipv6_set_socket_opts( struct icmp_data *data, struct icmp_send_echo_params *params )
+static void ipv6_set_socket_opts( struct icmp_socket *s )
 {
 #ifdef IPV6_UNICAST_HOPS
 {
-    int val = params->hop_limit;
+    int val = s->hop_limit;
 
-    setsockopt( data->socket, IPPROTO_IPV6, IPV6_UNICAST_HOPS, &val, sizeof(val) );
+    setsockopt( s->socket, IPPROTO_IPV6, IPV6_UNICAST_HOPS, &val, sizeof(val) );
 }
 #endif
 }
@@ -636,7 +644,7 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
     switch (icmp->type)
     {
     case ICMP6_ECHO_REPLY:
-        if ((!data->ping_socket && icmp->un.echo.id != data->id) ||
+        if ((!data->s->ping_socket && icmp->un.echo.id != data->s->id) ||
             icmp->un.echo.sequence != data->seq) return -1;
 
         ctx->status = IP_SUCCESS;
@@ -670,7 +678,7 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
         return -1;
     }
 
-    if (data->ping_socket) return 0;
+    if (data->s->ping_socket) return 0;
     /* Check that the appended packet is really ours. */
     if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr)) return -1;
     orig_ip_hdr = (struct ipv6_hdr *)(icmp + 1);
@@ -679,7 +687,7 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
     orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + sizeof(*orig_ip_hdr));
     if (orig_icmp_hdr->type != ICMP6_ECHO_REQUEST ||
         orig_icmp_hdr->code != 0 ||
-        (!data->ping_socket && orig_icmp_hdr->un.echo.id != data->id) ||
+        (!data->s->ping_socket && orig_icmp_hdr->un.echo.id != data->s->id) ||
         orig_icmp_hdr->un.echo.sequence != data->seq) return -1;
 
     ctx->status = status;
@@ -782,100 +790,116 @@ static BOOL sockaddr_to_SOCKADDR_INET( const struct sockaddr *in, SOCKADDR_INET
     return FALSE;
 }
 
-static NTSTATUS icmp_data_create( ADDRESS_FAMILY win_family, struct icmp_data **icmp_data )
+static void icmp_release_socket( struct icmp_socket *s )
+{
+    if (InterlockedDecrement( &s->ref)) return;
+    if (s->socket >= 0) close( s->socket );
+    free( s );
+}
+
+static void icmp_data_free( struct icmp_data *data )
 {
+    icmp_release_socket( data->s );
+    close( data->cancel_pipe[0] );
+    close( data->cancel_pipe[1] );
+    free( data );
+}
+
+static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct icmp_data **icmp_data )
+{
+    struct sockaddr *src, *dst;
     struct icmp_data *data;
     const struct family_ops *ops;
 
-    if (win_family == WS_AF_INET6)     ops = &ipv6;
-    else if (win_family == WS_AF_INET) ops = &ipv4;
+    if (params->dst->si_family == WS_AF_INET6)     ops = &ipv6;
+    else if (params->dst->si_family == WS_AF_INET) ops = &ipv4;
     else return STATUS_INVALID_PARAMETER;
 
     data = malloc( sizeof(*data) );
     if (!data) return STATUS_NO_MEMORY;
-
-    data->ping_socket = FALSE;
-    data->socket = socket( ops->family, SOCK_RAW, ops->icmp_protocol );
-    if (data->socket < 0) /* Try a ping-socket */
+    if (!(data->s = calloc( 1, sizeof(*data->s) )))
+    {
+        free( data );
+        return STATUS_NO_MEMORY;
+    }
+    data->s->ref = 1;
+    data->s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
+    data->s->ping_socket = FALSE;
+    data->s->socket = socket( ops->family, SOCK_RAW, ops->icmp_protocol );
+    if (data->s->socket < 0) /* Try a ping-socket */
     {
         TRACE( "failed to open raw sock, trying a dgram sock\n" );
-        data->socket = socket( ops->family, SOCK_DGRAM, ops->icmp_protocol );
-        if (data->socket < 0)
+        data->s->socket = socket( ops->family, SOCK_DGRAM, ops->icmp_protocol );
+        if (data->s->socket < 0)
         {
             WARN( "Unable to create socket\n" );
+            icmp_release_socket( data->s );
             free( data );
             return STATUS_ACCESS_DENIED;
         }
 #ifdef __linux__
         if (ops->family == AF_INET) ops = &ipv4_linux_ping;
-        data->ping_socket = TRUE;
+        data->s->ping_socket = TRUE;
 #endif
     }
     if (pipe( data->cancel_pipe ))
     {
-        close( data->socket );
+        icmp_release_socket( data->s );
         free( data );
         return STATUS_ACCESS_DENIED;
     }
 
-    data->ops = ops;
+    data->s->hop_limit = params->hop_limit;
+    data->s->ttl = params->ttl;
+    data->s->tos = params->tos;
+    src = (struct sockaddr *)&data->s->src_storage;
+    dst = (struct sockaddr *)&data->dst_storage;
+    data->s->src_len = SOCKADDR_INET_to_sockaddr( params->src, src, sizeof(data->s->src_storage) );
+    data->dst_len = SOCKADDR_INET_to_sockaddr( params->dst, dst, sizeof(data->dst_storage) );
+
+    if (bind( data->s->socket, src, data->s->src_len ))
+    {
+        icmp_data_free( data );
+        return STATUS_INVALID_ADDRESS_COMPONENT;
+    }
+
+    data->s->ops = ops;
+    data->s->ops->set_socket_opts( data->s );
     *icmp_data = data;
     return STATUS_SUCCESS;
 }
 
-static void icmp_data_free( struct icmp_data *data )
-{
-    close( data->socket );
-    close( data->cancel_pipe[0] );
-    close( data->cancel_pipe[1] );
-    free( data );
-}
-
 NTSTATUS icmp_send_echo( void *args )
 {
     struct icmp_send_echo_params *params = args;
     struct icmp_hdr *icmp_hdr; /* this is the same for both ipv4 and ipv6 */
     struct icmp_data *data;
     int ret;
-    struct sockaddr *src, *dst;
 
     NTSTATUS status;
 
-    status = icmp_data_create( params->dst->si_family, &data );
+    status = icmp_data_create( params, &data );
     if (status) return status;
 
-    data->hop_limit = params->hop_limit;
-    src = (struct sockaddr *)&data->src_storage;
-    dst = (struct sockaddr *)&data->dst_storage;
-    data->src_len = SOCKADDR_INET_to_sockaddr( params->src, src, sizeof(data->src_storage) );
-    data->dst_len = SOCKADDR_INET_to_sockaddr( params->dst, dst, sizeof(data->dst_storage) );
-
-    if (bind( data->socket, src, data->src_len ))
-    {
-        icmp_data_free( data );
-        return STATUS_INVALID_ADDRESS_COMPONENT;
-    }
-
-    data->ops->set_socket_opts( data, params );
-
     icmp_hdr = malloc( sizeof(*icmp_hdr) + params->request_size );
     if (!icmp_hdr)
     {
         icmp_data_free( data );
         return STATUS_NO_MEMORY;
     }
-    data->ops->init_icmp_hdr( data, icmp_hdr );
+    data->s->ops->init_icmp_hdr( data, icmp_hdr );
     memcpy( icmp_hdr + 1, params->request, params->request_size );
-    icmp_hdr->checksum = data->ops->chksum( data, (BYTE *)icmp_hdr, sizeof(*icmp_hdr) + params->request_size );
+    icmp_hdr->checksum = data->s->ops->chksum( data, (BYTE *)icmp_hdr, sizeof(*icmp_hdr) + params->request_size );
 
     NtQueryPerformanceCounter( &data->send_time, NULL );
-    ret = sendto( data->socket, icmp_hdr, sizeof(*icmp_hdr) + params->request_size, 0, dst, data->dst_len );
+    ret = sendto( data->s->socket, icmp_hdr, sizeof(*icmp_hdr) + params->request_size, 0,
+                  (struct sockaddr *)&data->dst_storage, data->dst_len );
     free( icmp_hdr );
 
     if (ret < 0)
     {
         TRACE( "sendto() rets %d errno %d\n", ret, errno );
-        params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
+        params->reply_len = data->s->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
         icmp_data_free( data );
         return STATUS_SUCCESS;
     }
@@ -917,27 +941,27 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *par
     char *reply_buf;
     struct icmp_hdr *icmp_hdr;
 
-    reply_buf_len = data->ops->reply_buffer_len( params );
+    reply_buf_len = data->s->ops->reply_buffer_len( params );
     reply_buf = malloc( reply_buf_len );
     if (!reply_buf) return STATUS_NO_MEMORY;
 
     iov[0].iov_base = reply_buf;
     iov[0].iov_len = reply_buf_len;
 
-    recvd = recvmsg( data->socket, &msg, 0 );
+    recvd = recvmsg( data->s->socket, &msg, 0 );
     TRACE( "recvmsg() rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
            recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
 
     if (recvd < 0) goto skip;
-    if (!data->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) goto skip;
+    if (!data->s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) goto skip;
     if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) goto skip;
 
     icmp_hdr = (struct icmp_hdr *)(reply_buf + ip_hdr_len);
-    if ((ctx.data_size = data->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0) goto skip;
+    if ((ctx.data_size = data->s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0) goto skip;
     if (ctx.data_size && msg.msg_flags & MSG_TRUNC)
     {
         free( reply_buf );
-        params->reply_len = data->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
         return STATUS_SUCCESS;
     }
 
@@ -945,7 +969,7 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *par
     ctx.round_trip_time = get_rtt( data->send_time );
     ctx.data = icmp_hdr + 1;
 
-    data->ops->fill_reply( params, &ctx );
+    data->s->ops->fill_reply( params, &ctx );
 
     free( reply_buf );
     return STATUS_SUCCESS;
@@ -966,14 +990,14 @@ NTSTATUS icmp_listen( void *args )
     data = handle_data( params->handle );
     if (!data) return STATUS_INVALID_PARAMETER;
 
-    if (data->dst_storage.ss_family == AF_INET6 && !data->hop_limit)
+    if (data->dst_storage.ss_family == AF_INET6 && !data->s->hop_limit)
     {
         TRACE( "Invalid hop_limit.\n" );
-        params->reply_len = data->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
         return STATUS_SUCCESS;
     }
 
-    fds[0].fd = data->socket;
+    fds[0].fd = data->s->socket;
     fds[0].events = POLLIN;
     fds[1].fd = data->cancel_pipe[0];
     fds[1].events = POLLIN;
@@ -994,11 +1018,11 @@ NTSTATUS icmp_listen( void *args )
     if (!ret) /* timeout */
     {
         TRACE( "timeout\n" );
-        params->reply_len = data->ops->set_reply_ip_status( IP_REQ_TIMED_OUT, params->bits, params->reply );
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_REQ_TIMED_OUT, params->bits, params->reply );
         return STATUS_SUCCESS;
     }
     /* ret < 0 */
-    params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
+    params->reply_len = data->s->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
     return STATUS_SUCCESS;
 }
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 2/9] nsiproxy.sys: Rename icmp_listen into icmp_get_reply.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/device.c           | 12 ++++++------
 dlls/nsiproxy.sys/icmp_echo.c        | 20 ++++++++++----------
 dlls/nsiproxy.sys/nsi.c              |  2 +-
 dlls/nsiproxy.sys/nsiproxy_private.h |  2 +-
 dlls/nsiproxy.sys/unix_private.h     |  2 +-
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/dlls/nsiproxy.sys/device.c b/dlls/nsiproxy.sys/device.c
index 4595bf8020c..18adc4ec037 100644
--- a/dlls/nsiproxy.sys/device.c
+++ b/dlls/nsiproxy.sys/device.c
@@ -63,7 +63,7 @@ enum unix_calls
 {
     icmp_cancel_listen,
     icmp_close,
-    icmp_listen,
+    icmp_get_reply,
     icmp_send_echo,
     nsi_enumerate_all_ex,
     nsi_get_all_parameters_ex,
@@ -225,13 +225,13 @@ static int icmp_echo_reply_struct_len( ULONG family, ULONG bits )
     return 0;
 }
 
-static DWORD WINAPI listen_thread_proc( void *arg )
+static DWORD WINAPI icmp_wait_reply( void *arg )
 {
     IRP *irp = arg;
     IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp );
     struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer;
     struct icmp_close_params close_params;
-    struct icmp_listen_params params;
+    struct icmp_get_reply_params params;
     NTSTATUS status;
 
     TRACE( "\n" );
@@ -243,8 +243,8 @@ static DWORD WINAPI listen_thread_proc( void *arg )
     params.reply = irp->AssociatedIrp.SystemBuffer;
     params.reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength;
 
-    status = nsiproxy_call( icmp_listen, ¶ms );
-    TRACE( "icmp_listen rets %08lx\n", status );
+    status = nsiproxy_call( icmp_get_reply, ¶ms );
+    TRACE( "icmp_get_reply rets %08lx\n", status );
 
     EnterCriticalSection( &icmp_echo_completion_cs );
     close_params.handle = irp_set_icmp_handle( irp, 0 );
@@ -297,7 +297,7 @@ static NTSTATUS handle_send_echo( IRP *irp )
     }
     IoMarkIrpPending( irp );
     irp_set_icmp_handle( irp, handle );
-    RtlQueueWorkItem( listen_thread_proc, irp, WT_EXECUTELONGFUNCTION );
+    RtlQueueWorkItem( icmp_wait_reply, irp, WT_EXECUTELONGFUNCTION );
     return STATUS_PENDING;
 }
 
diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 8d7502b2d85..55eae0779c2 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -289,14 +289,14 @@ static void ipv4_linux_ping_set_socket_opts( struct icmp_socket *s )
 }
 #endif
 
-static int ipv4_reply_buffer_len( struct icmp_listen_params *params )
+static int ipv4_reply_buffer_len( struct icmp_get_reply_params *params )
 {
     int struct_len = (params->bits == 32) ? sizeof(struct icmp_echo_reply_32) : sizeof(struct icmp_echo_reply_64);
     return sizeof(struct ip_hdr) + sizeof(struct icmp_hdr) + params->reply_len - struct_len;
 }
 
 #ifdef __linux__
-static int ipv4_linux_ping_reply_buffer_len( struct icmp_listen_params *params )
+static int ipv4_linux_ping_reply_buffer_len( struct icmp_get_reply_params *params )
 {
     int struct_len = (params->bits == 32) ? sizeof(struct icmp_echo_reply_32) : sizeof(struct icmp_echo_reply_64);
     return sizeof(struct icmp_hdr) + params->reply_len - struct_len;
@@ -434,7 +434,7 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     return 0;
 }
 
-static void ipv4_fill_reply( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx)
+static void ipv4_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
 {
     void *options_data;
     ULONG data_offset;
@@ -489,10 +489,10 @@ struct family_ops
     unsigned short (*chksum)( struct icmp_data *icmp_data, BYTE *data, unsigned int count );
     int (*set_reply_ip_status)( IP_STATUS ip_status, unsigned int bits, void *out );
     void (*set_socket_opts)( struct icmp_socket *s );
-    int (*reply_buffer_len)( struct icmp_listen_params *params );
+    int (*reply_buffer_len)( struct icmp_get_reply_params *params );
     BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
     int (*parse_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
-    void (*fill_reply)( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx );
+    void (*fill_reply)( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx );
 };
 
 static const struct family_ops ipv4 =
@@ -606,7 +606,7 @@ static void ipv6_set_socket_opts( struct icmp_socket *s )
 #endif
 }
 
-static int ipv6_reply_buffer_len( struct icmp_listen_params *params )
+static int ipv6_reply_buffer_len( struct icmp_get_reply_params *params )
 {
     return sizeof(struct icmp_hdr) + params->reply_len - sizeof(ICMPV6_ECHO_REPLY);
 }
@@ -695,7 +695,7 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
     return 0;
 }
 
-static void ipv6_fill_reply( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx)
+static void ipv6_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
 {
     ICMPV6_ECHO_REPLY *reply = params->reply;
 
@@ -928,7 +928,7 @@ static ULONG get_rtt( LARGE_INTEGER start )
     return (now.QuadPart - start.QuadPart) / 10000;
 }
 
-static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *params )
+static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_get_reply_params *params )
 {
     struct sockaddr_storage addr;
     struct icmp_reply_ctx ctx;
@@ -979,9 +979,9 @@ skip:
     return STATUS_RETRY;
 }
 
-NTSTATUS icmp_listen( void *args )
+NTSTATUS icmp_get_reply( void *args )
 {
-    struct icmp_listen_params *params = args;
+    struct icmp_get_reply_params *params = args;
     struct icmp_data *data;
     struct pollfd fds[2];
     NTSTATUS status;
diff --git a/dlls/nsiproxy.sys/nsi.c b/dlls/nsiproxy.sys/nsi.c
index 7fe1e1c9274..41aace2c2c4 100644
--- a/dlls/nsiproxy.sys/nsi.c
+++ b/dlls/nsiproxy.sys/nsi.c
@@ -345,7 +345,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] =
 {
     icmp_cancel_listen,
     icmp_close,
-    icmp_listen,
+    icmp_get_reply,
     icmp_send_echo,
     unix_nsi_enumerate_all_ex,
     unix_nsi_get_all_parameters_ex,
diff --git a/dlls/nsiproxy.sys/nsiproxy_private.h b/dlls/nsiproxy.sys/nsiproxy_private.h
index ab40fa2057a..a15d594bbdd 100644
--- a/dlls/nsiproxy.sys/nsiproxy_private.h
+++ b/dlls/nsiproxy.sys/nsiproxy_private.h
@@ -30,7 +30,7 @@ struct icmp_close_params
     icmp_handle handle;
 };
 
-struct icmp_listen_params
+struct icmp_get_reply_params
 {
     icmp_handle handle;
     void *reply;
diff --git a/dlls/nsiproxy.sys/unix_private.h b/dlls/nsiproxy.sys/unix_private.h
index 5487de5111d..8a332f2ba07 100644
--- a/dlls/nsiproxy.sys/unix_private.h
+++ b/dlls/nsiproxy.sys/unix_private.h
@@ -179,5 +179,5 @@ static inline int ascii_strcasecmp( const char *s1, const char *s2 )
 
 NTSTATUS icmp_cancel_listen( void *args );
 NTSTATUS icmp_close( void *args );
-NTSTATUS icmp_listen( void *args );
+NTSTATUS icmp_get_reply( void *args );
 NTSTATUS icmp_send_echo( void *args );
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 3/9] nsiproxy.sys: Contain all the reply data in struct icmp_reply_ctx.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 95 ++++++++++++-----------------------
 1 file changed, 33 insertions(+), 62 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 55eae0779c2..0de8ec5d2e9 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -121,8 +121,10 @@ struct icmp_reply_ctx
     BYTE tos;
     BYTE flags;
     BYTE options_size;
-    void *options_data;
-    void *data;
+    unsigned int options_data_offset;
+    unsigned int data_offset;
+    BYTE packet[65536];
+    unsigned int packet_size;
 };
 
 struct family_ops;
@@ -289,20 +291,6 @@ static void ipv4_linux_ping_set_socket_opts( struct icmp_socket *s )
 }
 #endif
 
-static int ipv4_reply_buffer_len( struct icmp_get_reply_params *params )
-{
-    int struct_len = (params->bits == 32) ? sizeof(struct icmp_echo_reply_32) : sizeof(struct icmp_echo_reply_64);
-    return sizeof(struct ip_hdr) + sizeof(struct icmp_hdr) + params->reply_len - struct_len;
-}
-
-#ifdef __linux__
-static int ipv4_linux_ping_reply_buffer_len( struct icmp_get_reply_params *params )
-{
-    int struct_len = (params->bits == 32) ? sizeof(struct icmp_echo_reply_32) : sizeof(struct icmp_echo_reply_64);
-    return sizeof(struct icmp_hdr) + params->reply_len - struct_len;
-}
-#endif
-
 static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
                                struct icmp_reply_ctx *ctx )
 {
@@ -313,7 +301,7 @@ static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
     if (ip_hdr->v_hl >> 4 != 4 || ip_hdr->protocol != IPPROTO_ICMP) return FALSE;
     *ip_hdr_len = (ip_hdr->v_hl & 0xf) << 2;
     if (*ip_hdr_len < sizeof(*ip_hdr)) return FALSE;
-    ctx->options_data = ip_hdr + 1;
+    ctx->options_data_offset = sizeof(*ip_hdr);
     ctx->ttl = ip_hdr->ttl;
     ctx->tos = ip_hdr->tos;
     ctx->flags = ip_hdr->frag_off >> 13;
@@ -329,7 +317,7 @@ static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip
     struct cmsghdr *cmsg;
 
     *ip_hdr_len = 0;
-    ctx->options_data = NULL;
+    ctx->options_data_offset = 0;
     ctx->ttl = 0;
     ctx->tos = 0;
     ctx->flags = 0;
@@ -434,7 +422,7 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     return 0;
 }
 
-static void ipv4_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
+static BOOL ipv4_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
 {
     void *options_data;
     ULONG data_offset;
@@ -473,12 +461,17 @@ static void ipv4_fill_reply( struct icmp_get_reply_params *params, struct icmp_r
         options_data = reply + 1;
     }
 
-    memcpy( options_data, ctx->options_data, ctx->options_size );
+    if (ctx->options_size && ((char *)options_data - (char *)params->reply) + ctx->options_size > params->reply_len)
+        return FALSE;
+    memcpy( options_data, ctx->packet + ctx->options_data_offset, ctx->options_size );
     if (ctx->options_size & 3)
         memset( (char *)options_data + ctx->options_size, 0, 4 - (ctx->options_size & 3) );
 
-    memcpy( (char *)params->reply + data_offset, ctx->data, ctx->data_size );
+    if (ctx->data_size && data_offset + ctx->data_size > params->reply_len)
+        return FALSE;
+    memcpy( (char *)params->reply + data_offset, ctx->packet + ctx->data_offset, ctx->data_size );
     params->reply_len = data_offset + ctx->data_size;
+    return TRUE;
 }
 
 struct family_ops
@@ -489,10 +482,9 @@ struct family_ops
     unsigned short (*chksum)( struct icmp_data *icmp_data, BYTE *data, unsigned int count );
     int (*set_reply_ip_status)( IP_STATUS ip_status, unsigned int bits, void *out );
     void (*set_socket_opts)( struct icmp_socket *s );
-    int (*reply_buffer_len)( struct icmp_get_reply_params *params );
     BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
     int (*parse_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
-    void (*fill_reply)( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx );
+    BOOL (*fill_reply)( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx );
 };
 
 static const struct family_ops ipv4 =
@@ -503,7 +495,6 @@ static const struct family_ops ipv4 =
     chksum,
     ipv4_set_reply_ip_status,
     ipv4_set_socket_opts,
-    ipv4_reply_buffer_len,
     ipv4_parse_ip_hdr,
     ipv4_parse_icmp_hdr,
     ipv4_fill_reply,
@@ -519,7 +510,6 @@ static const struct family_ops ipv4_linux_ping =
     chksum,
     ipv4_set_reply_ip_status,
     ipv4_linux_ping_set_socket_opts,
-    ipv4_linux_ping_reply_buffer_len,
     ipv4_linux_ping_parse_ip_hdr,
     ipv4_parse_icmp_hdr,
     ipv4_fill_reply,
@@ -606,16 +596,11 @@ static void ipv6_set_socket_opts( struct icmp_socket *s )
 #endif
 }
 
-static int ipv6_reply_buffer_len( struct icmp_get_reply_params *params )
-{
-    return sizeof(struct icmp_hdr) + params->reply_len - sizeof(ICMPV6_ECHO_REPLY);
-}
-
 static BOOL ipv6_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
                                           struct icmp_reply_ctx *ctx )
 {
     *ip_hdr_len = 0;
-    ctx->options_data = NULL;
+    ctx->options_data_offset = 0;
     ctx->ttl = 0;
     ctx->tos = 0;
     ctx->flags = 0;
@@ -695,7 +680,7 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
     return 0;
 }
 
-static void ipv6_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
+static BOOL ipv6_fill_reply( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx)
 {
     ICMPV6_ECHO_REPLY *reply = params->reply;
 
@@ -705,8 +690,11 @@ static void ipv6_fill_reply( struct icmp_get_reply_params *params, struct icmp_r
     reply->Address.sin6_port = ctx->addr.Ipv6.sin6_port;
     reply->Address.sin6_scope_id = ctx->addr.Ipv6.sin6_scope_id;
     reply->RoundTripTime = ctx->round_trip_time;
-    memcpy( reply + 1, ctx->data, ctx->data_size );
+    if (sizeof(*reply) + ctx->data_size > params->reply_len)
+        return FALSE;
+    memcpy( reply + 1, ctx->packet + ctx->data_offset, ctx->data_size );
     params->reply_len = sizeof(*reply) + ctx->data_size;
+    return TRUE;
 }
 
 static const struct family_ops ipv6 =
@@ -717,7 +705,6 @@ static const struct family_ops ipv6 =
     ipv6_chksum,
     ipv6_set_reply_ip_status,
     ipv6_set_socket_opts,
-    ipv6_reply_buffer_len,
     ipv6_parse_ip_hdr,
     ipv6_parse_icmp_hdr,
     ipv6_fill_reply,
@@ -937,46 +924,30 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_get_reply_params *
     struct msghdr msg = { .msg_name = &addr, .msg_namelen = sizeof(addr),
                           .msg_iov = iov, .msg_iovlen = ARRAY_SIZE(iov),
                           .msg_control = cmsg_buf, .msg_controllen = sizeof(cmsg_buf) };
-    int ip_hdr_len, recvd, reply_buf_len;
-    char *reply_buf;
+    int ip_hdr_len, recvd;
     struct icmp_hdr *icmp_hdr;
 
-    reply_buf_len = data->s->ops->reply_buffer_len( params );
-    reply_buf = malloc( reply_buf_len );
-    if (!reply_buf) return STATUS_NO_MEMORY;
-
-    iov[0].iov_base = reply_buf;
-    iov[0].iov_len = reply_buf_len;
+    iov[0].iov_base = ctx.packet;
+    iov[0].iov_len = sizeof(ctx.packet);
 
     recvd = recvmsg( data->s->socket, &msg, 0 );
     TRACE( "recvmsg() rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
            recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
 
-    if (recvd < 0) goto skip;
-    if (!data->s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) goto skip;
-    if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) goto skip;
-
-    icmp_hdr = (struct icmp_hdr *)(reply_buf + ip_hdr_len);
-    if ((ctx.data_size = data->s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0) goto skip;
-    if (ctx.data_size && msg.msg_flags & MSG_TRUNC)
-    {
-        free( reply_buf );
-        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
-        return STATUS_SUCCESS;
-    }
-
+    if (recvd < 0) return STATUS_RETRY;
+    if (!data->s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) return STATUS_RETRY;
+    if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) return STATUS_RETRY;
+    ctx.packet_size = recvd;
+    icmp_hdr = (struct icmp_hdr *)(ctx.packet + ip_hdr_len);
+    if ((ctx.data_size = data->s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0) return STATUS_RETRY;
     sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr );
     ctx.round_trip_time = get_rtt( data->send_time );
-    ctx.data = icmp_hdr + 1;
+    ctx.data_offset = (BYTE *)(icmp_hdr + 1) - ctx.packet;
 
-    data->s->ops->fill_reply( params, &ctx );
+    if (!data->s->ops->fill_reply( params, &ctx ))
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
 
-    free( reply_buf );
     return STATUS_SUCCESS;
-
-skip:
-    free( reply_buf );
-    return STATUS_RETRY;
 }
 
 NTSTATUS icmp_get_reply( void *args )
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 4/9] nsiproxy.sys: Manage ICMP listen threads on the Unix side.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/device.c           |  91 +++++++++------
 dlls/nsiproxy.sys/icmp_echo.c        | 166 +++++++++++++++++----------
 dlls/nsiproxy.sys/nsi.c              |   2 -
 dlls/nsiproxy.sys/nsiproxy_private.h |  12 +-
 dlls/nsiproxy.sys/unix_private.h     |   2 -
 5 files changed, 161 insertions(+), 112 deletions(-)

diff --git a/dlls/nsiproxy.sys/device.c b/dlls/nsiproxy.sys/device.c
index 18adc4ec037..b7180d2cb94 100644
--- a/dlls/nsiproxy.sys/device.c
+++ b/dlls/nsiproxy.sys/device.c
@@ -19,6 +19,7 @@
  */
 
 #include <stdarg.h>
+#include <assert.h>
 
 #include "ntstatus.h"
 #include "windef.h"
@@ -61,8 +62,6 @@ static NTSTATUS nsiproxy_call( unsigned int code, void *args )
 
 enum unix_calls
 {
-    icmp_cancel_listen,
-    icmp_close,
     icmp_get_reply,
     icmp_send_echo,
     nsi_enumerate_all_ex,
@@ -184,35 +183,41 @@ static NTSTATUS nsiproxy_get_parameter( IRP *irp )
     return status;
 }
 
-static inline icmp_handle irp_get_icmp_handle( IRP *irp )
+struct icmp_data
 {
-    return PtrToUlong( irp->Tail.Overlay.DriverContext[0] );
+    icmp_handle handle;
+    HANDLE completion_event;
+    HANDLE wait_object;
+};
+
+static void free_icmp_data( struct icmp_data *data )
+{
+    UnregisterWait( data->wait_object );
+    CloseHandle( data->completion_event );
+    free( data );
+}
+
+static struct icmp_data *irp_get_icmp_data( IRP *irp )
+{
+    return irp->Tail.Overlay.DriverContext[0];
 }
 
-static inline icmp_handle irp_set_icmp_handle( IRP *irp, icmp_handle handle )
+static struct icmp_data *irp_set_icmp_data( IRP *irp, struct icmp_data *data )
 {
-    return PtrToUlong( InterlockedExchangePointer( irp->Tail.Overlay.DriverContext,
-                                                   ULongToPtr( handle ) ) );
+    return InterlockedExchangePointer( irp->Tail.Overlay.DriverContext, data );
 }
 
 DECLARE_CRITICAL_SECTION( icmp_echo_completion_cs );
 
 static void WINAPI icmp_echo_cancel( DEVICE_OBJECT *device, IRP *irp )
 {
-    struct icmp_cancel_listen_params params;
+    struct icmp_data *data;
 
     TRACE( "device %p, irp %p.\n", device, irp );
 
     IoReleaseCancelSpinLock( irp->CancelIrql );
     EnterCriticalSection( &icmp_echo_completion_cs );
-
-    /* If the handle is not set, either the irp is still
-       in the request queue, in which case the request thread will
-       cancel it, or the irp has already finished.  If the handle
-       does exist then notify the listen thread.  In all cases the irp
-       will be completed elsewhere. */
-    params.handle = irp_get_icmp_handle( irp );
-    if (params.handle) nsiproxy_call( icmp_cancel_listen, ¶ms );
+    if ((data = irp_get_icmp_data( irp ))) SetEvent( data->completion_event );
     LeaveCriticalSection( &icmp_echo_completion_cs );
 }
 
@@ -225,48 +230,56 @@ static int icmp_echo_reply_struct_len( ULONG family, ULONG bits )
     return 0;
 }
 
-static DWORD WINAPI icmp_wait_reply( void *arg )
+static void CALLBACK icmp_wait_complete( void *arg, BOOLEAN timed_out )
 {
     IRP *irp = arg;
     IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp );
     struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer;
-    struct icmp_close_params close_params;
     struct icmp_get_reply_params params;
+    struct icmp_data *data;
     NTSTATUS status;
 
-    TRACE( "\n" );
+    TRACE( "irp %p, timed_out %d.\n", irp, timed_out );
 
+    EnterCriticalSection( &icmp_echo_completion_cs );
+    data = irp_set_icmp_data( irp, NULL );
+    assert( data );
     params.user_reply_ptr = in->user_reply_ptr;
-    params.handle = irp_get_icmp_handle( irp );
-    params.timeout = in->timeout;
+    params.handle = data->handle;
     params.bits = in->bits;
     params.reply = irp->AssociatedIrp.SystemBuffer;
     params.reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength;
 
     status = nsiproxy_call( icmp_get_reply, ¶ms );
-    TRACE( "icmp_get_reply rets %08lx\n", status );
-
-    EnterCriticalSection( &icmp_echo_completion_cs );
-    close_params.handle = irp_set_icmp_handle( irp, 0 );
-    nsiproxy_call( icmp_close, &close_params );
 
-    irp->IoStatus.Status = status;
-    if (status == STATUS_SUCCESS)
-        irp->IoStatus.Information = params.reply_len;
+    irp->IoStatus.Information = 0;
+    if (irp->Cancel)
+    {
+        TRACE( "irp %p cancelled.\n", irp );
+        irp->IoStatus.Status = STATUS_CANCELLED;
+    }
     else
-        irp->IoStatus.Information = 0;
+    {
+        irp->IoStatus.Status = status;
+        if (status == STATUS_SUCCESS)
+            irp->IoStatus.Information = params.reply_len;
+    }
     LeaveCriticalSection( &icmp_echo_completion_cs );
     IoCompleteRequest( irp, IO_NO_INCREMENT );
-    return 0;
+    free_icmp_data( data );
 }
 
 static NTSTATUS handle_send_echo( IRP *irp )
 {
     struct nsiproxy_icmp_echo *in = (struct nsiproxy_icmp_echo *)irp->AssociatedIrp.SystemBuffer;
     struct icmp_send_echo_params params;
-    icmp_handle handle;
+    struct icmp_data *data;
     NTSTATUS status;
 
+    data = calloc( 1, sizeof(*data) );
+    data->completion_event = CreateEventW( NULL, TRUE, FALSE, NULL );
+    irp_set_icmp_data( irp, data );
+
     TRACE( "\n" );
     params.request = in->data + ((in->opt_size + 3) & ~3);
     params.request_size = in->req_size;
@@ -277,7 +290,8 @@ static NTSTATUS handle_send_echo( IRP *irp )
     params.tos = in->tos;
     params.src = &in->src;
     params.dst = &in->dst;
-    params.handle = &handle;
+    params.completion_event = data->completion_event;
+    params.handle = &data->handle;
 
     status = nsiproxy_call( icmp_send_echo, ¶ms );
     TRACE( "icmp_send_echo status %#lx\n", status );
@@ -287,17 +301,24 @@ static NTSTATUS handle_send_echo( IRP *irp )
         irp->IoStatus.Status = status;
         if (status == STATUS_SUCCESS)
             irp->IoStatus.Information = params.reply_len;
+        free_icmp_data( data );
         return status;
     }
     IoSetCancelRoutine( irp, icmp_echo_cancel );
     if (irp->Cancel && IoSetCancelRoutine( irp, NULL ))
     {
         /* IRP was canceled before we set cancel routine */
+        icmp_wait_complete( irp, FALSE );
         return STATUS_CANCELLED;
     }
     IoMarkIrpPending( irp );
-    irp_set_icmp_handle( irp, handle );
-    RtlQueueWorkItem( icmp_wait_reply, irp, WT_EXECUTELONGFUNCTION );
+    if (in->dst.si_family == AF_INET6 && !in->hop_limit)
+    {
+        /* This should fail at once but asynchronously. */
+        SetEvent( data->completion_event );
+    }
+    RegisterWaitForSingleObject( &data->wait_object, data->completion_event, icmp_wait_complete, irp, in->timeout,
+                                 WT_EXECUTEONLYONCE | WT_EXECUTEINWAITTHREAD );
     return STATUS_PENDING;
 }
 
diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 0de8ec5d2e9..7eb2675a16f 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -31,6 +31,7 @@
 #include <pthread.h>
 #include <poll.h>
 #include <sys/socket.h>
+#include <assert.h>
 
 #ifdef HAVE_NETINET_IN_H
 #include <netinet/in.h>
@@ -150,13 +151,23 @@ struct icmp_data
     struct sockaddr_storage dst_storage;
     int dst_len;
     struct icmp_socket *s;
+    struct icmp_reply_ctx reply_ctx;
+    HANDLE completion_event;
+    BOOL polling;
+    NTSTATUS status;
+    HANDLE listen_thread;
 };
 
+
+static pthread_mutex_t listen_lock = PTHREAD_MUTEX_INITIALIZER;
+
 #define MAX_HANDLES 256 /* Max number of simultaneous pings - could become dynamic if need be */
 static struct icmp_data *handle_table[MAX_HANDLES];
 static pthread_mutex_t handle_lock = PTHREAD_MUTEX_INITIALIZER;
 static struct icmp_data **next_free, **next_unused = handle_table;
 
+static void icmp_listen( void *args );
+
 static icmp_handle handle_alloc( struct icmp_data *data )
 {
     struct icmp_data **entry;
@@ -784,8 +795,24 @@ static void icmp_release_socket( struct icmp_socket *s )
     free( s );
 }
 
+static BOOL icmp_data_stop_polling( struct icmp_data *data, NTSTATUS status )
+{
+    BOOL ret;
+
+    pthread_mutex_lock( &listen_lock );
+    if ((ret = data->polling))
+    {
+        data->polling = FALSE;
+        data->status = status;
+        write( data->cancel_pipe[1], "x", 1 );
+    }
+    pthread_mutex_unlock( &listen_lock );
+    return ret;
+}
+
 static void icmp_data_free( struct icmp_data *data )
 {
+    assert( !data->polling );
     icmp_release_socket( data->s );
     close( data->cancel_pipe[0] );
     close( data->cancel_pipe[1] );
@@ -802,7 +829,7 @@ static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct i
     else if (params->dst->si_family == WS_AF_INET) ops = &ipv4;
     else return STATUS_INVALID_PARAMETER;
 
-    data = malloc( sizeof(*data) );
+    data = calloc( 1, sizeof(*data) );
     if (!data) return STATUS_NO_MEMORY;
     if (!(data->s = calloc( 1, sizeof(*data->s) )))
     {
@@ -851,8 +878,8 @@ static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct i
     }
 
     data->s->ops = ops;
-    data->s->ops->set_socket_opts( data->s );
-    *icmp_data = data;
+    data->s->ops->set_socket_opts( data->s );    *icmp_data = data;
+    data->completion_event = params->completion_event;
     return STATUS_SUCCESS;
 }
 
@@ -861,7 +888,7 @@ NTSTATUS icmp_send_echo( void *args )
     struct icmp_send_echo_params *params = args;
     struct icmp_hdr *icmp_hdr; /* this is the same for both ipv4 and ipv6 */
     struct icmp_data *data;
-    int ret;
+    int ret, err;
 
     NTSTATUS status;
 
@@ -878,35 +905,40 @@ NTSTATUS icmp_send_echo( void *args )
     memcpy( icmp_hdr + 1, params->request, params->request_size );
     icmp_hdr->checksum = data->s->ops->chksum( data, (BYTE *)icmp_hdr, sizeof(*icmp_hdr) + params->request_size );
 
+    pthread_mutex_lock( &listen_lock );
+    data->polling = TRUE;
+    pthread_mutex_unlock( &listen_lock );
+
+    if (PsCreateSystemThread( &data->listen_thread, THREAD_ALL_ACCESS, NULL, 0, NULL, icmp_listen, data ))
+        ERR( "Could not create listen thread.\n" );
+
     NtQueryPerformanceCounter( &data->send_time, NULL );
     ret = sendto( data->s->socket, icmp_hdr, sizeof(*icmp_hdr) + params->request_size, 0,
                   (struct sockaddr *)&data->dst_storage, data->dst_len );
+    if (ret < 0) err = errno;
     free( icmp_hdr );
 
     if (ret < 0)
     {
-        TRACE( "sendto() rets %d errno %d\n", ret, errno );
-        params->reply_len = data->s->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
+        status = errno_to_ip_status( err );
+        TRACE( "sendto() rets %d errno %d\n", ret, err );
+        icmp_data_stop_polling( data, status );
+        NtWaitForSingleObject( data->listen_thread, FALSE, NULL );
+        NtClose( data->listen_thread );
+        params->reply_len = data->s->ops->set_reply_ip_status( status, params->bits, params->reply );
         icmp_data_free( data );
         return STATUS_SUCCESS;
     }
 
     *params->handle = handle_alloc( data );
-    if (!*params->handle) icmp_data_free( data );
+    if (!*params->handle)
+    {
+        icmp_data_stop_polling( data, STATUS_NO_MEMORY );
+        icmp_data_free( data );
+    }
     return *params->handle ? STATUS_PENDING : STATUS_NO_MEMORY;
 }
 
-static int get_timeout( LARGE_INTEGER start, UINT timeout )
-{
-    LARGE_INTEGER now, end;
-
-    end.QuadPart = start.QuadPart + (ULONGLONG)timeout * 10000;
-    NtQueryPerformanceCounter( &now, NULL );
-    if (now.QuadPart >= end.QuadPart) return 0;
-
-    return min( (end.QuadPart - now.QuadPart) / 10000, INT_MAX );
-}
-
 static ULONG get_rtt( LARGE_INTEGER start )
 {
     LARGE_INTEGER now;
@@ -915,7 +947,7 @@ static ULONG get_rtt( LARGE_INTEGER start )
     return (now.QuadPart - start.QuadPart) / 10000;
 }
 
-static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_get_reply_params *params )
+static NTSTATUS recv_msg( struct icmp_data *data )
 {
     struct sockaddr_storage addr;
     struct icmp_reply_ctx ctx;
@@ -943,77 +975,87 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_get_reply_params *
     sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr );
     ctx.round_trip_time = get_rtt( data->send_time );
     ctx.data_offset = (BYTE *)(icmp_hdr + 1) - ctx.packet;
-
-    if (!data->s->ops->fill_reply( params, &ctx ))
-        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
-
+    data->reply_ctx = ctx;
     return STATUS_SUCCESS;
 }
 
-NTSTATUS icmp_get_reply( void *args )
+static void icmp_listen( void *args )
 {
-    struct icmp_get_reply_params *params = args;
-    struct icmp_data *data;
+    struct icmp_data *data = args;
     struct pollfd fds[2];
     NTSTATUS status;
     int ret;
 
-    data = handle_data( params->handle );
-    if (!data) return STATUS_INVALID_PARAMETER;
-
-    if (data->dst_storage.ss_family == AF_INET6 && !data->s->hop_limit)
-    {
-        TRACE( "Invalid hop_limit.\n" );
-        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
-        return STATUS_SUCCESS;
-    }
-
     fds[0].fd = data->s->socket;
     fds[0].events = POLLIN;
     fds[1].fd = data->cancel_pipe[0];
     fds[1].events = POLLIN;
 
-    while ((ret = poll( fds, ARRAY_SIZE(fds), get_timeout( data->send_time, params->timeout ) )) > 0)
+    while ((ret = poll( fds, ARRAY_SIZE(fds), -1 )) > 0)
     {
         if (fds[1].revents & POLLIN)
         {
             TRACE( "cancelled\n" );
-            return STATUS_CANCELLED;
+            icmp_data_stop_polling( data, STATUS_CANCELLED );
+            return;
         }
 
-        status = recv_msg( data, params );
+        pthread_mutex_lock( &listen_lock );
+        if (data->polling)
+        {
+            status = recv_msg( data );
+            pthread_mutex_unlock( &listen_lock );
+            if (status != STATUS_RETRY)
+            {
+                data->status = status;
+                data->polling = FALSE;
+                NtSetEvent( data->completion_event, NULL );
+            }
+        }
+        else status = STATUS_CANCELLED;
+        pthread_mutex_unlock( &listen_lock );
         if (status == STATUS_RETRY) continue;
-        return status;
+        return;
     }
-
-    if (!ret) /* timeout */
-    {
-        TRACE( "timeout\n" );
-        params->reply_len = data->s->ops->set_reply_ip_status( IP_REQ_TIMED_OUT, params->bits, params->reply );
-        return STATUS_SUCCESS;
-    }
-    /* ret < 0 */
-    params->reply_len = data->s->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply );
-    return STATUS_SUCCESS;
 }
 
-NTSTATUS icmp_cancel_listen( void *args )
+NTSTATUS icmp_get_reply( void *args )
 {
-    struct icmp_cancel_listen_params *params = args;
-    struct icmp_data *data = handle_data( params->handle );
+    struct icmp_get_reply_params *params = args;
+    struct icmp_data *data;
+    NTSTATUS ret = STATUS_SUCCESS;
 
+    data = handle_data( params->handle );
     if (!data) return STATUS_INVALID_PARAMETER;
-    write( data->cancel_pipe[1], "x", 1 );
-    return STATUS_SUCCESS;
-}
 
-NTSTATUS icmp_close( void *args )
-{
-    struct icmp_close_params *params = args;
-    struct icmp_data *data = handle_data( params->handle );
+    if (data->dst_storage.ss_family == AF_INET6 && !data->s->hop_limit)
+    {
+        TRACE( "data %p, seq %u, invalid hop_limit.\n", data, data->seq );
+        icmp_data_stop_polling( data, STATUS_SUCCESS );
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
+        goto done;
+    }
+
+    if (icmp_data_stop_polling( data, STATUS_TIMEOUT ))
+    {
+        /* Was still polling, timeout. */
+        TRACE( "data %p, seq %u, timeout.\n", data, data->seq );
+        params->reply_len = data->s->ops->set_reply_ip_status( IP_REQ_TIMED_OUT, params->bits, params->reply );
+    }
+    else if (!(ret = data->status))
+    {
+        if (!data->s->ops->fill_reply( params, &data->reply_ctx ))
+        {
+            WARN( "data %p, seq %u, buffer too small.\n", data, data->seq );
+            params->reply_len = data->s->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply );
+        }
+        else TRACE( "data %p, seq %u, got reply %u.\n", data, data->seq, data->reply_ctx.status );
+    }
+done:
+    NtWaitForSingleObject( data->listen_thread, FALSE, NULL );
+    NtClose( data->listen_thread );
 
-    if (!data) return STATUS_INVALID_PARAMETER;
     icmp_data_free( data );
     handle_free( params->handle );
-    return STATUS_SUCCESS;
+    return ret;
 }
diff --git a/dlls/nsiproxy.sys/nsi.c b/dlls/nsiproxy.sys/nsi.c
index 41aace2c2c4..09ced8f814e 100644
--- a/dlls/nsiproxy.sys/nsi.c
+++ b/dlls/nsiproxy.sys/nsi.c
@@ -343,8 +343,6 @@ static NTSTATUS unix_nsi_get_notification( void *args )
 
 const unixlib_entry_t __wine_unix_call_funcs[] =
 {
-    icmp_cancel_listen,
-    icmp_close,
     icmp_get_reply,
     icmp_send_echo,
     unix_nsi_enumerate_all_ex,
diff --git a/dlls/nsiproxy.sys/nsiproxy_private.h b/dlls/nsiproxy.sys/nsiproxy_private.h
index a15d594bbdd..cd1a7508087 100644
--- a/dlls/nsiproxy.sys/nsiproxy_private.h
+++ b/dlls/nsiproxy.sys/nsiproxy_private.h
@@ -20,23 +20,12 @@
 
 typedef UINT icmp_handle;
 
-struct icmp_cancel_listen_params
-{
-    icmp_handle handle;
-};
-
-struct icmp_close_params
-{
-    icmp_handle handle;
-};
-
 struct icmp_get_reply_params
 {
     icmp_handle handle;
     void *reply;
     ULONGLONG user_reply_ptr;
     unsigned int bits, reply_len;
-    int timeout;
 };
 
 struct icmp_send_echo_params
@@ -47,6 +36,7 @@ struct icmp_send_echo_params
     UINT request_size, reply_len;
     BYTE bits, ttl, tos;
     int hop_limit;
+    HANDLE completion_event;
     icmp_handle *handle;
 };
 
diff --git a/dlls/nsiproxy.sys/unix_private.h b/dlls/nsiproxy.sys/unix_private.h
index 8a332f2ba07..170bdba665d 100644
--- a/dlls/nsiproxy.sys/unix_private.h
+++ b/dlls/nsiproxy.sys/unix_private.h
@@ -177,7 +177,5 @@ static inline int ascii_strcasecmp( const char *s1, const char *s2 )
     return ascii_strncasecmp( s1, s2, -1 );
 }
 
-NTSTATUS icmp_cancel_listen( void *args );
-NTSTATUS icmp_close( void *args );
 NTSTATUS icmp_get_reply( void *args );
 NTSTATUS icmp_send_echo( void *args );
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 5/9] nsiproxy.sys: Poll all icmp sockets in a single thread.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 200 +++++++++++++++++++++-------------
 1 file changed, 126 insertions(+), 74 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 7eb2675a16f..d0f6332f2b3 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -53,6 +53,7 @@
 #include "ipexport.h"
 #include "ipmib.h"
 #include "wine/nsi.h"
+#include "wine/list.h"
 #include "wine/debug.h"
 
 #include "nsiproxy_private.h"
@@ -132,6 +133,9 @@ struct family_ops;
 
 struct icmp_socket
 {
+    struct list entry;
+    struct list listen_entry;
+    struct list request_list;
     LONG ref;
     int socket;
     const struct family_ops *ops;
@@ -141,12 +145,13 @@ struct icmp_socket
     BYTE ttl, tos;
     BOOL ping_socket;
     unsigned short id;
+    BOOL polling;
 };
 
 struct icmp_data
 {
+    struct list entry;
     LARGE_INTEGER send_time;
-    int cancel_pipe[2];
     unsigned short seq;
     struct sockaddr_storage dst_storage;
     int dst_len;
@@ -155,11 +160,13 @@ struct icmp_data
     HANDLE completion_event;
     BOOL polling;
     NTSTATUS status;
-    HANDLE listen_thread;
 };
 
 
 static pthread_mutex_t listen_lock = PTHREAD_MUTEX_INITIALIZER;
+struct list socket_list = LIST_INIT( socket_list );
+static int socket_list_update_pipe[2];
+static pthread_once_t init_once = PTHREAD_ONCE_INIT;
 
 #define MAX_HANDLES 256 /* Max number of simultaneous pings - could become dynamic if need be */
 static struct icmp_data *handle_table[MAX_HANDLES];
@@ -788,11 +795,23 @@ static BOOL sockaddr_to_SOCKADDR_INET( const struct sockaddr *in, SOCKADDR_INET
     return FALSE;
 }
 
+static void icmp_grab_socket( struct icmp_socket *s )
+{
+    assert( s->ref > 0 );
+    InterlockedIncrement( &s->ref );
+}
+
 static void icmp_release_socket( struct icmp_socket *s )
 {
-    if (InterlockedDecrement( &s->ref)) return;
-    if (s->socket >= 0) close( s->socket );
-    free( s );
+    pthread_mutex_lock( &listen_lock );
+    if (!InterlockedDecrement( &s->ref ))
+    {
+        TRACE( "freeing socket %p, fd %d.\n", s, s->socket );
+        if (s->polling) list_remove( &s->entry );
+        if (s->socket >= 0) close( s->socket );
+        free( s );
+    }
+    pthread_mutex_unlock( &listen_lock );
 }
 
 static BOOL icmp_data_stop_polling( struct icmp_data *data, NTSTATUS status )
@@ -802,9 +821,9 @@ static BOOL icmp_data_stop_polling( struct icmp_data *data, NTSTATUS status )
     pthread_mutex_lock( &listen_lock );
     if ((ret = data->polling))
     {
+        list_remove( &data->entry );
         data->polling = FALSE;
         data->status = status;
-        write( data->cancel_pipe[1], "x", 1 );
     }
     pthread_mutex_unlock( &listen_lock );
     return ret;
@@ -814,17 +833,29 @@ static void icmp_data_free( struct icmp_data *data )
 {
     assert( !data->polling );
     icmp_release_socket( data->s );
-    close( data->cancel_pipe[0] );
-    close( data->cancel_pipe[1] );
     free( data );
 }
 
+static void init_listen(void)
+{
+    HANDLE thread;
+
+    pipe( socket_list_update_pipe );
+
+    if (PsCreateSystemThread( &thread, THREAD_ALL_ACCESS, NULL, 0, NULL, icmp_listen, NULL ))
+        ERR( "Could not create listen thread.\n" );
+    else
+        NtClose( thread );
+}
+
 static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct icmp_data **icmp_data )
 {
     struct sockaddr *src, *dst;
     struct icmp_data *data;
     const struct family_ops *ops;
 
+    pthread_once( &init_once, init_listen );
+
     if (params->dst->si_family == WS_AF_INET6)     ops = &ipv6;
     else if (params->dst->si_family == WS_AF_INET) ops = &ipv4;
     else return STATUS_INVALID_PARAMETER;
@@ -837,6 +868,7 @@ static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct i
         return STATUS_NO_MEMORY;
     }
     data->s->ref = 1;
+    list_init( &data->s->request_list );
     data->s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
     data->s->ping_socket = FALSE;
     data->s->socket = socket( ops->family, SOCK_RAW, ops->icmp_protocol );
@@ -856,12 +888,6 @@ static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct i
         data->s->ping_socket = TRUE;
 #endif
     }
-    if (pipe( data->cancel_pipe ))
-    {
-        icmp_release_socket( data->s );
-        free( data );
-        return STATUS_ACCESS_DENIED;
-    }
 
     data->s->hop_limit = params->hop_limit;
     data->s->ttl = params->ttl;
@@ -878,7 +904,14 @@ static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct i
     }
 
     data->s->ops = ops;
-    data->s->ops->set_socket_opts( data->s );    *icmp_data = data;
+    data->s->ops->set_socket_opts( data->s );
+    pthread_mutex_lock( &listen_lock );
+    list_add_tail( &socket_list, &data->s->entry );
+    data->s->polling = TRUE;
+    write( socket_list_update_pipe[1], "x", 1 );
+    pthread_mutex_unlock( &listen_lock );
+
+    *icmp_data = data;
     data->completion_event = params->completion_event;
     return STATUS_SUCCESS;
 }
@@ -906,12 +939,10 @@ NTSTATUS icmp_send_echo( void *args )
     icmp_hdr->checksum = data->s->ops->chksum( data, (BYTE *)icmp_hdr, sizeof(*icmp_hdr) + params->request_size );
 
     pthread_mutex_lock( &listen_lock );
+    list_add_tail( &data->s->request_list, &data->entry );
     data->polling = TRUE;
     pthread_mutex_unlock( &listen_lock );
 
-    if (PsCreateSystemThread( &data->listen_thread, THREAD_ALL_ACCESS, NULL, 0, NULL, icmp_listen, data ))
-        ERR( "Could not create listen thread.\n" );
-
     NtQueryPerformanceCounter( &data->send_time, NULL );
     ret = sendto( data->s->socket, icmp_hdr, sizeof(*icmp_hdr) + params->request_size, 0,
                   (struct sockaddr *)&data->dst_storage, data->dst_len );
@@ -923,8 +954,6 @@ NTSTATUS icmp_send_echo( void *args )
         status = errno_to_ip_status( err );
         TRACE( "sendto() rets %d errno %d\n", ret, err );
         icmp_data_stop_polling( data, status );
-        NtWaitForSingleObject( data->listen_thread, FALSE, NULL );
-        NtClose( data->listen_thread );
         params->reply_len = data->s->ops->set_reply_ip_status( status, params->bits, params->reply );
         icmp_data_free( data );
         return STATUS_SUCCESS;
@@ -947,75 +976,101 @@ static ULONG get_rtt( LARGE_INTEGER start )
     return (now.QuadPart - start.QuadPart) / 10000;
 }
 
-static NTSTATUS recv_msg( struct icmp_data *data )
+static void icmp_listen( void *args )
 {
     struct sockaddr_storage addr;
     struct icmp_reply_ctx ctx;
-    struct iovec iov[1];
     BYTE cmsg_buf[1024];
-    struct msghdr msg = { .msg_name = &addr, .msg_namelen = sizeof(addr),
-                          .msg_iov = iov, .msg_iovlen = ARRAY_SIZE(iov),
-                          .msg_control = cmsg_buf, .msg_controllen = sizeof(cmsg_buf) };
-    int ip_hdr_len, recvd;
+    struct iovec iov[1] = {{ .iov_base = ctx.packet, .iov_len = sizeof(ctx.packet) }};
+    struct msghdr msg = { .msg_iov = iov, .msg_iovlen = ARRAY_SIZE(iov) };
+    struct list listen_list;
+    unsigned int fds_size = 64;
+    struct pollfd *fds = malloc( fds_size * sizeof(*fds) );
+    struct icmp_socket *s, *next;
     struct icmp_hdr *icmp_hdr;
-
-    iov[0].iov_base = ctx.packet;
-    iov[0].iov_len = sizeof(ctx.packet);
-
-    recvd = recvmsg( data->s->socket, &msg, 0 );
-    TRACE( "recvmsg() rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
-           recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
-
-    if (recvd < 0) return STATUS_RETRY;
-    if (!data->s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) return STATUS_RETRY;
-    if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) return STATUS_RETRY;
-    ctx.packet_size = recvd;
-    icmp_hdr = (struct icmp_hdr *)(ctx.packet + ip_hdr_len);
-    if ((ctx.data_size = data->s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0) return STATUS_RETRY;
-    sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr );
-    ctx.round_trip_time = get_rtt( data->send_time );
-    ctx.data_offset = (BYTE *)(icmp_hdr + 1) - ctx.packet;
-    data->reply_ctx = ctx;
-    return STATUS_SUCCESS;
-}
-
-static void icmp_listen( void *args )
-{
-    struct icmp_data *data = args;
-    struct pollfd fds[2];
-    NTSTATUS status;
+    int ip_hdr_len, recvd;
+    struct icmp_data *data;
+    unsigned int i, count;
     int ret;
+    BYTE b;
 
-    fds[0].fd = data->s->socket;
-    fds[0].events = POLLIN;
-    fds[1].fd = data->cancel_pipe[0];
-    fds[1].events = POLLIN;
-
-    while ((ret = poll( fds, ARRAY_SIZE(fds), -1 )) > 0)
+    pthread_once( &init_once, init_listen );
+    while (1)
     {
-        if (fds[1].revents & POLLIN)
+        list_init( &listen_list );
+
+        pthread_mutex_lock( &listen_lock );
+        i = 0;
+        LIST_FOR_EACH_ENTRY( s, &socket_list, struct icmp_socket, entry )
         {
-            TRACE( "cancelled\n" );
-            icmp_data_stop_polling( data, STATUS_CANCELLED );
+            icmp_grab_socket( s );
+            list_add_tail( &listen_list, &s->listen_entry );
+            if (i + 1 >= fds_size)
+            {
+                fds_size *= 2;
+                fds = realloc( fds, fds_size * sizeof(*fds) );
+            }
+            fds[i].fd = s->socket;
+            fds[i].events = POLLIN;
+            ++i;
+        }
+        pthread_mutex_unlock( &listen_lock );
+
+        fds[i].fd = socket_list_update_pipe[0];
+        fds[i].events = POLLIN;
+        count = i;
+        while ((ret = poll( fds, count + 1, -1 )) < 0 && (errno == EINTR || errno == EAGAIN))
+            ;
+        if (ret < 0)
+        {
+            ERR( "poll(): ret %d, %s.\n", ret, strerror( errno ));
             return;
         }
 
-        pthread_mutex_lock( &listen_lock );
-        if (data->polling)
+        i = 0;
+        LIST_FOR_EACH_ENTRY( s, &listen_list, struct icmp_socket, listen_entry )
         {
-            status = recv_msg( data );
-            pthread_mutex_unlock( &listen_lock );
-            if (status != STATUS_RETRY)
+            if (!(fds[i].revents & POLLIN)) goto skip;
+            msg.msg_name = &addr;
+            msg.msg_namelen = sizeof(addr);
+            msg.msg_control = cmsg_buf;
+            msg.msg_controllen = sizeof(cmsg_buf);
+            recvd = recvmsg( s->socket, &msg, 0 );
+            TRACE( "s %p, recvmsg() rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
+                   s, recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
+            if (recvd < 0) goto skip;
+            if (!s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) goto skip;
+            if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) goto skip;
+            ctx.packet_size = recvd;
+            icmp_hdr = (struct icmp_hdr *)(ctx.packet + ip_hdr_len);
+            pthread_mutex_lock( &listen_lock );
+            LIST_FOR_EACH_ENTRY( data, &s->request_list, struct icmp_data, entry )
             {
-                data->status = status;
+                if ((ctx.data_size = s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0)
+                    continue;
                 data->polling = FALSE;
+                list_remove( &data->entry );
+                sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr );
+                ctx.round_trip_time = get_rtt( data->send_time );
+                ctx.data_offset = (BYTE *)(icmp_hdr + 1) - ctx.packet;
+                data->reply_ctx = ctx;
                 NtSetEvent( data->completion_event, NULL );
+                break;
             }
+            pthread_mutex_unlock( &listen_lock );
+skip:
+            ++i;
+        }
+        if (fds[count].revents & POLLIN)
+        {
+            read( socket_list_update_pipe[0], &b, 1 );
+            TRACE( "updating socket list.\n" );
+        }
+
+        LIST_FOR_EACH_ENTRY_SAFE( s, next, &listen_list, struct icmp_socket, listen_entry )
+        {
+            icmp_release_socket( s );
         }
-        else status = STATUS_CANCELLED;
-        pthread_mutex_unlock( &listen_lock );
-        if (status == STATUS_RETRY) continue;
-        return;
     }
 }
 
@@ -1052,9 +1107,6 @@ NTSTATUS icmp_get_reply( void *args )
         else TRACE( "data %p, seq %u, got reply %u.\n", data, data->seq, data->reply_ctx.status );
     }
 done:
-    NtWaitForSingleObject( data->listen_thread, FALSE, NULL );
-    NtClose( data->listen_thread );
-
     icmp_data_free( data );
     handle_free( params->handle );
     return ret;
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 6/9] nsiproxy.sys: Return ICMP ids from parse_icmp_hdr() instead of matching those.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 62 +++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 24 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index d0f6332f2b3..98bed473b1e 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -127,6 +127,7 @@ struct icmp_reply_ctx
     unsigned int data_offset;
     BYTE packet[65536];
     unsigned int packet_size;
+    uint16_t id, sequence;
 };
 
 struct family_ops;
@@ -358,7 +359,7 @@ static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip
 }
 #endif
 
-static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_size,
+static int ipv4_parse_icmp_hdr( struct icmp_socket *s, struct icmp_hdr *icmp, int icmp_size,
                                 struct icmp_reply_ctx *ctx )
 {
     static const IP_STATUS unreach_codes[] =
@@ -388,9 +389,8 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     switch (icmp->type)
     {
     case ICMP4_ECHO_REPLY:
-        if ((!data->s->ping_socket && icmp->un.echo.id != data->s->id) ||
-            icmp->un.echo.sequence != data->seq) return -1;
-
+        ctx->id = icmp->un.echo.id;
+        ctx->sequence = icmp->un.echo.sequence;
         ctx->status = IP_SUCCESS;
         return icmp_size - sizeof(*icmp);
 
@@ -420,11 +420,13 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
         return -1;
     }
 
-    if (data->s->ping_socket) return 0;
+    if (s->ping_socket)
+    {
+        /* Ping sockets only return echo reply through normal recvmsg() received data, should not get here. */
+        return -1;
+    }
 
-    /* Check that the appended packet is really ours -
-     * all handled icmp replies have an 8-byte header
-     * followed by the original ip hdr. */
+    /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
     if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr)) return -1;
     orig_ip_hdr = (struct ip_hdr *)(icmp + 1);
     if (orig_ip_hdr->v_hl >> 4 != 4 || orig_ip_hdr->protocol != IPPROTO_ICMP) return -1;
@@ -432,10 +434,10 @@ static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, i
     if (icmp_size < sizeof(*icmp) + orig_ip_hdr_len + sizeof(*orig_icmp_hdr)) return -1;
     orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + orig_ip_hdr_len);
     if (orig_icmp_hdr->type != ICMP4_ECHO_REQUEST ||
-        orig_icmp_hdr->code != 0 ||
-        (!data->s->ping_socket && orig_icmp_hdr->un.echo.id != data->s->id) ||
-        orig_icmp_hdr->un.echo.sequence != data->seq) return -1;
+        orig_icmp_hdr->code != 0) return -1;
 
+    ctx->id = orig_icmp_hdr->un.echo.id;
+    ctx->sequence = orig_icmp_hdr->un.echo.sequence;
     ctx->status = status;
     return 0;
 }
@@ -501,7 +503,7 @@ struct family_ops
     int (*set_reply_ip_status)( IP_STATUS ip_status, unsigned int bits, void *out );
     void (*set_socket_opts)( struct icmp_socket *s );
     BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
-    int (*parse_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
+    int (*parse_icmp_hdr)( struct icmp_socket *s, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
     BOOL (*fill_reply)( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx );
 };
 
@@ -626,7 +628,7 @@ static BOOL ipv6_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
     return TRUE;
 }
 
-static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
+static int ipv6_parse_icmp_hdr( struct icmp_socket *s, struct icmp_hdr *icmp,
                                 int icmp_size, struct icmp_reply_ctx *ctx )
 {
     static const IP_STATUS unreach_codes[] =
@@ -647,9 +649,8 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
     switch (icmp->type)
     {
     case ICMP6_ECHO_REPLY:
-        if ((!data->s->ping_socket && icmp->un.echo.id != data->s->id) ||
-            icmp->un.echo.sequence != data->seq) return -1;
-
+        ctx->id = icmp->un.echo.id;
+        ctx->sequence = icmp->un.echo.sequence;
         ctx->status = IP_SUCCESS;
         return icmp_size - sizeof(*icmp);
 
@@ -681,20 +682,24 @@ static int ipv6_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp,
         return -1;
     }
 
-    if (data->s->ping_socket) return 0;
-    /* Check that the appended packet is really ours. */
+    if (s->ping_socket)
+    {
+        /* Ping sockets only return echo reply through normal recvmsg() received data, should not get here. */
+        return -1;
+    }
+
+    /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
     if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr)) return -1;
     orig_ip_hdr = (struct ipv6_hdr *)(icmp + 1);
     if ((orig_ip_hdr->v_prio >> 4) != 6 || orig_ip_hdr->next_hdr != IPPROTO_ICMPV6) return -1;
     if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr) + sizeof(*orig_icmp_hdr)) return -1;
     orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + sizeof(*orig_ip_hdr));
     if (orig_icmp_hdr->type != ICMP6_ECHO_REQUEST ||
-        orig_icmp_hdr->code != 0 ||
-        (!data->s->ping_socket && orig_icmp_hdr->un.echo.id != data->s->id) ||
-        orig_icmp_hdr->un.echo.sequence != data->seq) return -1;
+        orig_icmp_hdr->code != 0) return -1;
 
+    ctx->id = orig_icmp_hdr->un.echo.id;
+    ctx->sequence = orig_icmp_hdr->un.echo.sequence;
     ctx->status = status;
-
     return 0;
 }
 
@@ -991,6 +996,7 @@ static void icmp_listen( void *args )
     int ip_hdr_len, recvd;
     struct icmp_data *data;
     unsigned int i, count;
+    BOOL matched;
     int ret;
     BYTE b;
 
@@ -1043,11 +1049,18 @@ static void icmp_listen( void *args )
             if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) goto skip;
             ctx.packet_size = recvd;
             icmp_hdr = (struct icmp_hdr *)(ctx.packet + ip_hdr_len);
+            if ((ctx.data_size = s->ops->parse_icmp_hdr( s, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0)
+            {
+                WARN( "parse_icmp_hdr failed.\n" );
+                goto skip;
+            }
             pthread_mutex_lock( &listen_lock );
+            matched = FALSE;
             LIST_FOR_EACH_ENTRY( data, &s->request_list, struct icmp_data, entry )
             {
-                if ((ctx.data_size = s->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, &ctx )) < 0)
-                    continue;
+                if ((!s->ping_socket && ctx.id != s->id) || ctx.sequence != data->seq) continue;
+                TRACE( "id %d, seq %d matched to %p.\n", ctx.id, ctx.sequence, data );
+                matched = TRUE;
                 data->polling = FALSE;
                 list_remove( &data->entry );
                 sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr );
@@ -1058,6 +1071,7 @@ static void icmp_listen( void *args )
                 break;
             }
             pthread_mutex_unlock( &listen_lock );
+            if (!matched) TRACE( "id %d, seq %d not matched.\n", ctx.id, ctx.sequence );
 skip:
             ++i;
         }
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 7/9] nsiproxy.sys: Reuse icmp socket with matching parameters.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 115 ++++++++++++++++++++++------------
 1 file changed, 76 insertions(+), 39 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 98bed473b1e..1733fab2a02 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -751,6 +751,7 @@ static int SOCKADDR_INET_to_sockaddr( const SOCKADDR_INET *in, struct sockaddr *
         struct sockaddr_in *sa = (struct sockaddr_in *)out;
 
         if (len < sizeof(*sa)) return 0;
+        memset( sa, 0, sizeof(*sa) );
         sa->sin_family = AF_INET;
         sa->sin_port = in->Ipv4.sin_port;
         sa->sin_addr.s_addr = in->Ipv4.sin_addr.WS_s_addr;
@@ -761,6 +762,7 @@ static int SOCKADDR_INET_to_sockaddr( const SOCKADDR_INET *in, struct sockaddr *
         struct sockaddr_in6 *sa = (struct sockaddr_in6 *)out;
 
         if (len < sizeof(*sa)) return 0;
+        memset( sa, 0, sizeof(*sa) );
         sa->sin6_family = AF_INET6;
         sa->sin6_port = in->Ipv6.sin6_port;
         sa->sin6_flowinfo = in->Ipv6.sin6_flowinfo;
@@ -853,71 +855,94 @@ static void init_listen(void)
         NtClose( thread );
 }
 
-static NTSTATUS icmp_data_create( struct icmp_send_echo_params *params, struct icmp_data **icmp_data )
+static NTSTATUS icmp_get_socket( struct icmp_send_echo_params *params, struct icmp_socket **ret_socket )
 {
-    struct sockaddr *src, *dst;
-    struct icmp_data *data;
     const struct family_ops *ops;
+    struct icmp_socket *s;
+    struct sockaddr *src;
+    int src_len;
+    struct sockaddr_storage src_storage;
 
-    pthread_once( &init_once, init_listen );
+    src = (struct sockaddr *)&src_storage;
+    src_len = SOCKADDR_INET_to_sockaddr( params->src, src, sizeof(src_storage) );
 
     if (params->dst->si_family == WS_AF_INET6)     ops = &ipv6;
     else if (params->dst->si_family == WS_AF_INET) ops = &ipv4;
     else return STATUS_INVALID_PARAMETER;
 
-    data = calloc( 1, sizeof(*data) );
-    if (!data) return STATUS_NO_MEMORY;
-    if (!(data->s = calloc( 1, sizeof(*data->s) )))
+    *ret_socket = NULL;
+    pthread_mutex_lock( &listen_lock );
+    LIST_FOR_EACH_ENTRY( s, &socket_list, struct icmp_socket, entry )
     {
-        free( data );
+        assert( s->polling );
+        if (params->dst->si_family == WS_AF_INET6 && ops->family != AF_INET6) continue;
+        if (params->dst->si_family == WS_AF_INET && ops->family != AF_INET) continue;
+        if (s->hop_limit != params->hop_limit || s->ttl != params->ttl || s->tos != params->tos) continue;
+        if (s->src_len != src_len) continue;
+        if (memcmp( src, &s->src_storage, src_len )) continue;
+
+        icmp_grab_socket( s );
+        pthread_mutex_unlock( &listen_lock );
+        *ret_socket = s;
+        TRACE( "Using socket %p.\n", s );
+        return STATUS_SUCCESS;
+    }
+
+    pthread_once( &init_once, init_listen );
+
+    if (!(s = calloc( 1, sizeof(*s) )))
+    {
+        pthread_mutex_unlock( &listen_lock );
         return STATUS_NO_MEMORY;
     }
-    data->s->ref = 1;
-    list_init( &data->s->request_list );
-    data->s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
-    data->s->ping_socket = FALSE;
-    data->s->socket = socket( ops->family, SOCK_RAW, ops->icmp_protocol );
-    if (data->s->socket < 0) /* Try a ping-socket */
+    s->ref = 1;
+    list_init( &s->request_list );
+    s->socket = -1;
+    s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
+    s->ping_socket = FALSE;
+
+    s->socket = socket( ops->family, SOCK_RAW, ops->icmp_protocol );
+    if (s->socket < 0) /* Try a ping-socket */
     {
+        int value = 1;
+
         TRACE( "failed to open raw sock, trying a dgram sock\n" );
-        data->s->socket = socket( ops->family, SOCK_DGRAM, ops->icmp_protocol );
-        if (data->s->socket < 0)
+        s->socket = socket( ops->family, SOCK_DGRAM, ops->icmp_protocol );
+        if (s->socket < 0)
         {
             WARN( "Unable to create socket\n" );
-            icmp_release_socket( data->s );
-            free( data );
+            pthread_mutex_unlock( &listen_lock );
+            icmp_release_socket( s );
             return STATUS_ACCESS_DENIED;
         }
+        setsockopt( s->socket, SOL_SOCKET, SO_REUSEADDR, &value, sizeof(value) );
 #ifdef __linux__
         if (ops->family == AF_INET) ops = &ipv4_linux_ping;
-        data->s->ping_socket = TRUE;
+        s->ping_socket = TRUE;
 #endif
     }
 
-    data->s->hop_limit = params->hop_limit;
-    data->s->ttl = params->ttl;
-    data->s->tos = params->tos;
-    src = (struct sockaddr *)&data->s->src_storage;
-    dst = (struct sockaddr *)&data->dst_storage;
-    data->s->src_len = SOCKADDR_INET_to_sockaddr( params->src, src, sizeof(data->s->src_storage) );
-    data->dst_len = SOCKADDR_INET_to_sockaddr( params->dst, dst, sizeof(data->dst_storage) );
+    s->hop_limit = params->hop_limit;
+    s->ttl = params->ttl;
+    s->tos = params->tos;
+    s->src_len = src_len;
+    s->src_storage = src_storage;
 
-    if (bind( data->s->socket, src, data->s->src_len ))
+    if (bind( s->socket, src, s->src_len ))
     {
-        icmp_data_free( data );
+        pthread_mutex_unlock( &listen_lock );
+        icmp_release_socket( s );
         return STATUS_INVALID_ADDRESS_COMPONENT;
     }
 
-    data->s->ops = ops;
-    data->s->ops->set_socket_opts( data->s );
-    pthread_mutex_lock( &listen_lock );
-    list_add_tail( &socket_list, &data->s->entry );
-    data->s->polling = TRUE;
-    write( socket_list_update_pipe[1], "x", 1 );
+    s->ops = ops;
+    s->ops->set_socket_opts( s );
+    list_add_tail( &socket_list, &s->entry );
+    s->polling = TRUE;
     pthread_mutex_unlock( &listen_lock );
-
-    *icmp_data = data;
-    data->completion_event = params->completion_event;
+    write( socket_list_update_pipe[1], "x", 1 );
+    *ret_socket = s;
+    TRACE( "created socket %p.\n", s );
     return STATUS_SUCCESS;
 }
 
@@ -926,12 +951,22 @@ NTSTATUS icmp_send_echo( void *args )
     struct icmp_send_echo_params *params = args;
     struct icmp_hdr *icmp_hdr; /* this is the same for both ipv4 and ipv6 */
     struct icmp_data *data;
+    struct sockaddr *dst;
     int ret, err;
 
     NTSTATUS status;
 
-    status = icmp_data_create( params, &data );
-    if (status) return status;
+    data = calloc( 1, sizeof(*data) );
+    if (!data) return STATUS_NO_MEMORY;
+    if ((status = icmp_get_socket( params, &data->s )))
+    {
+        free( data );
+        return status;
+    }
+    data->completion_event = params->completion_event;
+
+    dst = (struct sockaddr *)&data->dst_storage;
+    data->dst_len = SOCKADDR_INET_to_sockaddr( params->dst, dst, sizeof(data->dst_storage) );
 
     icmp_hdr = malloc( sizeof(*icmp_hdr) + params->request_size );
     if (!icmp_hdr)
@@ -943,6 +978,8 @@ NTSTATUS icmp_send_echo( void *args )
     memcpy( icmp_hdr + 1, params->request, params->request_size );
     icmp_hdr->checksum = data->s->ops->chksum( data, (BYTE *)icmp_hdr, sizeof(*icmp_hdr) + params->request_size );
 
+    TRACE( "data %p, seq %d.\n", data, data->seq );
+
     pthread_mutex_lock( &listen_lock );
     list_add_tail( &data->s->request_list, &data->entry );
     data->polling = TRUE;
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 8/9] nsiproxy.sys: Support ICMP error replies with ping sockets.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 configure.ac                  |   1 +
 dlls/nsiproxy.sys/icmp_echo.c | 130 +++++++++++++++++++++++++++-------
 2 files changed, 104 insertions(+), 27 deletions(-)

diff --git a/configure.ac b/configure.ac
index 74f6e988df8..95e1ac5d1b7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -736,6 +736,7 @@ AC_CHECK_HEADERS(\
 	gettext-po.h \
 	link.h \
 	linux/cdrom.h \
+	linux/errqueue.h \
 	linux/filter.h \
 	linux/fs.h \
 	linux/hdreg.h \
diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 1733fab2a02..0d2033b5237 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -41,6 +41,11 @@
 #include <netinet/ip.h>
 #endif
 
+#ifdef HAVE_LINUX_ERRQUEUE_H
+#include <linux/errqueue.h>
+#define USE_ERRQUEUE
+#endif
+
 #include "ntstatus.h"
 #include "windef.h"
 #include "winbase.h"
@@ -307,15 +312,16 @@ static void ipv4_linux_ping_set_socket_opts( struct icmp_socket *s )
 
     setsockopt( s->socket, IPPROTO_IP, IP_RECVTTL, &val, sizeof(val) );
     setsockopt( s->socket, IPPROTO_IP, IP_RECVTOS, &val, sizeof(val) );
+    setsockopt( s->socket, IPPROTO_IP, IP_RECVERR, &val, sizeof(val) );
 }
 #endif
 
-static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
+static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int *recvd, int *ip_hdr_len,
                                struct icmp_reply_ctx *ctx )
 {
     struct ip_hdr *ip_hdr;
 
-    if (recvd < sizeof(*ip_hdr)) return FALSE;
+    if (*recvd < sizeof(*ip_hdr)) return FALSE;
     ip_hdr = msg->msg_iov[0].iov_base;
     if (ip_hdr->v_hl >> 4 != 4 || ip_hdr->protocol != IPPROTO_ICMP) return FALSE;
     *ip_hdr_len = (ip_hdr->v_hl & 0xf) << 2;
@@ -330,7 +336,7 @@ static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
 }
 
 #ifdef __linux__
-static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
+static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int *recvd, int *ip_hdr_len,
                                           struct icmp_reply_ctx *ctx )
 {
     struct cmsghdr *cmsg;
@@ -353,6 +359,25 @@ static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip
         case IP_TOS:
             ctx->tos = *(BYTE *)CMSG_DATA( cmsg );
             break;
+#ifdef HAVE_LINUX_ERRQUEUE_H
+        case IP_RECVERR:
+        {
+            struct sock_extended_err *e;
+            struct icmp_hdr *icmp_h;
+
+            e = (struct sock_extended_err *)CMSG_DATA( cmsg );
+            icmp_h = msg->msg_iov[0].iov_base;
+            if (e->ee_origin != SO_EE_ORIGIN_ICMP) break;
+            if (*recvd + sizeof(*icmp_h) > ARRAY_SIZE(ctx->packet)) break;
+            memmove( ctx->packet + sizeof(*icmp_h), ctx->packet, *recvd );
+            *recvd += sizeof(*icmp_h);
+            memset( icmp_h, 0, sizeof(*icmp_h) );
+            icmp_h->type = e->ee_type;
+            icmp_h->code = e->ee_code;
+            TRACE( "got error packet type %d, code %d.\n", icmp_h->type, icmp_h->code );
+            break;
+        }
+#endif
         }
     }
     return TRUE;
@@ -420,19 +445,22 @@ static int ipv4_parse_icmp_hdr( struct icmp_socket *s, struct icmp_hdr *icmp, in
         return -1;
     }
 
+    icmp_size -= sizeof(*icmp);
     if (s->ping_socket)
     {
-        /* Ping sockets only return echo reply through normal recvmsg() received data, should not get here. */
-        return -1;
+        orig_ip_hdr_len = 0;
+        orig_icmp_hdr = icmp + 1;
     }
-
-    /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
-    if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr)) return -1;
-    orig_ip_hdr = (struct ip_hdr *)(icmp + 1);
-    if (orig_ip_hdr->v_hl >> 4 != 4 || orig_ip_hdr->protocol != IPPROTO_ICMP) return -1;
-    orig_ip_hdr_len = (orig_ip_hdr->v_hl & 0xf) << 2;
-    if (icmp_size < sizeof(*icmp) + orig_ip_hdr_len + sizeof(*orig_icmp_hdr)) return -1;
-    orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + orig_ip_hdr_len);
+    else
+    {
+        /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
+        if (icmp_size < sizeof(*orig_ip_hdr)) return -1;
+        orig_ip_hdr = (struct ip_hdr *)(icmp + 1);
+        if (orig_ip_hdr->v_hl >> 4 != 4 || orig_ip_hdr->protocol != IPPROTO_ICMP) return -1;
+        orig_ip_hdr_len = (orig_ip_hdr->v_hl & 0xf) << 2;
+        orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + orig_ip_hdr_len);
+    }
+    if (icmp_size < orig_ip_hdr_len + sizeof(*orig_icmp_hdr)) return -1;
     if (orig_icmp_hdr->type != ICMP4_ECHO_REQUEST ||
         orig_icmp_hdr->code != 0) return -1;
 
@@ -502,7 +530,7 @@ struct family_ops
     unsigned short (*chksum)( struct icmp_data *icmp_data, BYTE *data, unsigned int count );
     int (*set_reply_ip_status)( IP_STATUS ip_status, unsigned int bits, void *out );
     void (*set_socket_opts)( struct icmp_socket *s );
-    BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
+    BOOL (*parse_ip_hdr)( struct msghdr *msg, int *recvd, int *ip_hdr_len, struct icmp_reply_ctx *ctx );
     int (*parse_icmp_hdr)( struct icmp_socket *s, struct icmp_hdr *icmp, int icmp_len, struct icmp_reply_ctx *ctx );
     BOOL (*fill_reply)( struct icmp_get_reply_params *params, struct icmp_reply_ctx *ctx );
 };
@@ -614,9 +642,17 @@ static void ipv6_set_socket_opts( struct icmp_socket *s )
     setsockopt( s->socket, IPPROTO_IPV6, IPV6_UNICAST_HOPS, &val, sizeof(val) );
 }
 #endif
+#ifdef IP_RECVERR
+{
+    int val = 1;
+
+    if (s->ping_socket)
+        setsockopt( s->socket, IPPROTO_IPV6, IPV6_RECVERR, &val, sizeof(val) );
+}
+#endif
 }
 
-static BOOL ipv6_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
+static BOOL ipv6_parse_ip_hdr( struct msghdr *msg, int *recvd, int *ip_hdr_len,
                                           struct icmp_reply_ctx *ctx )
 {
     *ip_hdr_len = 0;
@@ -625,6 +661,34 @@ static BOOL ipv6_parse_ip_hdr( struct msghdr *msg, int recvd, int *ip_hdr_len,
     ctx->tos = 0;
     ctx->flags = 0;
     ctx->options_size = 0;
+
+#ifdef HAVE_LINUX_ERRQUEUE_H
+{
+    struct sock_extended_err *e;
+    struct icmp_hdr *icmp_h;
+    struct cmsghdr *cmsg;
+
+    for (cmsg = CMSG_FIRSTHDR( msg ); cmsg; cmsg = CMSG_NXTHDR( msg, cmsg ))
+    {
+        if (cmsg->cmsg_level != IPPROTO_IPV6) continue;
+        switch (cmsg->cmsg_type)
+        {
+        case IPV6_RECVERR:
+            e = (struct sock_extended_err *)CMSG_DATA( cmsg );
+            icmp_h = msg->msg_iov[0].iov_base;
+            if (e->ee_origin != SO_EE_ORIGIN_ICMP6) break;
+            if (*recvd + sizeof(*icmp_h) > ARRAY_SIZE(ctx->packet)) break;
+            TRACE( "got error packet.\n" );
+            memmove( ctx->packet + sizeof(*icmp_h), ctx->packet, *recvd );
+            *recvd += sizeof(*icmp_h);
+            memset( icmp_h, 0, sizeof(*icmp_h) );
+            icmp_h->type = e->ee_type;
+            icmp_h->code = e->ee_code;
+            break;
+        }
+    }
+}
+#endif
     return TRUE;
 }
 
@@ -682,18 +746,18 @@ static int ipv6_parse_icmp_hdr( struct icmp_socket *s, struct icmp_hdr *icmp,
         return -1;
     }
 
-    if (s->ping_socket)
+    icmp_size -= sizeof(*icmp);
+    if (s->ping_socket) orig_icmp_hdr = icmp + 1;
+    else
     {
-        /* Ping sockets only return echo reply through normal recvmsg() received data, should not get here. */
-        return -1;
+        /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
+        if (icmp_size < sizeof(*orig_ip_hdr)) return -1;
+        icmp_size -= sizeof(*orig_ip_hdr);
+        orig_ip_hdr = (struct ipv6_hdr *)(icmp + 1);
+        if ((orig_ip_hdr->v_prio >> 4) != 6 || orig_ip_hdr->next_hdr != IPPROTO_ICMPV6) return -1;
+        orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + sizeof(*orig_ip_hdr));
     }
-
-    /* All handled icmp replies have an 8-byte header followed by the original ip hdr. */
-    if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr)) return -1;
-    orig_ip_hdr = (struct ipv6_hdr *)(icmp + 1);
-    if ((orig_ip_hdr->v_prio >> 4) != 6 || orig_ip_hdr->next_hdr != IPPROTO_ICMPV6) return -1;
-    if (icmp_size < sizeof(*icmp) + sizeof(*orig_ip_hdr) + sizeof(*orig_icmp_hdr)) return -1;
-    orig_icmp_hdr = (const struct icmp_hdr *)((const BYTE *)orig_ip_hdr + sizeof(*orig_ip_hdr));
+    if (icmp_size < sizeof(*orig_icmp_hdr)) return -1;
     if (orig_icmp_hdr->type != ICMP6_ECHO_REQUEST ||
         orig_icmp_hdr->code != 0) return -1;
 
@@ -1073,7 +1137,7 @@ static void icmp_listen( void *args )
         i = 0;
         LIST_FOR_EACH_ENTRY( s, &listen_list, struct icmp_socket, listen_entry )
         {
-            if (!(fds[i].revents & POLLIN)) goto skip;
+            if (!(fds[i].revents & (POLLIN | POLLERR))) goto skip;
             msg.msg_name = &addr;
             msg.msg_namelen = sizeof(addr);
             msg.msg_control = cmsg_buf;
@@ -1081,8 +1145,20 @@ static void icmp_listen( void *args )
             recvd = recvmsg( s->socket, &msg, 0 );
             TRACE( "s %p, recvmsg() rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
                    s, recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
+#ifdef __linux__
+            if (recvd < 0 && s->ping_socket && fds[i].revents & POLLERR)
+            {
+                msg.msg_name = &addr;
+                msg.msg_namelen = sizeof(addr);
+                msg.msg_control = cmsg_buf;
+                msg.msg_controllen = sizeof(cmsg_buf);
+                recvd = recvmsg( s->socket, &msg, MSG_ERRQUEUE );
+                TRACE( "s %p, recvmsg(MSG_ERRQUEUE) rets %d errno %d addr_len %d iovlen %d msg_flags %x\n",
+                       s, recvd, errno, msg.msg_namelen, (int)iov[0].iov_len, msg.msg_flags );
+            }
+#endif
             if (recvd < 0) goto skip;
-            if (!s->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, &ctx )) goto skip;
+            if (!s->ops->parse_ip_hdr( &msg, &recvd, &ip_hdr_len, &ctx )) goto skip;
             if (recvd < ip_hdr_len + sizeof(*icmp_hdr)) goto skip;
             ctx.packet_size = recvd;
             icmp_hdr = (struct icmp_hdr *)(ctx.packet + ip_hdr_len);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[PATCH v7 9/9] nsiproxy.sys: Use compatible ICMP request id and seq numbers.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/nsiproxy.sys/icmp_echo.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c
index 0d2033b5237..22f63cf1427 100644
--- a/dlls/nsiproxy.sys/icmp_echo.c
+++ b/dlls/nsiproxy.sys/icmp_echo.c
@@ -241,8 +241,8 @@ static void ipv4_init_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp_hd
     icmp_hdr->type = ICMP4_ECHO_REQUEST;
     icmp_hdr->code = 0;
     icmp_hdr->checksum = 0;
-    icmp_hdr->un.echo.id = data->s->id = getpid() & 0xffff; /* will be overwritten for linux ping socks */
-    icmp_hdr->un.echo.sequence = data->seq = InterlockedIncrement( &icmp_sequence ) & 0xffff;
+    icmp_hdr->un.echo.id = data->s->id = htons( 1 ); /* will be overwritten for linux ping socks */
+    icmp_hdr->un.echo.sequence = data->seq = htons( InterlockedIncrement( &icmp_sequence ) & 0xffff );
 }
 
 /* rfc 1071 checksum */
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/10954

[Paul Gofman (＠gofman)]

v7:
 - split "nsiproxy.sys: Poll all icmp sockets in a single thread" into 3 patches.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10954#note_142429