[PATCH 1/3] ntoskrnl: Set DriverStart and DriverSize fields in DRIVER_OBJECT.

[Derek Lesho]

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 2da58dc104..7604c13b56 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -3645,6 +3645,8 @@ static NTSTATUS WINAPI init_driver( DRIVER_OBJECT *driver_object, UNICODE_STRING
         return STATUS_DLL_INIT_FAILED;
 
     driver_object->DriverSection = find_ldr_module( module );
+    driver_object->DriverStart = ((LDR_MODULE *)driver_object->DriverSection)->BaseAddress;
+    driver_object->DriverSize = ((LDR_MODULE *)driver_object->DriverSection)->SizeOfImage;
 
     nt = RtlImageNtHeader( module );
     if (!nt->OptionalHeader.AddressOfEntryPoint) return STATUS_SUCCESS;
-- 
2.26.2

[PATCH 2/3] ntoskrnl: Always use OBJ_KERNEL_HANDLE when opening objects in winedevice.

[Derek Lesho]

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 7604c13b56..818ff56d25 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -3235,7 +3235,7 @@ NTSTATUS WINAPI ObQueryNameString( void *object, OBJECT_NAME_INFORMATION *name,
 
     TRACE("object %p, name %p, size %u, ret_size %p.\n", object, name, size, ret_size);
 
-    if ((ret = ObOpenObjectByPointer( object, 0, NULL, 0, NULL, KernelMode, &handle )))
+    if ((ret = ObOpenObjectByPointer( object, OBJ_KERNEL_HANDLE, NULL, 0, NULL, KernelMode, &handle )))
         return ret;
     ret = NtQueryObject( handle, ObjectNameInformation, name, size, ret_size );
 
@@ -4189,7 +4189,7 @@ void * WINAPI PsGetProcessSectionBaseAddress(PEPROCESS process)
 
     TRACE("process %p.\n", process);
 
-    if ((status = ObOpenObjectByPointer(process, 0, NULL, PROCESS_ALL_ACCESS, NULL, KernelMode, &h)))
+    if ((status = ObOpenObjectByPointer(process, OBJ_KERNEL_HANDLE, NULL, PROCESS_ALL_ACCESS, NULL, KernelMode, &h)))
     {
         WARN("Error opening process object, status %#x.\n", status);
         return NULL;
-- 
2.26.2

[PATCH 2/3] ntoskrnl: Always use OBJ_KERNEL_HANDLE when opening objects in winedevice.

[Marvin]

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

Your paranoid android.


=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3649:47: error: expected expression before ‘)’ token
Task: The win32 Wine build failed

=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3649:47: error: expected expression before ‘)’ token
Task: The wow64 Wine build failed

[PATCH 3/3] ntoskrnl: Facilitate kernel object field offset fetchers.

[Derek Lesho]

EasyAntiCheat.sys reads IoThreadToProcess and PsGetThreadProcessId to find out the offset of the
KPROCESS and PID fields in the KTHREAD structure.  They rely on the mov instruction using a 32-bit
displacement to get the offset, so we have to make sure the fields are deep enough into the structure.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c         | 1 -
 dlls/ntoskrnl.exe/ntoskrnl_private.h | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 818ff56d25..51603ec3d7 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -2394,7 +2394,6 @@ HANDLE WINAPI PsGetThreadId(PETHREAD thread)
  */
 HANDLE WINAPI PsGetThreadProcessId( PETHREAD thread )
 {
-    TRACE( "%p -> %p\n", thread, thread->kthread.id.UniqueProcess );
     return thread->kthread.id.UniqueProcess;
 }
 
diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h
index a1e1b892e8..9d56b236a5 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl_private.h
+++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h
@@ -39,6 +39,8 @@ struct _OBJECT_TYPE
 struct _EPROCESS
 {
     DISPATCHER_HEADER header;
+    /* padding to require a 32-bit displacement */
+    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];
     PROCESS_BASIC_INFORMATION info;
     BOOL wow64;
 };
@@ -46,6 +48,8 @@ struct _EPROCESS
 struct _KTHREAD
 {
     DISPATCHER_HEADER header;
+    /* padding to require a 32-bit displacement */
+    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];
     PEPROCESS process;
     CLIENT_ID id;
     unsigned int critical_region;
-- 
2.26.2

[PATCH 3/3] ntoskrnl: Facilitate kernel object field offset fetchers.

[Marvin]

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

Your paranoid android.


=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3647:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3647:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:47: error: expected expression before ‘)’ token
Task: The win32 Wine build failed

=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3647:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3647:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:47: error: expected expression before ‘)’ token
Task: The wow64 Wine build failed

[PATCH 3/3] ntoskrnl: Facilitate kernel object field offset fetchers.

[Zebediah Figura]

This subject line seems very confusingly worded. I'd suggest trying to
describe what the patch does instead of why, e.g. 'force the "info"
field of "_EPROCESS" to have an offset of at least 256.'

On 6/19/20 12:35 PM, Derek Lesho wrote:
> EasyAntiCheat.sys reads IoThreadToProcess and PsGetThreadProcessId to find out the offset of the
> KPROCESS and PID fields in the KTHREAD structure.  They rely on the mov instruction using a 32-bit
> displacement to get the offset, so we have to make sure the fields are deep enough into the structure.
>
> Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
> ---
>  dlls/ntoskrnl.exe/ntoskrnl.c         | 1 -
>  dlls/ntoskrnl.exe/ntoskrnl_private.h | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
> index 818ff56d25..51603ec3d7 100644
> --- a/dlls/ntoskrnl.exe/ntoskrnl.c
> +++ b/dlls/ntoskrnl.exe/ntoskrnl.c
> @@ -2394,7 +2394,6 @@ HANDLE WINAPI PsGetThreadId(PETHREAD thread)
>   */
>  HANDLE WINAPI PsGetThreadProcessId( PETHREAD thread )
>  {
> -    TRACE( "%p -> %p\n", thread, thread->kthread.id.UniqueProcess );

Why remove this trace?
> return thread->kthread.id.UniqueProcess;

While this may reliably work in practice, there's no guarantee of it. It
may be a better idea to reimplement the functions in assembly for the
architectures that need it.
> }
>
> diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h
> index a1e1b892e8..9d56b236a5 100644
> --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h
> +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h
> @@ -39,6 +39,8 @@ struct _OBJECT_TYPE
>  struct _EPROCESS
>  {
>      DISPATCHER_HEADER header;
> +    /* padding to require a 32-bit displacement */

I don't think this comment is nearly specific enough. "32-bit
displacement" is meaningless unless you mention the architecture,
instruction, and where that instruction is used. Essentially, everything
that's in the patch summary should probably be here instead.
> +    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];

Presumably this doesn't have to be at offset exactly 0x100; i.e. the "-
sizeof(DISPATCHER_HEADER)" is unnecessary.
> PROCESS_BASIC_INFORMATION info;
>      BOOL wow64;
>  };
> @@ -46,6 +48,8 @@ struct _EPROCESS
>  struct _KTHREAD
>  {
>      DISPATCHER_HEADER header;
> +    /* padding to require a 32-bit displacement */
> +    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];

See above.
> PEPROCESS process;
>      CLIENT_ID id;
>      unsigned int critical_region;

[PATCH 3/3] ntoskrnl: Facilitate kernel object field offset fetchers.

[Derek Lesho]

On 6/19/20 1:43 PM, Zebediah Figura wrote:
> This subject line seems very confusingly worded. I'd suggest trying to
> describe what the patch does instead of why, e.g. 'force the "info"
> field of "_EPROCESS" to have an offset of at least 256.'

👌
> On 6/19/20 12:35 PM, Derek Lesho wrote:
> > EasyAntiCheat.sys reads IoThreadToProcess and PsGetThreadProcessId to find out the offset of the
> > KPROCESS and PID fields in the KTHREAD structure.  They rely on the mov instruction using a 32-bit
> > displacement to get the offset, so we have to make sure the fields are deep enough into the structure.
> >
> > Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
> > ---
> >   dlls/ntoskrnl.exe/ntoskrnl.c         | 1 -
> >   dlls/ntoskrnl.exe/ntoskrnl_private.h | 4 ++++
> >   2 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
> > index 818ff56d25..51603ec3d7 100644
> > --- a/dlls/ntoskrnl.exe/ntoskrnl.c
> > +++ b/dlls/ntoskrnl.exe/ntoskrnl.c
> > @@ -2394,7 +2394,6 @@ HANDLE WINAPI PsGetThreadId(PETHREAD thread)
> >    */
> >   HANDLE WINAPI PsGetThreadProcessId( PETHREAD thread )
> >   {
> > -    TRACE( "%p -> %p\n", thread, thread->kthread.id.UniqueProcess );
> > Why remove this trace?

Because EasyAntiCheat reads the first instruction of the function, if we 
used assembly to re-implement this function, we could avoid this.
> > return thread->kthread.id.UniqueProcess;
>
> While this may reliably work in practice, there's no guarantee of it. It
> may be a better idea to reimplement the functions in assembly for the
> architectures that need it.

Agreed, I'll just do that instead.
> > }
> >
> > diff --git a/dlls/ntoskrnl.exe/ntoskrnl_private.h b/dlls/ntoskrnl.exe/ntoskrnl_private.h
> > index a1e1b892e8..9d56b236a5 100644
> > --- a/dlls/ntoskrnl.exe/ntoskrnl_private.h
> > +++ b/dlls/ntoskrnl.exe/ntoskrnl_private.h
> > @@ -39,6 +39,8 @@ struct _OBJECT_TYPE
> >   struct _EPROCESS
> >   {
> >       DISPATCHER_HEADER header;
> > +    /* padding to require a 32-bit displacement */
> > I don't think this comment is nearly specific enough. "32-bit
> > displacement" is meaningless unless you mention the architecture,
> > instruction, and where that instruction is used. Essentially, everything
> > that's in the patch summary should probably be here instead.

Makes sense, what do you think about writing the x86_64 without an 
assembler in order to ensure a 32-bit displacement value that is below 
0x100?  We could remove the padding this way.
> > +    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];
> > Presumably this doesn't have to be at offset exactly 0x100; i.e. the "-
> > sizeof(DISPATCHER_HEADER)" is unnecessary.

It was just to save space I guess.
> > PROCESS_BASIC_INFORMATION info;
> >       BOOL wow64;
> >   };
> > @@ -46,6 +48,8 @@ struct _EPROCESS
> >   struct _KTHREAD
> >   {
> >       DISPATCHER_HEADER header;
> > +    /* padding to require a 32-bit displacement */
> > +    CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];
>
> See above.
> > PEPROCESS process;
> >       CLIENT_ID id;
> >       unsigned int critical_region;

[Marvin]

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

Your paranoid android.


=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3649:47: error: expected expression before ‘)’ token
Task: The win32 Wine build failed

=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:36: error: ‘LDR_MODULE’ undeclared (first use in this function); did you mean ‘PDI_MODULES’?
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:48: error: expected expression before ‘)’ token
../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3649:47: error: expected expression before ‘)’ token
Task: The wow64 Wine build failed

[PATCH v2 1/3] ntoskrnl: Set DriverStart and DriverSize fields in DRIVER_OBJECT.

[Derek Lesho]

---
v2: Fix structure name.
---
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 2da58dc104..a43cbbd82d 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -3645,6 +3645,8 @@ static NTSTATUS WINAPI init_driver( DRIVER_OBJECT *driver_object, UNICODE_STRING
         return STATUS_DLL_INIT_FAILED;
 
     driver_object->DriverSection = find_ldr_module( module );
+    driver_object->DriverStart = ((LDR_DATA_TABLE_ENTRY *)driver_object->DriverSection)->BaseAddress;
+    driver_object->DriverSize = ((LDR_DATA_TABLE_ENTRY *)driver_object->DriverSection)->SizeOfImage;
 
     nt = RtlImageNtHeader( module );
     if (!nt->OptionalHeader.AddressOfEntryPoint) return STATUS_SUCCESS;
-- 
2.26.2

[PATCH v2 1/3] ntoskrnl: Set DriverStart and DriverSize fields in DRIVER_OBJECT.

[Marvin]

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

Your paranoid android.


=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:88: error: ‘LDR_DATA_TABLE_ENTRY’ {aka ‘struct _LDR_DATA_TABLE_ENTRY’} has no member named ‘BaseAddress’
Task: The win32 Wine build failed

=== debiant (build log) ===

../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:88: error: ‘LDR_DATA_TABLE_ENTRY’ {aka ‘struct _LDR_DATA_TABLE_ENTRY’} has no member named ‘BaseAddress’
Task: The wow64 Wine build failed

[PATCH v2 1/3] ntoskrnl: Set DriverStart and DriverSize fields in DRIVER_OBJECT.

[Derek Lesho]

On 6/19/20 12:59 PM, Marvin wrote:
> 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=73799
>
> Your paranoid android.
>
> === debiant (build log) ===
>
> ../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:88: error: ‘LDR_DATA_TABLE_ENTRY’ {aka ‘struct _LDR_DATA_TABLE_ENTRY’} has no member named ‘BaseAddress’
> Task: The win32 Wine build failed
>
> === debiant (build log) ===
>
> ../../../wine/dlls/ntoskrnl.exe/ntoskrnl.c:3648:88: error: ‘LDR_DATA_TABLE_ENTRY’ {aka ‘struct _LDR_DATA_TABLE_ENTRY’} has no member named ‘BaseAddress’
> Task: The wow64 Wine build failed
> I thought this patch was simple enough not the compile on the rebased 
> tree, clearly not

😅

<https://emojipedia.org/grinning-face-with-sweat/>


  Search Results


    Web results

[PATCH v4 1/3] ntoskrnl: Set DriverStart and DriverSize fields in DRIVER_OBJECT.

[Derek Lesho]

v3: Fixed a field name 😅
---
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/ntoskrnl.exe/ntoskrnl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 2da58dc104..f952e5d53b 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -3645,6 +3645,8 @@ static NTSTATUS WINAPI init_driver( DRIVER_OBJECT *driver_object, UNICODE_STRING
         return STATUS_DLL_INIT_FAILED;
 
     driver_object->DriverSection = find_ldr_module( module );
+    driver_object->DriverStart = ((LDR_DATA_TABLE_ENTRY *)driver_object->DriverSection)->DllBase;
+    driver_object->DriverSize = ((LDR_DATA_TABLE_ENTRY *)driver_object->DriverSection)->SizeOfImage;
 
     nt = RtlImageNtHeader( module );
     if (!nt->OptionalHeader.AddressOfEntryPoint) return STATUS_SUCCESS;
-- 
2.26.2