[PATCH 1/5] ntoskrnl.exe: Implement KeAreApcsDisabled using critical region functions.
Signed-off-by: Derek Lesho <dereklesho52(a)Gmail.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 16 ++++++++++++++-- dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- dlls/ntoskrnl.exe/ntoskrnl_private.h | 1 + dlls/ntoskrnl.exe/tests/driver.c | 17 +++++++++++++++++ include/ddk/ntddk.h | 1 + 5 files changed, 34 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index c1e6a9cf02..76046c8d90 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2515,6 +2515,8 @@ static void *create_thread_object( HANDLE handle ) if (!NtQueryInformationThread( handle, ThreadBasicInformation, &info, sizeof(info), NULL )) thread->id = info.ClientId; + thread->critical_region_count = 0; + return thread; } @@ -3417,7 +3419,8 @@ NTSTATUS WINAPI IoCsqInitialize(PIO_CSQ csq, PIO_CSQ_INSERT_IRP insert_irp, PIO_ */ void WINAPI KeEnterCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count++; } /*********************************************************************** @@ -3425,7 +3428,8 @@ void WINAPI KeEnterCriticalRegion(void) */ void WINAPI KeLeaveCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count--; } /*********************************************************************** @@ -4357,3 +4361,11 @@ ULONG WINAPI ExSetTimerResolution(ULONG time, BOOLEAN set_resolution) FIXME("stub: %u %d\n", time, set_resolution); return KeQueryTimeIncrement(); } + +/********************************************************************* + * KeAreApcsDisabled (NTOSKRNL.@) + */ +BOOLEAN WINAPI KeAreApcsDisabled(void) +{ + return !!KeGetCurrentThread()->critical_region_count; +} diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 633a8c4b6c..97fc4ec0db 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -521,7 +521,7 @@ @ stdcall KeAcquireSpinLockAtDpcLevel(ptr) @ stdcall -arch=arm,arm64,x86_64 KeAcquireSpinLockRaiseToDpc(ptr) @ stub KeAddSystemServiceTable -@ stub KeAreApcsDisabled +@ stdcall KeAreApcsDisabled() @ stub KeAttachProcess @ stub KeBugCheck @ stub KeBugCheckEx diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h index 5f309616d3..4f5ae14cfb 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h @@ -36,6 +36,7 @@ struct _KTHREAD { DISPATCHER_HEADER header; CLIENT_ID id; + unsigned int critical_region_count; }; void *alloc_kernel_object( POBJECT_TYPE type, HANDLE handle, SIZE_T size, LONG ref ) DECLSPEC_HIDDEN; diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index 030c95507c..f70f865c40 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -1187,6 +1187,22 @@ static void test_lookup_thread(void) "PsLookupThreadByThreadId returned %#x\n", status); } +static void test_critical_regions(void) +{ + BOOLEAN result; + + result = KeAreApcsDisabled(); + ok(!result, "got %u, expected 0\n", result); + + KeEnterCriticalRegion(); + + result = KeAreApcsDisabled(); + + KeLeaveCriticalRegion(); + + ok(result, "got %u, expected 1\n", result); +} + static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *stack, ULONG_PTR *info) { ULONG length = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -1231,6 +1247,7 @@ static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *st test_ob_reference(test_input->path); test_resource(); test_lookup_thread(); + test_critical_regions(); /* print process report */ if (winetest_debug) diff --git a/include/ddk/ntddk.h b/include/ddk/ntddk.h index 719ba67c6a..f09f879032 100644 --- a/include/ddk/ntddk.h +++ b/include/ddk/ntddk.h @@ -213,6 +213,7 @@ NTSTATUS WINAPI IoQueryDeviceDescription(PINTERFACE_TYPE,PULONG,PCONFIGURATION_ PCONFIGURATION_TYPE,PULONG,PIO_QUERY_DEVICE_ROUTINE,PVOID); void WINAPI IoRegisterDriverReinitialization(PDRIVER_OBJECT,PDRIVER_REINITIALIZE,PVOID); NTSTATUS WINAPI IoRegisterShutdownNotification(PDEVICE_OBJECT); +BOOLEAN WINAPI KeAreApcsDisabled(void); NTSTATUS WINAPI KeExpandKernelStackAndCallout(PEXPAND_STACK_CALLOUT,void*,SIZE_T); void WINAPI KeSetTargetProcessorDpc(PRKDPC,CCHAR); BOOLEAN WINAPI MmIsAddressValid(void *); -- 2.20.1
Jacek says that duplicating and closing the sent handle in kernel_object_from_handle every time would add unecessary overhead. Signed-off-by: Derek Lesho <dereklesho52(a)Gmail.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 76046c8d90..533ac662d1 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2504,6 +2504,7 @@ NTSTATUS WINAPI PsLookupProcessByProcessId( HANDLE processid, PEPROCESS *process static void *create_thread_object( HANDLE handle ) { + NTSTATUS status; THREAD_BASIC_INFORMATION info; struct _KTHREAD *thread; @@ -2512,8 +2513,20 @@ static void *create_thread_object( HANDLE handle ) thread->header.Type = 6; thread->header.WaitListHead.Blink = INVALID_HANDLE_VALUE; /* mark as kernel object */ - if (!NtQueryInformationThread( handle, ThreadBasicInformation, &info, sizeof(info), NULL )) + if (!(status = NtQueryInformationThread( handle, ThreadBasicInformation, &info, sizeof(info), NULL ))) thread->id = info.ClientId; + else if (status == STATUS_ACCESS_DENIED) + { + HANDLE info_handle; + + DuplicateHandle( GetCurrentProcess(), handle, GetCurrentProcess(), + &info_handle, THREAD_QUERY_LIMITED_INFORMATION, FALSE, 0); + + if (!NtQueryInformationThread( handle, ThreadBasicInformation, &info, sizeof(info), NULL )) + thread->id = info.ClientId; + + NtClose( info_handle ); + } thread->critical_region_count = 0; -- 2.20.1
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=29460 Signed-off-by: Derek Lesho <dereklesho52(a)Gmail.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 6 ++++-- dlls/ntoskrnl.exe/ntoskrnl_private.h | 1 + dlls/ntoskrnl.exe/tests/driver.c | 9 ++++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 533ac662d1..711386c5ad 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -2478,8 +2478,7 @@ POBJECT_TYPE PsProcessType = &process_type; */ PEPROCESS WINAPI IoGetCurrentProcess(void) { - FIXME("() stub\n"); - return NULL; + return KeGetCurrentThread()->process; } /*********************************************************************** @@ -2528,6 +2527,9 @@ static void *create_thread_object( HANDLE handle ) NtClose( info_handle ); } + PsLookupProcessByProcessId( thread->id.UniqueProcess, &thread->process ); + ObDereferenceObject( thread->process ); + thread->critical_region_count = 0; return thread; diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h index 4f5ae14cfb..5c62e0d29c 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h @@ -37,6 +37,7 @@ struct _KTHREAD DISPATCHER_HEADER header; CLIENT_ID id; unsigned int critical_region_count; + PEPROCESS process; }; void *alloc_kernel_object( POBJECT_TYPE type, HANDLE handle, SIZE_T size, LONG ref ) DECLSPEC_HIDDEN; diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index f70f865c40..0410c32ccc 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -326,9 +326,16 @@ static void test_currentprocess(void) NTSTATUS ret; current = IoGetCurrentProcess(); -todo_wine ok(current != NULL, "Expected current process to be non-NULL\n"); + if (!!current) + { + DISPATCHER_HEADER *header = current; + ok(header->Type == 3, "header->Type != 3, = %u\n", header->Type); + ret = wait_single( current, 0 ); + ok(ret == STATUS_TIMEOUT, "got %#x\n", ret); + } + thread = PsGetCurrentThread(); ret = wait_single( thread, 0 ); ok(ret == STATUS_TIMEOUT, "got %#x\n", ret); -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=51450 Your paranoid android. === w8 (32 bit report) === ntoskrnl.exe: driver.c:1202: Test failed: got 1, expected 0 Report errors: ntoskrnl.exe:ntoskrnl contains a misplaced failure message for driver === debian9 (32 bit report) === ntoskrnl.exe: driver.c:659: Test failed: got 0 driver.c:662: Test failed: got 0x102 driver.c:671: Test failed: got 0 driver.c:674: Test failed: got 0x102 === debian9 (32 bit Japanese:Japan report) === ntoskrnl.exe: driver.c:645: Test failed: got 0 === debian9 (64 bit WoW report) === ntoskrnl.exe: driver.c:870: Test failed: obj1 != obj2
Signed-off-by: Derek Lesho <dereklesho52(a)Gmail.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 4 ++++ dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 711386c5ad..5abd9b5469 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -940,6 +940,8 @@ static void unload_driver( struct wine_rb_entry *entry, void *context ) CloseServiceHandle( (void *)service_handle ); } +PEPROCESS PsInitialSystemProcess = NULL; + /*********************************************************************** * wine_ntoskrnl_main_loop (Not a Windows API) */ @@ -953,6 +955,8 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ) void *in_buff = NULL; HANDLE handles[2]; + /* Set the system process global before setting up the request thread trickery */ + PsInitialSystemProcess = IoGetCurrentProcess(); request_thread = GetCurrentThreadId(); handles[0] = stop_event; diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index 97fc4ec0db..f8240c529a 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -906,7 +906,7 @@ @ stub PsGetThreadWin32Thread @ stdcall PsGetVersion(ptr ptr ptr ptr) @ stdcall PsImpersonateClient(ptr ptr long long long) -@ stub PsInitialSystemProcess +@ extern PsInitialSystemProcess @ stub PsIsProcessBeingDebugged @ stub PsIsSystemThread @ stub PsIsThreadImpersonating -- 2.20.1
Signed-off-by: Derek Lesho <dereklesho52(a)Gmail.com> --- dlls/ntoskrnl.exe/ntoskrnl.c | 9 +++++++++ dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 2 +- dlls/ntoskrnl.exe/tests/driver.c | 19 ++++++++++++++++++- include/ddk/ntifs.h | 1 + 4 files changed, 29 insertions(+), 2 deletions(-) diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c index 5abd9b5469..8842e9844b 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/ntoskrnl.c @@ -3066,6 +3066,15 @@ HANDLE WINAPI PsGetCurrentThreadId(void) } +/*********************************************************************** + * PsIsSystemThread (NTOSKRNL.EXE.@) + */ +BOOLEAN WINAPI PsIsSystemThread(PETHREAD thread) +{ + return ((PKTHREAD)thread)->process == PsInitialSystemProcess; +} + + /*********************************************************************** * PsGetVersion (NTOSKRNL.EXE.@) */ diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec index f8240c529a..09177a7d75 100644 --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec @@ -908,7 +908,7 @@ @ stdcall PsImpersonateClient(ptr ptr long long long) @ extern PsInitialSystemProcess @ stub PsIsProcessBeingDebugged -@ stub PsIsSystemThread +@ stdcall PsIsSystemThread(ptr) @ stub PsIsThreadImpersonating @ stub PsIsThreadTerminating @ stub PsJobType diff --git a/dlls/ntoskrnl.exe/tests/driver.c b/dlls/ntoskrnl.exe/tests/driver.c index 0410c32ccc..0758e043f1 100644 --- a/dlls/ntoskrnl.exe/tests/driver.c +++ b/dlls/ntoskrnl.exe/tests/driver.c @@ -330,7 +330,7 @@ static void test_currentprocess(void) if (!!current) { - DISPATCHER_HEADER *header = current; + DISPATCHER_HEADER *header = (DISPATCHER_HEADER *)current; ok(header->Type == 3, "header->Type != 3, = %u\n", header->Type); ret = wait_single( current, 0 ); ok(ret == STATUS_TIMEOUT, "got %#x\n", ret); @@ -1210,6 +1210,22 @@ static void test_critical_regions(void) ok(result, "got %u, expected 1\n", result); } +static void WINAPI system_thread( void *arg ) +{ + BOOLEAN result = PsIsSystemThread((PETHREAD)KeGetCurrentThread()); + ok((result), "got %u\n", result); + + PsTerminateSystemThread( STATUS_SUCCESS ); +} + +static void test_system_thread(void) +{ + BOOLEAN result = PsIsSystemThread((PETHREAD)KeGetCurrentThread()); + ok(!(result), "got %u\n", result); + + run_thread( system_thread, (void*)0 ); +} + static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *stack, ULONG_PTR *info) { ULONG length = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -1255,6 +1271,7 @@ static NTSTATUS main_test(DEVICE_OBJECT *device, IRP *irp, IO_STACK_LOCATION *st test_resource(); test_lookup_thread(); test_critical_regions(); + test_system_thread(); /* print process report */ if (winetest_debug) diff --git a/include/ddk/ntifs.h b/include/ddk/ntifs.h index ec4d1d5aa7..2c61329d9e 100644 --- a/include/ddk/ntifs.h +++ b/include/ddk/ntifs.h @@ -131,6 +131,7 @@ typedef struct _FS_FILTER_CALLBACKS BOOLEAN WINAPI FsRtlIsNameInExpression(PUNICODE_STRING, PUNICODE_STRING, BOOLEAN, PWCH); NTSTATUS WINAPI ObQueryNameString(PVOID,POBJECT_NAME_INFORMATION,ULONG,PULONG); +BOOLEAN WINAPI PsIsSystemThread(PETHREAD); NTSTATUS WINAPI PsLookupProcessByProcessId(HANDLE,PEPROCESS*); NTSTATUS WINAPI PsLookupThreadByThreadId(HANDLE,PETHREAD*); void WINAPI PsRevertToSelf(void); -- 2.20.1
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=51453 Your paranoid android. === w8 (32 bit report) === ntoskrnl.exe: driver.c:1202: Test failed: got 1, expected 0 Report errors: ntoskrnl.exe:ntoskrnl contains a misplaced failure message for driver === debian9 (32 bit Japanese:Japan report) === ntoskrnl.exe: driver.c:659: Test failed: got 0 driver.c:662: Test failed: got 0x102 driver.c:671: Test failed: got 0 driver.c:674: Test failed: got 0x102 === debian9 (32 bit Chinese:China report) === ntoskrnl.exe: driver.c:671: Test failed: got 0 driver.c:674: Test failed: got 0x102 ntoskrnl: Timeout === debian9 (64 bit WoW report) === ntoskrnl.exe: driver.c:870: Test failed: obj1 != obj2
Derek Lesho <dereklesho52(a)gmail.com> wrote:
void WINAPI KeEnterCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count++; } ... void WINAPI KeLeaveCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count--; } ... +BOOLEAN WINAPI KeAreApcsDisabled(void) +{ + return !!KeGetCurrentThread()->critical_region_count; +}
Shouldn't these APIs use interlocked operations? -- Dmitry.
I thought about that, but since the only documented way to access it is from the current thread. On Wed, Apr 24, 2019 at 10:32 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Derek Lesho <dereklesho52(a)gmail.com> wrote:
void WINAPI KeEnterCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count++; } ... void WINAPI KeLeaveCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count--; } ... +BOOLEAN WINAPI KeAreApcsDisabled(void) +{ + return !!KeGetCurrentThread()->critical_region_count; +}
Shouldn't these APIs use interlocked operations?
-- Dmitry.
*but since the only documented way to access it is from the current thread, I thought it was unnecessary. On Wed, Apr 24, 2019 at 10:41 AM Derek Lesho <dereklesho52(a)gmail.com> wrote:
I thought about that, but since the only documented way to access it is from the current thread.
On Wed, Apr 24, 2019 at 10:32 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Derek Lesho <dereklesho52(a)gmail.com> wrote:
void WINAPI KeEnterCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count++; } ... void WINAPI KeLeaveCriticalRegion(void) { - FIXME(": stub\n"); + TRACE(": semi-stub\n"); + KeGetCurrentThread()->critical_region_count--; } ... +BOOLEAN WINAPI KeAreApcsDisabled(void) +{ + return !!KeGetCurrentThread()->critical_region_count; +}
Shouldn't these APIs use interlocked operations?
-- Dmitry.
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=51448 Your paranoid android. === w8 (32 bit report) === ntoskrnl.exe: driver.c:1195: Test failed: got 1, expected 0 Report errors: ntoskrnl.exe:ntoskrnl contains a misplaced failure message for driver === debian9 (32 bit Japanese:Japan report) === ntoskrnl.exe: driver.c:638: Test failed: got 0 driver.c:652: Test failed: got 0 driver.c:655: Test failed: got 0x102 === debian9 (64 bit WoW report) === ntoskrnl.exe: driver.c:863: Test failed: obj1 != obj2
participants (3)
-
Derek Lesho -
Dmitry Timoshkov -
Marvin