<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>On 6/19/20 1:43 PM, Zebediah Figura wrote:<br>
</p>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">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.'</pre>
</blockquote>
<br>
<h1 style="margin: 0px 0px 10px; padding: 0px; border: 0px; outline:
0px; font-size: 28px; vertical-align: baseline; background: 0px
0px; color: rgb(0, 0, 0); font-family: "helvetica neue",
Helvetica, Arial, sans-serif; font-style: normal;
font-variant-ligatures: normal; font-variant-caps: normal;
letter-spacing: normal; orphans: 2; text-align: start;
text-indent: 0px; text-transform: none; white-space: normal;
widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px;
text-decoration-style: initial; text-decoration-color: initial;"><span
class="emoji" style="margin: 0px; padding: 0px; border: 0px;
outline: 0px; font-size: 28px; vertical-align: baseline;
background: 0px 0px; font-weight: 400; font-family: "apple
color emoji", "segoe ui emoji", "noto color
emoji", "android emoji", emojisymbols,
"emojione mozilla", "twemoji mozilla",
"segoe ui symbol";">👌</span></h1>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">
On 6/19/20 12:35 PM, Derek Lesho wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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 <a class="moz-txt-link-rfc2396E" href="mailto:dlesho@codeweavers.com"><dlesho@codeweavers.com></a>
---
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 );
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Why remove this trace?</pre>
</blockquote>
Because EasyAntiCheat reads the first instruction of the function,
if we used assembly to re-implement this function, we could avoid
this.<br>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> return thread->kthread.id.UniqueProcess;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.</pre>
</blockquote>
Agreed, I'll just do that instead.<br>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> }
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 */
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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.</pre>
</blockquote>
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.<br>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ CHAR padding[0x100 - sizeof(DISPATCHER_HEADER)];
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Presumably this doesn't have to be at offset exactly 0x100; i.e. the "-
sizeof(DISPATCHER_HEADER)" is unnecessary.</pre>
</blockquote>
It was just to save space I guess.<br>
<blockquote type="cite"
cite="mid:94c2cc1f-76c5-a275-9496-3fb76e0d4d32@codeweavers.com">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> 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)];
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
See above.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> PEPROCESS process;
CLIENT_ID id;
unsigned int critical_region;
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
</html>