[PATCH v3 0/2] MR6548: nsiproxy: Only set the connection count from tcp_conns_enumerate_all/udp_endpoint_enumerate_all when appropriate.
Fixes heap corruption downstream in NsiEnumerateObjectsAllParametersEx. Fixes regressions from 9085bc7b87f and a3f737f6143. --- I missed this piece of logic in MR !6399. The original line that implemented it is this: https://gitlab.winehq.org/wine/wine/-/blob/1d05064e747f9998fd705345659030e60... -- v3: nsiproxy: Only set the endpoint count from udp_endpoint_enumerate_all when appropriate. nsiproxy: Only set the connection count from tcp_conns_enumerate_all when appropriate. https://gitlab.winehq.org/wine/wine/-/merge_requests/6548
From: Tim Clem <tclem(a)codeweavers.com> Fixes heap corruption downstream in NsiEnumerateObjectsAllParametersEx. Fixes a regression from 9085bc7b87f. --- dlls/nsiproxy.sys/tcp.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dlls/nsiproxy.sys/tcp.c b/dlls/nsiproxy.sys/tcp.c index 7a3004b1a97..e472d8600a2 100644 --- a/dlls/nsiproxy.sys/tcp.c +++ b/dlls/nsiproxy.sys/tcp.c @@ -322,13 +322,17 @@ static NTSTATUS tcp_conns_enumerate_all( UINT filter, struct nsi_tcp_conn_key *k *count = reply->count; else if (ret == STATUS_BUFFER_TOO_SMALL) { - *count = reply->count; - if (want_data) + if (!want_data) { - free( connections ); - return STATUS_BUFFER_OVERFLOW; + /* If we were given buffers, the outgoing count must never be + greater than the incoming one. If we weren't, the count + should be set to the actual count. */ + *count = reply->count; + return STATUS_SUCCESS; } - return STATUS_SUCCESS; + + free( connections ); + return STATUS_BUFFER_OVERFLOW; } } SERVER_END_REQ; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6548
From: Tim Clem <tclem(a)codeweavers.com> Fixes heap corruption downstream in NsiEnumerateObjectsAllParametersEx. Fixes a regression from a3f737f6143. --- dlls/nsiproxy.sys/udp.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dlls/nsiproxy.sys/udp.c b/dlls/nsiproxy.sys/udp.c index 1d790484d8f..897c90c131b 100644 --- a/dlls/nsiproxy.sys/udp.c +++ b/dlls/nsiproxy.sys/udp.c @@ -231,13 +231,17 @@ static NTSTATUS udp_endpoint_enumerate_all( void *key_data, UINT key_size, void *count = reply->count; else if (ret == STATUS_BUFFER_TOO_SMALL) { - *count = reply->count; - if (want_data) + if (!want_data) { - free( endpoints ); - return STATUS_BUFFER_OVERFLOW; + /* If we were given buffers, the outgoing count must never be + greater than the incoming one. If we weren't, the count + should be set to the actual count. */ + *count = reply->count; + return STATUS_SUCCESS; } - else return STATUS_SUCCESS; + + free( endpoints ); + return STATUS_BUFFER_OVERFLOW; } } SERVER_END_REQ; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6548
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148591 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000001CA00E8, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6548
participants (4)
-
Huw Davies (＠huw) -
Marvin -
Tim Clem
-
Tim Clem (＠tclem)