[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 at 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;
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20200619/b840acec/attachment.sig>