[PATCH v3 1/3] nsiproxy: Convert the reply to ICMP_ECHO_REPLY in the output buffer.
Because iphlpapi has no opportunity to convert the buffer in async mode. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Handles architecture differences and family (can be easily extended to support IPv6's struct). dlls/iphlpapi/iphlpapi_main.c | 44 ++---------- dlls/nsiproxy.sys/device.c | 103 ++++++++++++++++++++++++--- dlls/nsiproxy.sys/nsiproxy_private.h | 55 ++++++++++++++ include/wine/nsi.h | 21 +----- 4 files changed, 155 insertions(+), 68 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index d9a85be..a024224 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -4579,33 +4579,6 @@ DWORD WINAPI IcmpParseReplies( void *reply, DWORD reply_size ) return num_pkts; } -/************************************************************************* - * icmpv4_echo_reply_fixup - * - * Convert struct nsiproxy_icmpv4_echo_reply into ICMP_ECHO_REPLY. - * - * This is necessary due to the different sizes of ICMP_ECHO_REPLY on - * 32 and 64-bits. Despite mention of ICMP_ECHO_REPLY32, 64-bit Windows - * actually does return a full 64-bit version. - */ -static void icmpv4_echo_reply_fixup( ICMP_ECHO_REPLY *dst, struct nsiproxy_icmp_echo_reply *reply ) -{ - dst->Address = reply->addr.Ipv4.sin_addr.s_addr; - dst->Status = reply->status; - dst->RoundTripTime = reply->round_trip_time; - dst->DataSize = reply->data_size; - dst->Reserved = reply->num_of_pkts; - dst->Data = (BYTE *)(dst + 1) + ((reply->opts.options_size + 3) & ~3); - dst->Options.Ttl = reply->opts.ttl; - dst->Options.Tos = reply->opts.tos; - dst->Options.Flags = reply->opts.flags; - dst->Options.OptionsSize = reply->opts.options_size; - dst->Options.OptionsData = (BYTE *)(reply + 1); - - memcpy( dst->Options.OptionsData, (BYTE *)reply + reply->opts.options_offset, reply->opts.options_size ); - memcpy( dst->Data, (BYTE *)reply + reply->data_offset, reply->data_size ); -} - /*********************************************************************** * IcmpSendEcho (IPHLPAPI.@) */ @@ -4636,9 +4609,8 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r void *reply, DWORD reply_size, DWORD timeout ) { struct icmp_handle_data *data = (struct icmp_handle_data *)handle; - DWORD opt_size, in_size, ret = 0, out_size; + DWORD opt_size, in_size, ret = 0; struct nsiproxy_icmp_echo *in; - struct nsiproxy_icmp_echo_reply *out; HANDLE request_event; IO_STATUS_BLOCK iosb; NTSTATUS status; @@ -4658,17 +4630,15 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r opt_size = opts ? (opts->OptionsSize + 3) & ~3 : 0; in_size = FIELD_OFFSET(struct nsiproxy_icmp_echo, data[opt_size + request_size]); in = heap_alloc_zero( in_size ); - out_size = reply_size - sizeof(ICMP_ECHO_REPLY) + sizeof(*out); - out = heap_alloc( out_size ); - if (!in || !out) + if (!in) { - heap_free( out ); - heap_free( in ); SetLastError( IP_NO_RESOURCES ); return 0; } + in->addr = (ULONG_PTR)reply; + in->bits = sizeof(void*) * 8; in->src.Ipv4.sin_family = AF_INET; in->src.Ipv4.sin_addr.s_addr = src; in->dst.Ipv4.sin_family = AF_INET; @@ -4689,19 +4659,15 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r status = NtDeviceIoControlFile( data->nsi_device, request_event, NULL, NULL, &iosb, IOCTL_NSIPROXY_WINE_ICMP_ECHO, in, in_size, - out, out_size ); + reply, reply_size ); if (status == STATUS_PENDING && !WaitForSingleObject( request_event, INFINITE )) status = iosb.Status; if (!status) - { - icmpv4_echo_reply_fixup( reply, out ); ret = IcmpParseReplies( reply, reply_size ); - } CloseHandle( request_event ); - heap_free( out ); heap_free( in ); if (status) SetLastError( RtlNtStatusToDosError( status ) ); diff --git a/dlls/nsiproxy.sys/device.c b/dlls/nsiproxy.sys/device.c index 90a7a3d..e310805 100644 --- a/dlls/nsiproxy.sys/device.c +++ b/dlls/nsiproxy.sys/device.c @@ -31,6 +31,7 @@ #include "netiodef.h" #include "wine/nsi.h" #include "wine/debug.h" +#include "wine/heap.h" #include "wine/unixlib.h" #include "nsiproxy_private.h" @@ -211,6 +212,57 @@ static void WINAPI icmp_echo_cancel( DEVICE_OBJECT *device, IRP *irp ) LeaveCriticalSection( &nsiproxy_cs ); } +static void icmp_echo_reply_fixup( void *out, struct nsiproxy_icmp_echo_reply *reply, ULONGLONG addr, ULONG family, ULONG bits ) +{ + void *data = out; + if (family == AF_INET) + { + void *options_data; + if (bits == 32) + { + struct icmp_echo_reply_32 *dst = out; + dst->addr = reply->addr.Ipv4.sin_addr.s_addr; + dst->status = reply->status; + dst->round_trip_time = reply->round_trip_time; + dst->data_size = reply->data_size; + dst->num_of_pkts = reply->num_of_pkts; + dst->data_ptr = addr + sizeof(*dst) + ((reply->opts.options_size + 3) & ~3); + dst->opts.ttl = reply->opts.ttl; + dst->opts.tos = reply->opts.tos; + dst->opts.flags = reply->opts.flags; + dst->opts.options_size = reply->opts.options_size; + dst->opts.options_ptr = addr + sizeof(*dst); + options_data = dst + 1; + } + else + { + struct icmp_echo_reply_64 *dst = out; + dst->addr = reply->addr.Ipv4.sin_addr.s_addr; + dst->status = reply->status; + dst->round_trip_time = reply->round_trip_time; + dst->data_size = reply->data_size; + dst->num_of_pkts = reply->num_of_pkts; + dst->data_ptr = addr + sizeof(*dst) + ((reply->opts.options_size + 3) & ~3); + dst->opts.ttl = reply->opts.ttl; + dst->opts.tos = reply->opts.tos; + dst->opts.flags = reply->opts.flags; + dst->opts.options_size = reply->opts.options_size; + dst->opts.options_ptr = addr + sizeof(*dst); + options_data = dst + 1; + } + memcpy( options_data, (BYTE *)reply + reply->opts.options_offset, reply->opts.options_size ); + data = (BYTE *)options_data + ((reply->opts.options_size + 3) & ~3); + } + memcpy( data, (BYTE *)reply + reply->data_offset, reply->data_size ); +} + +static int icmp_echo_reply_struct_len( ULONG family, ULONG bits ) +{ + if (family == AF_INET) + return (bits == 32) ? sizeof(struct icmp_echo_reply_32) : sizeof(struct icmp_echo_reply_64); + return 0; +} + static NTSTATUS nsiproxy_icmp_echo( IRP *irp ) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); @@ -222,7 +274,7 @@ static NTSTATUS nsiproxy_icmp_echo( IRP *irp ) if (in_len < offsetof(struct nsiproxy_icmp_echo, data[0]) || in_len < offsetof(struct nsiproxy_icmp_echo, data[((in->opt_size + 3) & ~3) + in->req_size]) || - out_len < sizeof(struct nsiproxy_icmp_echo_reply)) + out_len < icmp_echo_reply_struct_len( in->dst.si_family, in->bits )) return STATUS_INVALID_PARAMETER; switch (in->dst.si_family) @@ -320,19 +372,34 @@ static DWORD WINAPI listen_thread_proc( void *arg ) { IRP *irp = arg; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); + unsigned int reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer; struct icmp_listen_params params; + ULONG family = in->dst.si_family; + ULONGLONG addr = in->addr; + ULONG bits = in->bits; + ULONG struct_len; NTSTATUS status; TRACE( "\n" ); + struct_len = icmp_echo_reply_struct_len( family, bits ); params.handle = irp_get_icmp_handle( irp ); params.timeout = in->timeout; - params.reply = irp->AssociatedIrp.SystemBuffer; - params.reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; + params.reply_len = reply_len - struct_len + sizeof(struct nsiproxy_icmp_echo_reply); + params.reply = heap_alloc( params.reply_len ); - status = nsiproxy_call( icmp_listen, ¶ms ); - TRACE( "icmp_listen rets %08x\n", status ); + if (params.reply) + { + void *reply = irp->AssociatedIrp.SystemBuffer; + status = nsiproxy_call( icmp_listen, ¶ms ); + TRACE( "icmp_listen rets %08x\n", status ); + + icmp_echo_reply_fixup( reply, params.reply, addr, family, bits ); + heap_free( params.reply ); + reply_len = params.reply_len - sizeof(struct nsiproxy_icmp_echo_reply) + struct_len; + } + else status = STATUS_NO_MEMORY; EnterCriticalSection( &nsiproxy_cs ); @@ -340,7 +407,7 @@ static DWORD WINAPI listen_thread_proc( void *arg ) irp->IoStatus.Status = status; if (status == STATUS_SUCCESS) - irp->IoStatus.Information = params.reply_len; + irp->IoStatus.Information = reply_len; else irp->IoStatus.Information = 0; IoCompleteRequest( irp, IO_NO_INCREMENT ); @@ -353,8 +420,10 @@ static DWORD WINAPI listen_thread_proc( void *arg ) static void handle_queued_send_echo( IRP *irp ) { struct nsiproxy_icmp_echo *in = (struct nsiproxy_icmp_echo *)irp->AssociatedIrp.SystemBuffer; - struct nsiproxy_icmp_echo_reply *reply = (struct nsiproxy_icmp_echo_reply *)irp->AssociatedIrp.SystemBuffer; + void *out = irp->AssociatedIrp.SystemBuffer; struct icmp_send_echo_params params; + ULONG family = in->dst.si_family; + ULONG bits = in->bits; NTSTATUS status; TRACE( "\n" ); @@ -372,9 +441,23 @@ static void handle_queued_send_echo( IRP *irp ) irp->IoStatus.Status = status; if (status == STATUS_SUCCESS) { - memset( reply, 0, sizeof(*reply) ); - reply->status = params.ip_status; - irp->IoStatus.Information = sizeof(*reply); + if (family == AF_INET) + { + if (bits == 32) + { + struct icmp_echo_reply_32 *reply = out; + memset( reply, 0, sizeof(*reply) ); + reply->status = params.ip_status; + irp->IoStatus.Information = sizeof(*reply); + } + else + { + struct icmp_echo_reply_64 *reply = out; + memset( reply, 0, sizeof(*reply) ); + reply->status = params.ip_status; + irp->IoStatus.Information = sizeof(*reply); + } + } } IoCompleteRequest( irp, IO_NO_INCREMENT ); } diff --git a/dlls/nsiproxy.sys/nsiproxy_private.h b/dlls/nsiproxy.sys/nsiproxy_private.h index 91dd393..0694757 100644 --- a/dlls/nsiproxy.sys/nsiproxy_private.h +++ b/dlls/nsiproxy.sys/nsiproxy_private.h @@ -34,3 +34,58 @@ struct icmp_send_echo_params HANDLE handle; ULONG ip_status; }; + +/* output for IOCTL_NSIPROXY_WINE_ICMP_ECHO - cf. ICMP_ECHO_REPLY */ +struct nsiproxy_icmp_echo_reply +{ + SOCKADDR_INET addr; + ULONG status; + ULONG round_trip_time; + USHORT data_size; + USHORT num_of_pkts; + DWORD data_offset; + struct + { + BYTE ttl; + BYTE tos; + BYTE flags; + BYTE options_size; + DWORD options_offset; + } opts; +}; + +struct icmp_echo_reply_32 +{ + ULONG addr; + ULONG status; + ULONG round_trip_time; + USHORT data_size; + USHORT num_of_pkts; + ULONG data_ptr; + struct + { + BYTE ttl; + BYTE tos; + BYTE flags; + BYTE options_size; + ULONG options_ptr; + } opts; +}; + +struct icmp_echo_reply_64 +{ + ULONG addr; + ULONG status; + ULONG round_trip_time; + USHORT data_size; + USHORT num_of_pkts; + ULONGLONG data_ptr; + struct + { + BYTE ttl; + BYTE tos; + BYTE flags; + BYTE options_size; + ULONGLONG options_ptr; + } opts; +}; diff --git a/include/wine/nsi.h b/include/wine/nsi.h index 9664b53..4e60ff6 100644 --- a/include/wine/nsi.h +++ b/include/wine/nsi.h @@ -426,6 +426,8 @@ struct nsiproxy_icmp_echo { SOCKADDR_INET src; SOCKADDR_INET dst; + ULONGLONG addr; /* user-mode reply buffer address */ + BYTE bits; BYTE ttl; BYTE tos; BYTE flags; @@ -435,25 +437,6 @@ struct nsiproxy_icmp_echo BYTE data[1]; /* ((opt_size + 3) & ~3) + req_size */ }; -/* output for IOCTL_NSIPROXY_WINE_ICMP_ECHO - cf. ICMP_ECHO_REPLY */ -struct nsiproxy_icmp_echo_reply -{ - SOCKADDR_INET addr; - ULONG status; - ULONG round_trip_time; - USHORT data_size; - USHORT num_of_pkts; - DWORD data_offset; - struct - { - BYTE ttl; - BYTE tos; - BYTE flags; - BYTE options_size; - DWORD options_offset; - } opts; -}; - /* Undocumented Nsi api */ #define NSI_PARAM_TYPE_RW 0 -- 2.31.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/iphlpapi/iphlpapi_main.c | 19 ++- dlls/iphlpapi/tests/iphlpapi.c | 248 +++++++++++++++++++++++++++++++++ 2 files changed, 256 insertions(+), 11 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index a024224..cfb735c 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -4615,12 +4615,6 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r IO_STATUS_BLOCK iosb; NTSTATUS status; - if (event || apc_routine) - { - FIXME( "Async requests not yet supported\n" ); - return 0; - } - if (handle == INVALID_HANDLE_VALUE || !reply) { SetLastError( ERROR_INVALID_PARAMETER ); @@ -4655,19 +4649,22 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r in->timeout = timeout; memcpy( in->data + opt_size, request, request_size ); - request_event = CreateEventW( NULL, 0, 0, NULL ); + request_event = event ? event : (apc_routine ? NULL : CreateEventW( NULL, 0, 0, NULL )); - status = NtDeviceIoControlFile( data->nsi_device, request_event, NULL, NULL, + status = NtDeviceIoControlFile( data->nsi_device, request_event, apc_routine, apc_ctxt, &iosb, IOCTL_NSIPROXY_WINE_ICMP_ECHO, in, in_size, reply, reply_size ); - if (status == STATUS_PENDING && !WaitForSingleObject( request_event, INFINITE )) - status = iosb.Status; + if (status == STATUS_PENDING) + { + if (!event && !apc_routine && !WaitForSingleObject( request_event, INFINITE )) + status = iosb.Status; + } if (!status) ret = IcmpParseReplies( reply, reply_size ); - CloseHandle( request_event ); + if (!event && request_event) CloseHandle( request_event ); heap_free( in ); if (status) SetLastError( RtlNtStatusToDosError( status ) ); diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c index e20ada8..2adacd3 100644 --- a/dlls/iphlpapi/tests/iphlpapi.c +++ b/dlls/iphlpapi/tests/iphlpapi.c @@ -38,6 +38,7 @@ #include "winsock2.h" #include "windef.h" #include "winbase.h" +#include "winternl.h" #include "ws2tcpip.h" #include "windns.h" #include "iphlpapi.h" @@ -873,13 +874,35 @@ static void testSetTcpEntry(void) "got %u, expected %u\n", ret, ERROR_MR_MID_NOT_FOUND); } +static BOOL icmp_send_echo_test_apc_expect; +static void WINAPI icmp_send_echo_test_apc_xp(void *context) +{ + ok(icmp_send_echo_test_apc_expect, "Unexpected APC execution\n"); + ok(context == (void*)0xdeadc0de, "Wrong context: %p\n", context); + icmp_send_echo_test_apc_expect = FALSE; +} + +static void WINAPI icmp_send_echo_test_apc(void *context, IO_STATUS_BLOCK *io_status, ULONG reserved) +{ + icmp_send_echo_test_apc_xp(context); + ok(io_status->Status == 0, "Got IO Status 0x%08x\n", io_status->Status); + ok(io_status->Information == sizeof(ICMP_ECHO_REPLY) + 32 /* sizeof(senddata) */, + "Got IO Information %lu\n", io_status->Information); +} + static void testIcmpSendEcho(void) { + /* The APC's signature is different pre-Vista */ + const PIO_APC_ROUTINE apc = broken(LOBYTE(LOWORD(GetVersion())) < 6) + ? (PIO_APC_ROUTINE)icmp_send_echo_test_apc_xp + : icmp_send_echo_test_apc; HANDLE icmp; char senddata[32], replydata[sizeof(senddata) + sizeof(ICMP_ECHO_REPLY)]; + char replydata2[sizeof(replydata) + sizeof(IO_STATUS_BLOCK)]; DWORD ret, error, replysz = sizeof(replydata); IPAddr address; ICMP_ECHO_REPLY *reply; + HANDLE event; INT i; memset(senddata, 0, sizeof(senddata)); @@ -893,6 +916,15 @@ static void testIcmpSendEcho(void) || broken(error == ERROR_INVALID_HANDLE) /* <= 2003 */, "expected 87, got %d\n", error); + address = htonl(INADDR_LOOPBACK); + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(INVALID_HANDLE_VALUE, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata, replysz, 1000); + error = GetLastError(); + ok (!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok (error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_INVALID_HANDLE) /* <= 2003 */, + "expected 87, got %d\n", error); + icmp = IcmpCreateFile(); ok (icmp != INVALID_HANDLE_VALUE, "IcmpCreateFile failed unexpectedly with error %d\n", GetLastError()); @@ -1036,6 +1068,222 @@ static void testIcmpSendEcho(void) ok(reply->DataSize == sizeof(senddata), "Got size:%d\n", reply->DataSize); ok(!memcmp(senddata, reply->Data, min(sizeof(senddata), reply->DataSize)), "Data mismatch\n"); + + /* + * IcmpSendEcho2 + */ + address = 0; + replysz = sizeof(replydata2); + memset(senddata, 0, sizeof(senddata)); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_NETNAME + || broken(error == IP_BAD_DESTINATION) /* <= 2003 */, + "expected 1214, got %d\n", error); + + event = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(event != NULL, "CreateEventW failed unexpectedly with error %d\n", GetLastError()); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 returned success unexpectedly\n"); + ok(error == ERROR_INVALID_NETNAME + || broken(error == ERROR_IO_PENDING) /* <= 2003 */, + "Got last error: 0x%08x\n", error); + if (error == ERROR_IO_PENDING) + { + ret = WaitForSingleObjectEx(event, 2000, TRUE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObjectEx failed unexpectedly with %u\n", ret); + } + + address = htonl(INADDR_LOOPBACK); + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, NULL, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_NOACCESS) /* <= 2003 */, + "expected 87, got %d\n", error); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, NULL, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_NOACCESS) /* <= 2003 */, + "expected 87, got %d\n", error); + ok(WaitForSingleObjectEx(event, 0, TRUE) == WAIT_TIMEOUT, "Event was unexpectedly signalled.\n"); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, 0, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_INSUFFICIENT_BUFFER) /* <= 2003 */, + "expected 87, got %d\n", error); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, 0, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_INSUFFICIENT_BUFFER) /* <= 2003 */, + "expected 87, got %d\n", error); + ok(WaitForSingleObjectEx(event, 0, TRUE) == WAIT_TIMEOUT, "Event was unexpectedly signalled.\n"); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, NULL, 0, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_INSUFFICIENT_BUFFER) /* <= 2003 */, + "expected 87, got %d\n", error); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, NULL, 0, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 succeeded unexpectedly\n"); + ok(error == ERROR_INVALID_PARAMETER + || broken(error == ERROR_INSUFFICIENT_BUFFER) /* <= 2003 */, + "expected 87, got %d\n", error); + ok(WaitForSingleObjectEx(event, 0, TRUE) == WAIT_TIMEOUT, "Event was unexpectedly signalled.\n"); + + /* synchronous tests */ + SetLastError(0xdeadbeef); + address = htonl(INADDR_LOOPBACK); + replysz = sizeof(ICMP_ECHO_REPLY) + sizeof(IO_STATUS_BLOCK); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, 0, NULL, replydata2, replysz, 1000); + ok(ret, "IcmpSendEcho2 failed unexpectedly with error %d\n", GetLastError()); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, NULL, 0, NULL, replydata2, replysz, 1000); + ok(ret, "IcmpSendEcho2 failed unexpectedly with error %d\n", GetLastError()); + + SetLastError(0xdeadbeef); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, 0, NULL, replydata2, replysz, 1000); + ok(ret, "IcmpSendEcho2 failed unexpectedly with error %d\n", GetLastError()); + + SetLastError(0xdeadbeef); + replysz = sizeof(ICMP_ECHO_REPLY) + sizeof(IO_STATUS_BLOCK) + ICMP_MINLEN; + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, ICMP_MINLEN, NULL, replydata2, replysz, 1000); + ok(ret, "IcmpSendEcho2 failed unexpectedly with error %d\n", GetLastError()); + + SetLastError(0xdeadbeef); + replysz = sizeof(replydata2); + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + if (!ret) + { + error = GetLastError(); + skip("Failed to ping with error %d, is lo interface down?\n", error); + } + else if (winetest_debug > 1) + { + reply = (ICMP_ECHO_REPLY*)replydata2; + trace("send addr : %s\n", ntoa(address)); + trace("reply addr : %s\n", ntoa(reply->Address)); + trace("reply size : %u\n", replysz); + trace("roundtrip : %u ms\n", reply->RoundTripTime); + trace("status : %u\n", reply->Status); + trace("recv size : %u\n", reply->DataSize); + trace("ttl : %u\n", reply->Options.Ttl); + trace("flags : 0x%x\n", reply->Options.Flags); + } + + SetLastError(0xdeadbeef); + for (i = 0; i < ARRAY_SIZE(senddata); i++) senddata[i] = i & 0xff; + ret = IcmpSendEcho2(icmp, NULL, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + reply = (ICMP_ECHO_REPLY*)replydata2; + ok(ret, "IcmpSendEcho2 failed unexpectedly\n"); + ok(error == NO_ERROR, "Expect last error: 0x%08x, got: 0x%08x\n", NO_ERROR, error); + ok(ntohl(reply->Address) == INADDR_LOOPBACK, "Address mismatch, expect: %s, got: %s\n", ntoa(INADDR_LOOPBACK), + ntoa(reply->Address)); + ok(reply->Status == IP_SUCCESS, "Expect status: 0x%08x, got: 0x%08x\n", IP_SUCCESS, reply->Status); + ok(reply->DataSize == sizeof(senddata), "Got size: %d\n", reply->DataSize); + ok(!memcmp(senddata, reply->Data, min(sizeof(senddata), reply->DataSize)), "Data mismatch\n"); + + /* asynchronous tests with event */ + SetLastError(0xdeadbeef); + replysz = sizeof(replydata2); + address = htonl(INADDR_LOOPBACK); + memset(senddata, 0, sizeof(senddata)); + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + if (!ret && error != ERROR_IO_PENDING) + { + skip("Failed to ping with error %d, is lo interface down?\n", error); + } + else + { + ok(!ret, "IcmpSendEcho2 returned success unexpectedly\n"); + ok(error == ERROR_IO_PENDING, "Expect last error: 0x%08x, got: 0x%08x\n", ERROR_IO_PENDING, error); + ret = WaitForSingleObjectEx(event, 2000, TRUE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObjectEx failed unexpectedly with %u\n", ret); + reply = (ICMP_ECHO_REPLY*)replydata2; + ok(ntohl(reply->Address) == INADDR_LOOPBACK, "Address mismatch, expect: %s, got: %s\n", ntoa(INADDR_LOOPBACK), + ntoa(reply->Address)); + ok(reply->Status == IP_SUCCESS, "Expect status: 0x%08x, got: 0x%08x\n", IP_SUCCESS, reply->Status); + ok(reply->DataSize == sizeof(senddata), "Got size: %d\n", reply->DataSize); + if (winetest_debug > 1) + { + reply = (ICMP_ECHO_REPLY*)replydata2; + trace("send addr : %s\n", ntoa(address)); + trace("reply addr : %s\n", ntoa(reply->Address)); + trace("reply size : %u\n", replysz); + trace("roundtrip : %u ms\n", reply->RoundTripTime); + trace("status : %u\n", reply->Status); + trace("recv size : %u\n", reply->DataSize); + trace("ttl : %u\n", reply->Options.Ttl); + trace("flags : 0x%x\n", reply->Options.Flags); + } + } + + SetLastError(0xdeadbeef); + for (i = 0; i < ARRAY_SIZE(senddata); i++) senddata[i] = i & 0xff; + ret = IcmpSendEcho2(icmp, event, NULL, NULL, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 returned success unexpectedly\n"); + ok(error == ERROR_IO_PENDING, "Expect last error: 0x%08x, got: 0x%08x\n", ERROR_IO_PENDING, error); + ret = WaitForSingleObjectEx(event, 2000, TRUE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObjectEx failed unexpectedly with %u\n", ret); + reply = (ICMP_ECHO_REPLY*)replydata2; + ok(ntohl(reply->Address) == INADDR_LOOPBACK, "Address mismatch, expect: %s, got: %s\n", ntoa(INADDR_LOOPBACK), + ntoa(reply->Address)); + ok(reply->Status == IP_SUCCESS, "Expect status: 0x%08x, got: 0x%08x\n", IP_SUCCESS, reply->Status); + ok(reply->DataSize == sizeof(senddata), "Got size: %d\n", reply->DataSize); + /* pre-Vista, reply->Data is an offset; otherwise it's a pointer, so hardcode the offset */ + ok(!memcmp(senddata, reply + 1, min(sizeof(senddata), reply->DataSize)), "Data mismatch\n"); + + CloseHandle(event); + + /* asynchronous tests with APC */ + SetLastError(0xdeadbeef); + replysz = sizeof(replydata2) + 10; + address = htonl(INADDR_LOOPBACK); + for (i = 0; i < ARRAY_SIZE(senddata); i++) senddata[i] = ~i & 0xff; + icmp_send_echo_test_apc_expect = TRUE; + /* + NOTE: Supplying both event and apc has varying behavior across Windows versions, so not tested. + */ + ret = IcmpSendEcho2(icmp, NULL, apc, (void*)0xdeadc0de, address, senddata, sizeof(senddata), NULL, replydata2, replysz, 1000); + error = GetLastError(); + ok(!ret, "IcmpSendEcho2 returned success unexpectedly\n"); + ok(error == ERROR_IO_PENDING, "Expect last error: 0x%08x, got: 0x%08x\n", ERROR_IO_PENDING, error); + SleepEx(200, TRUE); + SleepEx(0, TRUE); + ok(icmp_send_echo_test_apc_expect == FALSE, "APC was not executed!\n"); + reply = (ICMP_ECHO_REPLY*)replydata2; + ok(ntohl(reply->Address) == INADDR_LOOPBACK, "Address mismatch, expect: %s, got: %s\n", ntoa(INADDR_LOOPBACK), + ntoa(reply->Address)); + ok(reply->Status == IP_SUCCESS, "Expect status: 0x%08x, got: 0x%08x\n", IP_SUCCESS, reply->Status); + ok(reply->DataSize == sizeof(senddata), "Got size: %d\n", reply->DataSize); + /* pre-Vista, reply->Data is an offset; otherwise it's a pointer, so hardcode the offset */ + ok(!memcmp(senddata, reply + 1, min(sizeof(senddata), reply->DataSize)), "Data mismatch\n"); + IcmpCloseHandle(icmp); } -- 2.31.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- IMO it's better to have it for safety. Because this would be hell to debug if an app does depend on it and then we get corrupted memory (assuming it releases the buffers after it closes it, for example). dlls/iphlpapi/iphlpapi_main.c | 10 +++ dlls/nsiproxy.sys/device.c | 131 +++++++++++++++++++++++++++++++++- include/wine/nsi.h | 1 + 3 files changed, 139 insertions(+), 3 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index cfb735c..2730c36 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -4536,7 +4536,17 @@ struct icmp_handle_data BOOL WINAPI IcmpCloseHandle( HANDLE handle ) { struct icmp_handle_data *data = (struct icmp_handle_data *)handle; + IO_STATUS_BLOCK iosb; + NTSTATUS status; + HANDLE event; + event = CreateEventW( NULL, 0, 0, NULL ); + status = NtDeviceIoControlFile( data->nsi_device, event, NULL, NULL, + &iosb, IOCTL_NSIPROXY_WINE_ICMP_CLOSE, NULL, 0, + NULL, 0 ); + if (status == STATUS_PENDING) + WaitForSingleObject( event, INFINITE ); + CloseHandle( event ); CloseHandle( data->nsi_device ); heap_free( data ); return TRUE; diff --git a/dlls/nsiproxy.sys/device.c b/dlls/nsiproxy.sys/device.c index e310805..93415f5 100644 --- a/dlls/nsiproxy.sys/device.c +++ b/dlls/nsiproxy.sys/device.c @@ -52,6 +52,12 @@ DECLARE_CRITICAL_SECTION( nsiproxy_cs ); #define LIST_ENTRY_INIT( list ) { .Flink = &(list), .Blink = &(list) } static LIST_ENTRY request_queue = LIST_ENTRY_INIT( request_queue ); +struct irp_file_ctx +{ + HANDLE async_event; + ULONG_PTR async_count; +}; + static NTSTATUS nsiproxy_call( unsigned int code, void *args ) { return __wine_unix_call( nsiproxy_handle, code, args ); @@ -68,6 +74,18 @@ enum unix_calls nsi_get_parameter_ex, }; +static void complete_async( IRP *irp ) +{ + struct irp_file_ctx *ctx = IoGetCurrentIrpStackLocation( irp )->FileObject->FsContext; + + if (ctx) + { + ctx->async_count--; + SetEvent( ctx->async_event ); + } + IoCompleteRequest( irp, IO_NO_INCREMENT ); +} + static NTSTATUS nsiproxy_enumerate_all( IRP *irp ) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); @@ -269,6 +287,7 @@ static NTSTATUS nsiproxy_icmp_echo( IRP *irp ) struct nsiproxy_icmp_echo *in = (struct nsiproxy_icmp_echo *)irp->AssociatedIrp.SystemBuffer; DWORD in_len = irpsp->Parameters.DeviceIoControl.InputBufferLength; DWORD out_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; + struct irp_file_ctx *ctx; TRACE( "\n" ); @@ -297,6 +316,10 @@ static NTSTATUS nsiproxy_icmp_echo( IRP *irp ) return STATUS_CANCELLED; } + ctx = irpsp->FileObject->FsContext; + if (ctx) + ctx->async_count++; + InsertTailList( &request_queue, &irp->Tail.Overlay.ListEntry ); IoMarkIrpPending( irp ); @@ -306,6 +329,58 @@ static NTSTATUS nsiproxy_icmp_echo( IRP *irp ) return STATUS_PENDING; } +static DWORD WINAPI icmp_close_wait_proc( void *arg ) +{ + IRP *irp = arg; + FILE_OBJECT *obj = IoGetCurrentIrpStackLocation( irp )->FileObject; + struct irp_file_ctx *ctx; + HANDLE async_event; + + TRACE( "%p\n", obj ); + + for (;;) + { + EnterCriticalSection( &nsiproxy_cs ); + ctx = obj->FsContext; + if (!ctx || !ctx->async_count) + break; + async_event = ctx->async_event; + LeaveCriticalSection( &nsiproxy_cs ); + WaitForSingleObjectEx( async_event, INFINITE, TRUE ); + } + irp->IoStatus.Status = STATUS_SUCCESS; + irp->IoStatus.Information = 0; + IoCompleteRequest( irp, IO_NO_INCREMENT ); + + LeaveCriticalSection( &nsiproxy_cs ); + return 0; +} + +static NTSTATUS nsiproxy_icmp_close( IRP *irp ) +{ + FILE_OBJECT *obj = IoGetCurrentIrpStackLocation( irp )->FileObject; + struct irp_file_ctx *ctx; + + TRACE( "%p\n", obj ); + + EnterCriticalSection( &nsiproxy_cs ); + ctx = obj->FsContext; + if (ctx) + { + /* Native waits until all outstanding async requests are complete or timed out */ + if (ctx->async_count) + { + IoMarkIrpPending( irp ); + LeaveCriticalSection( &nsiproxy_cs ); + RtlQueueWorkItem( icmp_close_wait_proc, irp, WT_EXECUTELONGFUNCTION ); + return STATUS_PENDING; + } + } + LeaveCriticalSection( &nsiproxy_cs ); + + return STATUS_SUCCESS; +} + static NTSTATUS WINAPI nsi_ioctl( DEVICE_OBJECT *device, IRP *irp ) { IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); @@ -334,6 +409,10 @@ static NTSTATUS WINAPI nsi_ioctl( DEVICE_OBJECT *device, IRP *irp ) status = nsiproxy_icmp_echo( irp ); break; + case IOCTL_NSIPROXY_WINE_ICMP_CLOSE: + status = nsiproxy_icmp_close( irp ); + break; + default: FIXME( "ioctl %x not supported\n", irpsp->Parameters.DeviceIoControl.IoControlCode ); status = STATUS_NOT_SUPPORTED; @@ -348,6 +427,50 @@ static NTSTATUS WINAPI nsi_ioctl( DEVICE_OBJECT *device, IRP *irp ) return status; } +static NTSTATUS WINAPI nsi_create( DEVICE_OBJECT *device, IRP *irp ) +{ + FILE_OBJECT *obj = IoGetCurrentIrpStackLocation( irp )->FileObject; + struct irp_file_ctx *ctx; + + TRACE( "%p\n", obj ); + + if (!(ctx = heap_alloc( sizeof(*ctx) ))) + return STATUS_NO_MEMORY; + if (!(ctx->async_event = CreateEventW( NULL, FALSE, FALSE, NULL ))) + { + heap_free( ctx ); + return STATUS_NO_MEMORY; + } + ctx->async_count = 0; + obj->FsContext = ctx; + + irp->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest( irp, IO_NO_INCREMENT ); + return STATUS_SUCCESS; +} + +static NTSTATUS WINAPI nsi_close( DEVICE_OBJECT *device, IRP *irp ) +{ + FILE_OBJECT *obj = IoGetCurrentIrpStackLocation( irp )->FileObject; + struct irp_file_ctx *ctx; + + TRACE( "%p\n", obj ); + + EnterCriticalSection( &nsiproxy_cs ); + ctx = obj->FsContext; + if (ctx) + { + obj->FsContext = NULL; + CloseHandle( ctx->async_event ); + heap_free( ctx ); + } + irp->IoStatus.Status = STATUS_SUCCESS; + IoCompleteRequest( irp, IO_NO_INCREMENT ); + + LeaveCriticalSection( &nsiproxy_cs ); + return STATUS_SUCCESS; +} + static int add_device( DRIVER_OBJECT *driver ) { UNICODE_STRING name, link; @@ -410,7 +533,7 @@ static DWORD WINAPI listen_thread_proc( void *arg ) irp->IoStatus.Information = reply_len; else irp->IoStatus.Information = 0; - IoCompleteRequest( irp, IO_NO_INCREMENT ); + complete_async( irp ); LeaveCriticalSection( &nsiproxy_cs ); @@ -459,7 +582,7 @@ static void handle_queued_send_echo( IRP *irp ) } } } - IoCompleteRequest( irp, IO_NO_INCREMENT ); + complete_async( irp ); } else { @@ -484,7 +607,7 @@ static DWORD WINAPI request_thread_proc( void *arg ) { irp->IoStatus.Status = STATUS_CANCELLED; TRACE( "already cancelled\n" ); - IoCompleteRequest( irp, IO_NO_INCREMENT ); + complete_async( irp ); continue; } @@ -508,6 +631,8 @@ NTSTATUS WINAPI DriverEntry( DRIVER_OBJECT *driver, UNICODE_STRING *path ) &nsiproxy_handle, sizeof(nsiproxy_handle), NULL ); if (status) return status; + driver->MajorFunction[IRP_MJ_CREATE] = nsi_create; + driver->MajorFunction[IRP_MJ_CLOSE] = nsi_close; driver->MajorFunction[IRP_MJ_DEVICE_CONTROL] = nsi_ioctl; add_device( driver ); diff --git a/include/wine/nsi.h b/include/wine/nsi.h index 4e60ff6..68b3ed9 100644 --- a/include/wine/nsi.h +++ b/include/wine/nsi.h @@ -381,6 +381,7 @@ struct nsi_udp_endpoint_static #define IOCTL_NSIPROXY_WINE_GET_ALL_PARAMETERS CTL_CODE(FILE_DEVICE_NETWORK, 0x401, METHOD_BUFFERED, 0) #define IOCTL_NSIPROXY_WINE_GET_PARAMETER CTL_CODE(FILE_DEVICE_NETWORK, 0x402, METHOD_BUFFERED, 0) #define IOCTL_NSIPROXY_WINE_ICMP_ECHO CTL_CODE(FILE_DEVICE_NETWORK, 0x403, METHOD_BUFFERED, 0) +#define IOCTL_NSIPROXY_WINE_ICMP_CLOSE CTL_CODE(FILE_DEVICE_NETWORK, 0x404, METHOD_BUFFERED, 0) /* input for IOCTL_NSIPROXY_WINE_ENUMERATE_ALL */ struct nsiproxy_enumerate_all -- 2.31.1
On Mon, Oct 11, 2021 at 06:14:38PM +0300, Gabriel Ivăncescu wrote: I haven't looked at this properly, but here are a few comments. The patch subject could do with some work.
@@ -4658,17 +4630,15 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r opt_size = opts ? (opts->OptionsSize + 3) & ~3 : 0; in_size = FIELD_OFFSET(struct nsiproxy_icmp_echo, data[opt_size + request_size]); in = heap_alloc_zero( in_size ); - out_size = reply_size - sizeof(ICMP_ECHO_REPLY) + sizeof(*out); - out = heap_alloc( out_size );
- if (!in || !out) + if (!in) { - heap_free( out ); - heap_free( in ); SetLastError( IP_NO_RESOURCES ); return 0; }
+ in->addr = (ULONG_PTR)reply;
In the context of sending an icmp message, "addr" is an extremely poor name choice! Something like "user_reply_ptr" would be better.
@@ -320,19 +372,34 @@ static DWORD WINAPI listen_thread_proc( void *arg ) { IRP *irp = arg; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); + unsigned int reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer; struct icmp_listen_params params; + ULONG family = in->dst.si_family; + ULONGLONG addr = in->addr; + ULONG bits = in->bits;
Why do "family", "bits" and "addr" need separate variables? Please drop patch [3/3]. Huw.
Hi Huw, Thanks for the review. On 14/10/2021 10:02, Huw Davies wrote:
On Mon, Oct 11, 2021 at 06:14:38PM +0300, Gabriel Ivăncescu wrote:
I haven't looked at this properly, but here are a few comments.
The patch subject could do with some work.
@@ -4658,17 +4630,15 @@ DWORD WINAPI IcmpSendEcho2Ex( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc_r opt_size = opts ? (opts->OptionsSize + 3) & ~3 : 0; in_size = FIELD_OFFSET(struct nsiproxy_icmp_echo, data[opt_size + request_size]); in = heap_alloc_zero( in_size ); - out_size = reply_size - sizeof(ICMP_ECHO_REPLY) + sizeof(*out); - out = heap_alloc( out_size );
- if (!in || !out) + if (!in) { - heap_free( out ); - heap_free( in ); SetLastError( IP_NO_RESOURCES ); return 0; }
+ in->addr = (ULONG_PTR)reply;
In the context of sending an icmp message, "addr" is an extremely poor name choice! Something like "user_reply_ptr" would be better.
Yeah I just realized how confusing it can be. :-)
@@ -320,19 +372,34 @@ static DWORD WINAPI listen_thread_proc( void *arg ) { IRP *irp = arg; IO_STACK_LOCATION *irpsp = IoGetCurrentIrpStackLocation( irp ); + unsigned int reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; struct nsiproxy_icmp_echo *in = irp->AssociatedIrp.SystemBuffer; struct icmp_listen_params params; + ULONG family = in->dst.si_family; + ULONGLONG addr = in->addr; + ULONG bits = in->bits;
Why do "family", "bits" and "addr" need separate variables?
Do you mean I should use them directly from the in-> buffer, or something else? If it's that, I wasn't exactly sure how the buffer worked, I see the output is on the same buffer and overrides it, so I wanted to be cautious. Thanks, Gabriel
participants (2)
-
Gabriel Ivăncescu -
Huw Davies