[PATCH v2 0/1] MR4742: server: Allow PROCESS_QUERY_INFORMATION on LIMITED handle.

[Bernhard Übelacker]

A handle created with just PROCESS_QUERY_LIMITED_INFORMATION
should be returned when queried with PROCESS_QUERY_INFORMATION.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56093

--
  v2: server: Allow VirtualQueryEx on "limited" handle.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4742

[PATCH v2 1/1] server: Allow VirtualQueryEx on "limited" handle.

[Bernhard Übelacker]

From: Bernhard Übelacker <bernhardu(a)mailbox.org>

A handle created with just PROCESS_QUERY_LIMITED_INFORMATION
should allow VirtualQueryEx / APC_VIRTUAL_QUERY.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56093
---
 dlls/kernel32/tests/process.c | 27 +++++++++++++++++++++++++++
 server/thread.c               |  2 +-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c
index e0f1d210e58..4a98716741b 100644
--- a/dlls/kernel32/tests/process.c
+++ b/dlls/kernel32/tests/process.c
@@ -2545,6 +2545,32 @@ static void test_DuplicateHandle(void)
     CloseHandle(out);
 }
 
+static void test_DuplicateHandle_limited(void)
+{
+    void *addr;
+    HANDLE h;
+    MEMORY_BASIC_INFORMATION info;
+    int ret;
+
+    addr = VirtualAllocEx(GetCurrentProcess(), 0, 0xFFFC, MEM_RESERVE, PAGE_NOACCESS);
+    ok(addr != NULL, "VirtualAllocEx error %ld\n", GetLastError());
+
+    ret = DuplicateHandle(GetCurrentProcess(), GetCurrentProcess(), GetCurrentProcess(), &h,
+            SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_DUP_HANDLE | PROCESS_VM_READ |PROCESS_VM_OPERATION,
+            TRUE, 0);
+    ok(ret, "DuplicateHandle error %ld\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    ret = VirtualQueryEx(h, addr, &info, sizeof(info));
+    ok(ret == sizeof(info) ||
+       broken(ret == 0 && GetLastError() == ERROR_ACCESS_DENIED) /* win8 and below */,
+       "VirtualQueryEx error ret=0x%x %ld\n", ret, GetLastError());
+
+    ok(CloseHandle(h), "CloseHandle error %ld\n", GetLastError());
+
+    ok(VirtualFree(addr, 0, MEM_RELEASE), "VirtualFree error %ld\n", GetLastError());
+}
+
 #define test_completion(a, b, c, d, e) _test_completion(__LINE__, a, b, c, d, e)
 static void _test_completion(int line, HANDLE port, DWORD ekey, ULONG_PTR evalue, ULONG_PTR eoverlapped, DWORD wait)
 {
@@ -5458,6 +5484,7 @@ START_TEST(process)
     test_ProcessorCount();
     test_RegistryQuota();
     test_DuplicateHandle();
+    test_DuplicateHandle_limited();
     test_StdHandleInheritance();
     test_GetNumaProcessorNode();
     test_session_info();
diff --git a/server/thread.c b/server/thread.c
index 50eddabe8eb..56f57cefd8f 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -1734,7 +1734,7 @@ DECL_HANDLER(queue_apc)
         process = get_process_from_handle( req->handle, PROCESS_VM_OPERATION );
         break;
     case APC_VIRTUAL_QUERY:
-        process = get_process_from_handle( req->handle, PROCESS_QUERY_INFORMATION );
+        process = get_process_from_handle( req->handle, PROCESS_QUERY_LIMITED_INFORMATION );
         break;
     case APC_MAP_VIEW:
     case APC_MAP_VIEW_EX:
-- 
GitLab

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

[Bernhard Übelacker]

Thanks for the feedback.

If I see it right `duplicate_handle` would put both access on the handle, thanks to  `process_map_access` (PROCESS_QUERY_INFORMATION and PROCESS_QUERY_LIMITED_INFORMATION).

Therefore I modified the patch now to always use PROCESS_QUERY_LIMITED_INFORMATION in the APC_VIRTUAL_QUERY path. This should work for full and limited handles.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4742#note_56480

[eric pouech (＠epo)]

fix looks good to me

last nitpick: looking at existing tests, perhaps you'd better amend existing kernel32/tests/process.c:test_OpenProcess() instead of adding a brand new test function

the issue you're looking at is in fact hidden by somehow incorrect test.

something like this should be better (didn't test it though)

```plaintext
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c
index e0f1d210e58..0e5bf82530b 100644
--- a/dlls/kernel32/tests/process.c
+++ b/dlls/kernel32/tests/process.c
@@ -1852,7 +1852,7 @@ static void test_OpenProcess(void)
ok(info.Type == MEM_PRIVATE, "%lx != MEM_PRIVATE\n", info.Type);
}
else /* before win8 */
-            ok(GetLastError() == ERROR_ACCESS_DENIED, "wrong error %ld\n", GetLastError());
+            ok(broken(GetLastError() == ERROR_ACCESS_DENIED), "wrong error %ld\n", GetLastError());
SetLastError(0xdeadbeef);
ok(!VirtualFreeEx(hproc, addr1, 0, MEM_RELEASE),
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4742#note_56487

[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 full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=141512

Your paranoid android.


=== build (build log) ===

error: corrupt patch at line 15
Task: Patch failed to apply

=== debian11 (build log) ===

error: corrupt patch at line 15
Task: Patch failed to apply

=== debian11b (build log) ===

error: corrupt patch at line 15
Task: Patch failed to apply