[PATCH v2 0/1] MR7082: ntdll: Return an error if count is zero in NtRemoveIoCompletionEx.

[Alex Henrie (＠alexhenrie)]

Without this check, the return value is undefined.

--
  v2: ntdll: Return an error if count is zero in NtRemoveIoCompletionEx.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7082

[PATCH v2 1/1] ntdll: Return an error if count is zero in NtRemoveIoCompletionEx.

[Alex Henrie]

From: Alex Henrie <alexhenrie24(a)gmail.com>

Without this check, the return value is undefined.
---
 dlls/ntdll/tests/file.c | 5 +++++
 dlls/ntdll/unix/sync.c  | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
index 02bdf67796d..8465ad4543a 100644
--- a/dlls/ntdll/tests/file.c
+++ b/dlls/ntdll/tests/file.c
@@ -1039,6 +1039,11 @@ static void test_set_io_completion(void)
         return;
     }
 
+    count = 0xdeadbeef;
+    res = pNtRemoveIoCompletionEx( h, info, 0, &count, &timeout, FALSE );
+    ok( res == STATUS_INVALID_PARAMETER, "NtRemoveIoCompletionEx failed: %#lx\n", res );
+    ok( count == 0xdeadbeef, "wrong count %lu\n", count );
+
     count = 0xdeadbeef;
     res = pNtRemoveIoCompletionEx( h, info, 2, &count, &timeout, FALSE );
     ok( res == STATUS_TIMEOUT, "NtRemoveIoCompletionEx failed: %#lx\n", res );
diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c
index d486b50001d..bc2aa0271a1 100644
--- a/dlls/ntdll/unix/sync.c
+++ b/dlls/ntdll/unix/sync.c
@@ -2081,6 +2081,8 @@ NTSTATUS WINAPI NtRemoveIoCompletionEx( HANDLE handle, FILE_IO_COMPLETION_INFORM
 
     TRACE( "%p %p %u %p %p %u\n", handle, info, (int)count, written, timeout, alertable );
 
+    if (!count) return STATUS_INVALID_PARAMETER;
+
     while (i < count)
     {
         SERVER_START_REQ( remove_completion )
-- 
GitLab

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

[PATCH v2 1/1] ntdll: Return an error if count is zero in NtRemoveIoCompletionEx.

[Marvin]

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=150647

Your paranoid android.


=== debian11b (64 bit WoW report) ===

Report validation errors:
shell32:shelllink crashed (c0000005)

[Alex Henrie (＠alexhenrie)]

On Wed Jan  1 03:36:01 2025 +0000, Jinoh Kang wrote:
> Please test if and how `count` variable is overwritten.
> Specifically, set `count = 0xdeadbeef;` beforehand and then check how
> the value changes.
> Done. The value does not change.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7082#note_91205

[PATCH v2 0/1] MR7082: ntdll: Return an error if count is zero in NtRemoveIoCompletionEx. - approved

[Jinoh Kang (＠iamahuman)]

This merge request was approved by Jinoh Kang.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7082

[Alexandre Julliard (＠julliard)]

The test fails in new wow64 mode.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7082#note_91461

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/unix/sync.c:
> TRACE( "%p %p %u %p %p %u\n", handle, info, (int)count, written, timeout, alertable );
>
> +    if (!count) return STATUS_INVALID_PARAMETER;

Would you implement the check at `wow64_NtRemoveIoCompletionEx` side as well?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7082#note_91464