November 2021 - Wine-Devel - list.winehq.org

Re: [PATCH v3 4/4] wined3d: Try to allocate new Vulkan BOs from the client thread for DISCARD maps.

by Zebediah Figura

On 11/5/21 12:39 PM, Henri Verbeet wrote: > On Fri, 5 Nov 2021 at 04:52, Zebediah Figura <zfigura(a)codeweavers.com> wrote: >> +struct wined3d_client_bo_vk_map_ctx >> +{ >> + struct wined3d_device *device; >> + struct wined3d_client_bo_vk *client_bo; >> +}; >> + > We don't need this anymore. > >> +static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource, >> + unsigned int sub_resource_idx, struct wined3d_bo_address *addr) >> +{ >> + struct wined3d_device_vk *device_vk = wined3d_device_vk(device); >> + struct wined3d_context_vk *context_vk = &device_vk->context_vk; >> + >> + wined3d_not_from_cs(device->cs); >> + assert(device->context_count); >> + >> + if (resource->type == WINED3D_RTYPE_BUFFER) >> + { >> + struct wined3d_bo_vk *bo_vk; >> + >> + if (!(bo_vk = heap_alloc(sizeof(*bo_vk)))) >> + return false; >> + >> + if (!(wined3d_context_vk_create_bo(context_vk, resource->size, >> + vk_buffer_usage_from_bind_flags(resource->bind_flags), >> + vk_memory_type_from_access_flags(resource->access, resource->usage), bo_vk))) >> + { >> + WARN("Failed to create Vulkan buffer.\n"); >> + return false; >> + } > We leak "bo_vk" here. > >> + if (!bo_vk->b.map_ptr) >> + { >> + WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", >> + bo_vk, bo_vk->memory ? bo_vk->memory->chunk : NULL, bo_vk->slab); >> + >> + if (!wined3d_bo_vk_map(bo_vk, context_vk)) >> + ERR("Failed to map bo.\n"); >> + } >> + > I suppose we could clean up and return "false" if mapping the bo fails > here. On the other hand, I suppose it shouldn't fail in the first > place. Cleaning up is a bit nontrivial; I'd rather leave it as is (or use an assert?) > >> +void wined3d_buffer_set_bo(struct wined3d_buffer *buffer, struct wined3d_context *context, struct wined3d_bo *bo) >> +{ >> + TRACE("buffer %p, context %p, bo %p.\n", buffer, context, bo); >> + >> + buffer->buffer_ops->buffer_set_bo(buffer, context, bo); >> + buffer->buffer_object = (uintptr_t)bo; >> + wined3d_buffer_validate_location(buffer, WINED3D_LOCATION_BUFFER); >> + wined3d_buffer_invalidate_location(buffer, ~WINED3D_LOCATION_BUFFER); >> +} >> + > Is wined3d_buffer_set_bo() the proper place for the > validate/invalidate? Arguably it would be more appropriate to do that > in wined3d_cs_exec_update_sub_resource(). I suppose it mirrors > wined3d_buffer_copy_bo_address(). It definitely seems that most validation logic is done within buffer.c, and that makes more sense to me, along the lines of letting that be a buffer-specific implementation detail. I can change it if you feel strongly otherwise, though. > >> +static void wined3d_buffer_vk_set_bo(struct wined3d_buffer *buffer, >> + struct wined3d_context *context, struct wined3d_bo *bo) >> +{ >> + struct wined3d_bo_vk *prev_bo = (struct wined3d_bo_vk *)buffer->buffer_object; >> + struct wined3d_context_vk *context_vk = wined3d_context_vk(context); >> + struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer); >> + struct wined3d_bo_vk *bo_vk = wined3d_bo_vk(bo); >> + >> + /* We can't just copy the contents of bo_vk into buffer_vk->bo, because the >> + * new BO might still be in use by the client thread. We could allow both to >> + * be valid, although knowing when to destroy a BO then becomes tricky, and >> + * ensuring it's not mapped more than once also becomes tricky. */ >> + > There's probably less of a need for the comment now. > >> + if (prev_bo) >> + { >> + struct wined3d_bo_user *bo_user; >> + >> + LIST_FOR_EACH_ENTRY(bo_user, &prev_bo->b.users, struct wined3d_bo_user, entry) >> + bo_user->valid = false; >> + assert(list_empty(&bo_vk->b.users)); >> + list_move_head(&bo_vk->b.users, &prev_bo->b.users); >> + >> + wined3d_context_vk_destroy_bo(context_vk, prev_bo); >> + heap_free(prev_bo); >> + } >> + else >> + { >> + list_add_head(&bo_vk->b.users, &buffer_vk->b.bo_user.entry); >> + } >> +} >> + > Note that aside from the wined3d_context_vk_destroy_bo() call, this is > all just generic code now. > Yes, which is why I wanted to turn destroy_bo() into an adapter op earlier. I guess if we had a wined3d_texture_set_bo() it would also be all generic code except for a destroy_bo() call, so maybe it does make sense to make destroy_bo() an adapter op, if not per se for unload_location() as well.

Nov. 8, 2021

Re: [PATCH 2/5] win32u: Move NtUserSetProp implementation from user32.

by Zebediah Figura

Any particular reason that 1/5 uses a specfile import, but none of the rest of these functions do?

Nov. 8, 2021

Re: [PATCH 2/2] winegstreamer: Allow some audio resampling in the default pipeline.

by Rémi Bernon

On 10/27/21 22:34, Zebediah Figura (she/her) wrote: > On 10/27/21 10:25, Rémi Bernon wrote: >> Planet Coaster requests an output format with 44100 rate for user >> provided music, which may not match what the files are decoded to. >> >> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> > > This one looks good, although it'd be nice for this description to be in > the code itself. > > It could also be split into front and backend parts. > So it ends up being a little bit more complicated than that, and I don't think hardcoding a list of supported audio format is the right way to do it. In order to support all possible user music formats, we would have to hardcode all possible variations of media types. As far as I could see (and with tests from https://source.winehq.org/patches/data/219024) native streams only enumerate their native media types. Then, more media types are supported by the IMFSourceReader, but probably using a dynamically allocated decoder / converter MF transforms, or using MF topology elements which I believe are doing more advanced logic than what src_reader_SetCurrentMediaType does. Doing it this way would mean instantiating an audio_converter MF transform for instance, and would then make the audioresampler element useless. This seems to be the direction existing code is generally going, but it kind of defeat the idea of using GStreamer and its dynamically created pipelines. Another way I can see to make this dynamic, using an always present audioresampler element, would be instead to change the way we match IMFSourceReader stream media types, by delegating the type matching to winegstreamer, and GStreamer through gst_pad_query_caps calls. I'm not sure how we can do that, maybe with a custom IMFMediaTypeHandler, but it's not supposed to do this. -- Rémi Bernon <rbernon(a)codeweavers.com>

Nov. 8, 2021

Re: [PATCH 2/4] ddraw/tests: Port test_filling_convention to ddraw.

by Stefan Dösinger

> Am 08.11.2021 um 18:01 schrieb Marvin <testbot(a)winehq.org>: > > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 2x2, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 3x2, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 2x3, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 3x3, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 2x4, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 3x4, case 0. > ddraw1.c:14874: Test failed: Got unexpected colour 000000ff, 2x5, case 0. This looks like a genuine problem with the patch. Please skip it for now

Nov. 8, 2021

[PATCH v6 5/5] iphlpapi: Implement async requests for IcmpSendEcho2Ex.

by Gabriel Ivăncescu

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 8a31241..15c4f06 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

Nov. 8, 2021

[PATCH v6 4/5] nsiproxy: Fill the reply with the exact ICMP_ECHO_REPLY in the output buffer.

by Gabriel Ivăncescu

Because iphlpapi has no opportunity to convert the reply in async mode. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/iphlpapi/iphlpapi_main.c | 44 ++----------- dlls/nsiproxy.sys/device.c | 12 +++- dlls/nsiproxy.sys/icmp_echo.c | 93 ++++++++++++++++++---------- dlls/nsiproxy.sys/nsiproxy_private.h | 42 ++++++++++++- include/wine/nsi.h | 21 +------ 5 files changed, 120 insertions(+), 92 deletions(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index d9a85be..8a31241 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->user_reply_ptr = (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 dd08282..9e0a1d1 100644 --- a/dlls/nsiproxy.sys/device.c +++ b/dlls/nsiproxy.sys/device.c @@ -211,6 +211,13 @@ static void WINAPI icmp_echo_cancel( DEVICE_OBJECT *device, IRP *irp ) LeaveCriticalSection( &nsiproxy_cs ); } +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 +229,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) @@ -326,8 +333,10 @@ static DWORD WINAPI listen_thread_proc( void *arg ) TRACE( "\n" ); + params.user_reply_ptr = in->user_reply_ptr; params.handle = irp_get_icmp_handle( irp ); params.timeout = in->timeout; + params.bits = in->bits; params.reply = irp->AssociatedIrp.SystemBuffer; params.reply_len = irpsp->Parameters.DeviceIoControl.OutputBufferLength; @@ -360,6 +369,7 @@ static void handle_queued_send_echo( IRP *irp ) params.request = in->data + ((in->opt_size + 3) & ~3); params.request_size = in->req_size; params.reply = irp->AssociatedIrp.SystemBuffer; + params.bits = in->bits; params.ttl = in->ttl; params.tos = in->tos; params.dst = &in->dst; diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c index 4807569..07d76f0 100644 --- a/dlls/nsiproxy.sys/icmp_echo.c +++ b/dlls/nsiproxy.sys/icmp_echo.c @@ -224,12 +224,22 @@ static unsigned short null_chksum( BYTE *data, unsigned int count ) } #endif -static int ipv4_set_reply_ip_status( IP_STATUS ip_status, void *out ) +static int ipv4_set_reply_ip_status( IP_STATUS ip_status, unsigned int bits, void *out ) { - struct nsiproxy_icmp_echo_reply *reply = out; - memset( reply, 0, sizeof(*reply) ); - reply->status = ip_status; - return sizeof(*reply); + if (bits == 32) + { + struct icmp_echo_reply_32 *reply = out; + memset( reply, 0, sizeof(*reply) ); + reply->status = ip_status; + return sizeof(*reply); + } + else + { + struct icmp_echo_reply_64 *reply = out; + memset( reply, 0, sizeof(*reply) ); + reply->status = ip_status; + return sizeof(*reply); + } } static void ipv4_set_socket_opts( struct icmp_data *data, struct icmp_send_echo_params *params ) @@ -254,15 +264,17 @@ static void ipv4_linux_ping_set_socket_opts( struct icmp_data *data, struct icmp } #endif -static int ipv4_reply_buffer_len( int reply_len ) +static int ipv4_reply_buffer_len( struct icmp_listen_params *params ) { - return sizeof(struct ip_hdr) + sizeof(struct icmp_hdr) + reply_len - sizeof(struct nsiproxy_icmp_echo_reply); + 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( int reply_len ) +static int ipv4_linux_ping_reply_buffer_len( struct icmp_listen_params *params ) { - return sizeof(struct icmp_hdr) + reply_len - sizeof(struct nsiproxy_icmp_echo_reply); + 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 @@ -411,23 +423,42 @@ static int ipv4_linux_ping_parse_icmp_hdr( struct icmp_data *data, struct icmp_h static void ipv4_fill_reply( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx) { - struct nsiproxy_icmp_echo_reply *reply = params->reply; void *options_data; ULONG data_offset; - - data_offset = sizeof(*reply) + ((ctx->options_size + 3) & ~3); - reply->addr = ctx->addr; - reply->status = ctx->status; - reply->round_trip_time = ctx->round_trip_time; - reply->data_size = ctx->data_size; - reply->num_of_pkts = 1; - reply->data_offset = data_offset; - reply->opts.ttl = ctx->ttl; - reply->opts.tos = ctx->tos; - reply->opts.flags = ctx->flags; - reply->opts.options_size = ctx->options_size; - reply->opts.options_offset = sizeof(*reply); - options_data = reply + 1; + if (params->bits == 32) + { + struct icmp_echo_reply_32 *reply = params->reply; + data_offset = sizeof(*reply) + ((ctx->options_size + 3) & ~3); + reply->addr = ctx->addr.Ipv4.sin_addr.WS_s_addr; + reply->status = ctx->status; + reply->round_trip_time = ctx->round_trip_time; + reply->data_size = ctx->data_size; + reply->num_of_pkts = 1; + reply->data_ptr = params->user_reply_ptr + data_offset; + reply->opts.ttl = ctx->ttl; + reply->opts.tos = ctx->tos; + reply->opts.flags = ctx->flags; + reply->opts.options_size = ctx->options_size; + reply->opts.options_ptr = params->user_reply_ptr + sizeof(*reply); + options_data = reply + 1; + } + else + { + struct icmp_echo_reply_64 *reply = params->reply; + data_offset = sizeof(*reply) + ((ctx->options_size + 3) & ~3); + reply->addr = ctx->addr.Ipv4.sin_addr.WS_s_addr; + reply->status = ctx->status; + reply->round_trip_time = ctx->round_trip_time; + reply->data_size = ctx->data_size; + reply->num_of_pkts = 1; + reply->data_ptr = params->user_reply_ptr + data_offset; + reply->opts.ttl = ctx->ttl; + reply->opts.tos = ctx->tos; + reply->opts.flags = ctx->flags; + reply->opts.options_size = ctx->options_size; + reply->opts.options_ptr = params->user_reply_ptr + sizeof(*reply); + options_data = reply + 1; + } memcpy( options_data, ctx->options_data, ctx->options_size ); if (ctx->options_size & 3) @@ -443,9 +474,9 @@ struct family_ops int icmp_protocol; void (*init_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp_hdr ); unsigned short (*chksum)( BYTE *data, unsigned int count ); - int (*set_reply_ip_status)( IP_STATUS ip_status, void *out ); + 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 ); - int (*reply_buffer_len)( int reply_len ); + 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 ); void (*fill_reply)( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx ); @@ -629,7 +660,7 @@ NTSTATUS icmp_send_echo( void *args ) { TRACE( "sendto() rets %d errno %d\n", ret, errno ); icmp_data_free( data ); - params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->reply ); + params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply ); return STATUS_SUCCESS; } @@ -670,7 +701,7 @@ 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_len ); + reply_buf_len = data->ops->reply_buffer_len( params ); reply_buf = malloc( reply_buf_len ); if (!reply_buf) return STATUS_NO_MEMORY; @@ -689,7 +720,7 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *par 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->reply ); + params->reply_len = data->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->bits, params->reply ); return STATUS_SUCCESS; } @@ -739,11 +770,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->reply ); + params->reply_len = data->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->reply ); + params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->bits, params->reply ); return STATUS_SUCCESS; } diff --git a/dlls/nsiproxy.sys/nsiproxy_private.h b/dlls/nsiproxy.sys/nsiproxy_private.h index 8a9f2dd..bdea1c6 100644 --- a/dlls/nsiproxy.sys/nsiproxy_private.h +++ b/dlls/nsiproxy.sys/nsiproxy_private.h @@ -21,7 +21,8 @@ struct icmp_listen_params { HANDLE handle; void *reply; - unsigned int reply_len; + ULONGLONG user_reply_ptr; + unsigned int bits, reply_len; int timeout; }; @@ -30,6 +31,43 @@ struct icmp_send_echo_params SOCKADDR_INET *dst; void *request, *reply; DWORD request_size, reply_len; - BYTE ttl, tos; + BYTE bits, ttl, tos; HANDLE handle; }; + +/* output for IOCTL_NSIPROXY_WINE_ICMP_ECHO - cf. ICMP_ECHO_REPLY */ +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..8c3488d 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 user_reply_ptr; + 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

Nov. 8, 2021

[PATCH v6 3/5] nsiproxy: Use a context and fill the reply using a family op.

by Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/nsiproxy.sys/icmp_echo.c | 132 +++++++++++++++++++++------------- 1 file changed, 83 insertions(+), 49 deletions(-) diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c index a9bdb82..4807569 100644 --- a/dlls/nsiproxy.sys/icmp_echo.c +++ b/dlls/nsiproxy.sys/icmp_echo.c @@ -93,6 +93,20 @@ struct icmp_hdr } un; }; +struct icmp_reply_ctx +{ + SOCKADDR_INET addr; + ULONG status; + ULONG round_trip_time; + LONG data_size; + BYTE ttl; + BYTE tos; + BYTE flags; + BYTE options_size; + void *options_data; + void *data; +}; + struct family_ops; struct icmp_data { @@ -252,8 +266,8 @@ static int ipv4_linux_ping_reply_buffer_len( int reply_len ) } #endif -static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, - int *ip_hdr_len, struct nsiproxy_icmp_echo_reply *reply, void **opts ) +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; @@ -262,27 +276,27 @@ static BOOL ipv4_parse_ip_hdr( struct msghdr *msg, int recvd, 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; - *opts = ip_hdr + 1; - reply->opts.ttl = ip_hdr->ttl; - reply->opts.tos = ip_hdr->tos; - reply->opts.flags = ip_hdr->frag_off >> 13; - reply->opts.options_size = *ip_hdr_len - sizeof(*ip_hdr); + ctx->options_data = ip_hdr + 1; + ctx->ttl = ip_hdr->ttl; + ctx->tos = ip_hdr->tos; + ctx->flags = ip_hdr->frag_off >> 13; + ctx->options_size = *ip_hdr_len - sizeof(*ip_hdr); return TRUE; } #ifdef __linux__ -static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, - int *ip_hdr_len, struct nsiproxy_icmp_echo_reply *reply, void **opts ) +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; *ip_hdr_len = 0; - *opts = NULL; - reply->opts.ttl = 0; - reply->opts.tos = 0; - reply->opts.flags = 0; - reply->opts.options_size = 0; /* FIXME from IP_OPTIONS but will require checking for space in the reply */ + ctx->options_data = NULL; + ctx->ttl = 0; + ctx->tos = 0; + ctx->flags = 0; + ctx->options_size = 0; /* FIXME from IP_OPTIONS but will require checking for space in the reply */ for (cmsg = CMSG_FIRSTHDR( msg ); cmsg; cmsg = CMSG_NXTHDR( msg, cmsg )) { @@ -290,10 +304,10 @@ static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, switch (cmsg->cmsg_type) { case IP_TTL: - reply->opts.ttl = *(BYTE *)CMSG_DATA( cmsg ); + ctx->ttl = *(BYTE *)CMSG_DATA( cmsg ); break; case IP_TOS: - reply->opts.tos = *(BYTE *)CMSG_DATA( cmsg ); + ctx->tos = *(BYTE *)CMSG_DATA( cmsg ); break; } } @@ -302,7 +316,7 @@ static BOOL ipv4_linux_ping_parse_ip_hdr( struct msghdr *msg, int recvd, #endif static int ipv4_parse_icmp_hdr_( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_size, - struct nsiproxy_icmp_echo_reply *reply, int ping_socket ) + struct icmp_reply_ctx *ctx, int ping_socket ) { static const IP_STATUS unreach_codes[] = { @@ -334,7 +348,7 @@ static int ipv4_parse_icmp_hdr_( struct icmp_data *data, struct icmp_hdr *icmp, if ((!ping_socket && icmp->un.echo.id != data->id) || icmp->un.echo.sequence != data->seq) return -1; - reply->status = IP_SUCCESS; + ctx->status = IP_SUCCESS; return icmp_size - sizeof(*icmp); case ICMP4_DST_UNREACH: @@ -377,24 +391,52 @@ static int ipv4_parse_icmp_hdr_( struct icmp_data *data, struct icmp_hdr *icmp, (!ping_socket && orig_icmp_hdr->un.echo.id != data->id) || orig_icmp_hdr->un.echo.sequence != data->seq) return -1; - reply->status = status; + ctx->status = status; return 0; } -static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_size, - struct nsiproxy_icmp_echo_reply *reply ) +static int ipv4_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, + int icmp_size, struct icmp_reply_ctx *ctx) { - return ipv4_parse_icmp_hdr_( data, icmp, icmp_size, reply, 0 ); + return ipv4_parse_icmp_hdr_( data, icmp, icmp_size, ctx, 0 ); } #ifdef __linux__ -static int ipv4_linux_ping_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_size, - struct nsiproxy_icmp_echo_reply *reply ) +static int ipv4_linux_ping_parse_icmp_hdr( struct icmp_data *data, struct icmp_hdr *icmp, + int icmp_size, struct icmp_reply_ctx *ctx ) { - return ipv4_parse_icmp_hdr_( data, icmp, icmp_size, reply, 1 ); + return ipv4_parse_icmp_hdr_( data, icmp, icmp_size, ctx, 1 ); } #endif +static void ipv4_fill_reply( struct icmp_listen_params *params, struct icmp_reply_ctx *ctx) +{ + struct nsiproxy_icmp_echo_reply *reply = params->reply; + void *options_data; + ULONG data_offset; + + data_offset = sizeof(*reply) + ((ctx->options_size + 3) & ~3); + reply->addr = ctx->addr; + reply->status = ctx->status; + reply->round_trip_time = ctx->round_trip_time; + reply->data_size = ctx->data_size; + reply->num_of_pkts = 1; + reply->data_offset = data_offset; + reply->opts.ttl = ctx->ttl; + reply->opts.tos = ctx->tos; + reply->opts.flags = ctx->flags; + reply->opts.options_size = ctx->options_size; + reply->opts.options_offset = sizeof(*reply); + options_data = reply + 1; + + memcpy( options_data, ctx->options_data, 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 ); + params->reply_len = data_offset + ctx->data_size; +} + struct family_ops { int family; @@ -404,10 +446,9 @@ struct family_ops int (*set_reply_ip_status)( IP_STATUS ip_status, void *out ); void (*set_socket_opts)( struct icmp_data *data, struct icmp_send_echo_params *params ); int (*reply_buffer_len)( int reply_len ); - BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, - int *ip_hdr_len, struct nsiproxy_icmp_echo_reply *reply, void **opts ); - int (*parse_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp, int icmp_len, - struct nsiproxy_icmp_echo_reply *reply ); + 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 ); }; static const struct family_ops ipv4 = @@ -421,6 +462,7 @@ static const struct family_ops ipv4 = ipv4_reply_buffer_len, ipv4_parse_ip_hdr, ipv4_parse_icmp_hdr, + ipv4_fill_reply, }; #ifdef __linux__ @@ -436,6 +478,7 @@ static const struct family_ops ipv4_linux_ping = ipv4_linux_ping_reply_buffer_len, ipv4_linux_ping_parse_ip_hdr, ipv4_linux_ping_parse_icmp_hdr, + ipv4_fill_reply, }; #endif @@ -616,16 +659,15 @@ static ULONG get_rtt( LARGE_INTEGER start ) static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *params ) { - struct nsiproxy_icmp_echo_reply *reply = (struct nsiproxy_icmp_echo_reply *)params->reply; 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, reply_buf_len, data_size; + int ip_hdr_len, recvd, reply_buf_len; char *reply_buf; - void *opts; struct icmp_hdr *icmp_hdr; reply_buf_len = data->ops->reply_buffer_len( params->reply_len ); @@ -639,32 +681,24 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *par 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 (!data->ops->parse_ip_hdr( &msg, recvd, &ip_hdr_len, reply, &opts )) goto skip; + if (!data->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 ((data_size = data->ops->parse_icmp_hdr( data, icmp_hdr, recvd - ip_hdr_len, reply )) < 0) goto skip; - reply->data_size = data_size; - if (reply->data_size && msg.msg_flags & MSG_TRUNC) + if ((ctx.data_size = data->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->reply ); return STATUS_SUCCESS; } - sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &reply->addr ); - reply->round_trip_time = get_rtt( data->send_time ); - reply->num_of_pkts = 1; - reply->opts.options_offset = sizeof(*reply); - reply->data_offset = sizeof(*reply) + ((reply->opts.options_size + 3) & ~3); - if (reply->opts.options_size) - memcpy( (char *)reply + reply->opts.options_offset, opts, reply->opts.options_size ); - if (reply->opts.options_size & 3) - memset( (char *)reply + reply->opts.options_offset + reply->opts.options_size, 0, 4 - (reply->opts.options_size & 3) ); - if (reply->data_size) - memcpy( (char *)reply + reply->data_offset, icmp_hdr + 1, reply->data_size ); - - params->reply_len = reply->data_offset + reply->data_size; + sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &ctx.addr ); + ctx.round_trip_time = get_rtt( data->send_time ); + ctx.data = icmp_hdr + 1; + + data->ops->fill_reply( params, &ctx ); + free( reply_buf ); return STATUS_SUCCESS; -- 2.31.1

Nov. 8, 2021

[PATCH v6 2/5] nsiproxy: Fill the reply when it's not pending on the unix-side.

by Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/nsiproxy.sys/device.c | 8 ++------ dlls/nsiproxy.sys/icmp_echo.c | 2 +- dlls/nsiproxy.sys/nsiproxy_private.h | 5 ++--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/dlls/nsiproxy.sys/device.c b/dlls/nsiproxy.sys/device.c index 90a7a3d..dd08282 100644 --- a/dlls/nsiproxy.sys/device.c +++ b/dlls/nsiproxy.sys/device.c @@ -353,13 +353,13 @@ 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; struct icmp_send_echo_params params; NTSTATUS status; TRACE( "\n" ); params.request = in->data + ((in->opt_size + 3) & ~3); params.request_size = in->req_size; + params.reply = irp->AssociatedIrp.SystemBuffer; params.ttl = in->ttl; params.tos = in->tos; params.dst = &in->dst; @@ -371,11 +371,7 @@ 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); - } + irp->IoStatus.Information = params.reply_len; IoCompleteRequest( irp, IO_NO_INCREMENT ); } else diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c index a4ef02a..a9bdb82 100644 --- a/dlls/nsiproxy.sys/icmp_echo.c +++ b/dlls/nsiproxy.sys/icmp_echo.c @@ -586,7 +586,7 @@ NTSTATUS icmp_send_echo( void *args ) { TRACE( "sendto() rets %d errno %d\n", ret, errno ); icmp_data_free( data ); - params->ip_status = errno_to_ip_status( errno ); + params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->reply ); return STATUS_SUCCESS; } diff --git a/dlls/nsiproxy.sys/nsiproxy_private.h b/dlls/nsiproxy.sys/nsiproxy_private.h index 91dd393..8a9f2dd 100644 --- a/dlls/nsiproxy.sys/nsiproxy_private.h +++ b/dlls/nsiproxy.sys/nsiproxy_private.h @@ -28,9 +28,8 @@ struct icmp_listen_params struct icmp_send_echo_params { SOCKADDR_INET *dst; - void *request; - DWORD request_size; + void *request, *reply; + DWORD request_size, reply_len; BYTE ttl, tos; HANDLE handle; - ULONG ip_status; }; -- 2.31.1

Nov. 8, 2021

[PATCH v6 1/5] nsiproxy: Move set_reply_ip_status to a family op.

by Gabriel Ivăncescu

Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- It is returning the reply_len because it will be used also for icmp_send_echo, which has different params struct. dlls/nsiproxy.sys/icmp_echo.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/dlls/nsiproxy.sys/icmp_echo.c b/dlls/nsiproxy.sys/icmp_echo.c index 955fdfd..a4ef02a 100644 --- a/dlls/nsiproxy.sys/icmp_echo.c +++ b/dlls/nsiproxy.sys/icmp_echo.c @@ -210,6 +210,14 @@ static unsigned short null_chksum( BYTE *data, unsigned int count ) } #endif +static int ipv4_set_reply_ip_status( IP_STATUS ip_status, void *out ) +{ + struct nsiproxy_icmp_echo_reply *reply = out; + memset( reply, 0, sizeof(*reply) ); + reply->status = ip_status; + return sizeof(*reply); +} + static void ipv4_set_socket_opts( struct icmp_data *data, struct icmp_send_echo_params *params ) { int val; @@ -393,6 +401,7 @@ struct family_ops int icmp_protocol; void (*init_icmp_hdr)( struct icmp_data *data, struct icmp_hdr *icmp_hdr ); unsigned short (*chksum)( BYTE *data, unsigned int count ); + int (*set_reply_ip_status)( IP_STATUS ip_status, void *out ); void (*set_socket_opts)( struct icmp_data *data, struct icmp_send_echo_params *params ); int (*reply_buffer_len)( int reply_len ); BOOL (*parse_ip_hdr)( struct msghdr *msg, int recvd, @@ -407,6 +416,7 @@ static const struct family_ops ipv4 = IPPROTO_ICMP, ipv4_init_icmp_hdr, chksum, + ipv4_set_reply_ip_status, ipv4_set_socket_opts, ipv4_reply_buffer_len, ipv4_parse_ip_hdr, @@ -421,6 +431,7 @@ static const struct family_ops ipv4_linux_ping = IPPROTO_ICMP, ipv4_init_icmp_hdr, null_chksum, + ipv4_set_reply_ip_status, ipv4_linux_ping_set_socket_opts, ipv4_linux_ping_reply_buffer_len, ipv4_linux_ping_parse_ip_hdr, @@ -584,16 +595,6 @@ NTSTATUS icmp_send_echo( void *args ) return params->handle ? STATUS_PENDING : STATUS_NO_MEMORY; } -static NTSTATUS set_reply_ip_status( struct icmp_listen_params *params, IP_STATUS ip_status ) -{ - struct nsiproxy_icmp_echo_reply *reply = params->reply; - - memset( reply, 0, sizeof(*reply) ); - reply->status = ip_status; - params->reply_len = sizeof(*reply); - return STATUS_SUCCESS; -} - static int get_timeout( LARGE_INTEGER start, DWORD timeout ) { LARGE_INTEGER now, end; @@ -647,7 +648,8 @@ static NTSTATUS recv_msg( struct icmp_data *data, struct icmp_listen_params *par if (reply->data_size && msg.msg_flags & MSG_TRUNC) { free( reply_buf ); - return set_reply_ip_status( params, IP_GENERAL_FAILURE ); + params->reply_len = data->ops->set_reply_ip_status( IP_GENERAL_FAILURE, params->reply ); + return STATUS_SUCCESS; } sockaddr_to_SOCKADDR_INET( (struct sockaddr *)&addr, &reply->addr ); @@ -703,10 +705,12 @@ NTSTATUS icmp_listen( void *args ) if (!ret) /* timeout */ { TRACE( "timeout\n" ); - return set_reply_ip_status( params, IP_REQ_TIMED_OUT ); + params->reply_len = data->ops->set_reply_ip_status( IP_REQ_TIMED_OUT, params->reply ); + return STATUS_SUCCESS; } /* ret < 0 */ - return set_reply_ip_status( params, errno_to_ip_status( errno ) ); + params->reply_len = data->ops->set_reply_ip_status( errno_to_ip_status( errno ), params->reply ); + return STATUS_SUCCESS; } NTSTATUS icmp_cancel_listen( void *args ) -- 2.31.1

Nov. 8, 2021

Re: [PATCH] msvcrt: Better handle multiple width specifiers

by Piotr Caban

Signed-off-by: Piotr Caban <piotr(a)codeweavers.com>

Nov. 8, 2021