Share Gracia Final Teleport Crash Partial Fix

[eressea]

There's a race condition error in engine.dll:

 

engine.dll:00CE80AA lea     eax, [eax+edx*4]

engine.dll:00CE80AD cmp     dword ptr [eax+10h], 0  <----- now it's not 0, so it won't jump on the next line

engine.dll:00CE80B1 jz      0CE80C8h     -- meanwhile some other thread sets dword ptr [eax+10h] to 0 --

engine.dll:00CE80B3 mov     ecx, [eax+10h]    <----- so now we have ecx == 0

engine.dll:00CE80B6 mov     ecx, [ecx+1Ch]    <----- read dword ptr [0+1ch] -> CRASH

engine.dll:00CE80B9 test    ecx, 0x4000000

engine.dll:00CE80BF jnz     short loc_CE80C8

engine.dll:00CE80C1 mov     dword ptr [eax+10h], 0

 

It can be fixed this way:

 

engine.dll:00CE80AA lea     eax, [eax+edx*4]

engine.dll:00CE80AD mov     ecx, [eax+10h]   <----- we copy that value from memory

engine.dll:00CE80B0 jecxz   0CE80C8h          <----- and compare it this way - jecxz is a nice instruction :)

engine.dll:00CE80B2 mov     ecx, [ecx+1ch]    <----- even if some other thread resets dword ptr [eax+10h], we have still copy in ecx

engine.dll:00CE80B5 test    ecx, 0x4000000

engine.dll:00CE80BB jne     0CE80C8h

engine.dll:00CE80BD nop

engine.dll:00CE80BE nop           <----- we saved 4 bytes :)

engine.dll:00CE80BF nop

engine.dll:00CE80C0 nop

engine.dll:00CE80C1 mov     dword ptr [eax+10h], 0

 

There are two occurrences of this bug in engine.dll, to fix them both replace following (in unpacked engine.dll ofc)

 

old: 83 78 10 00 74 15 8B 48 10 8B 49 1C F7 C1 00 00 00 04 75 07

new: 8B 48 10 E3 16 8B 49 1C F7 C1 00 00 00 04 75 0B 90 90 90 90

 

old: 83 78 10 00 74 13 8B 48 10 F7 41 1C 00 00 00 04 75 07

new: 8B 48 10 E3 14 F7 41 1C 00 00 00 04 75 0b 90 90 90 90

 

Enjoy ;)

[svipben]

Send you a PM, please read it!

[LightFusion]

 

There's a race condition error in engine.dll:

 

engine.dll:00CE80AA lea     eax, [eax+edx*4]

engine.dll:00CE80AD cmp     dword ptr [eax+10h], 0  <----- now it's not 0, so it won't jump on the next line

engine.dll:00CE80B1 jz      0CE80C8h     -- meanwhile some other thread sets dword ptr [eax+10h] to 0 --

engine.dll:00CE80B3 mov     ecx, [eax+10h]    <----- so now we have ecx == 0

engine.dll:00CE80B6 mov     ecx, [ecx+1Ch]    <----- read dword ptr [0+1ch] -> CRASH

engine.dll:00CE80B9 test    ecx, 0x4000000

engine.dll:00CE80BF jnz     short loc_CE80C8

engine.dll:00CE80C1 mov     dword ptr [eax+10h], 0

 

It can be fixed this way:

 

engine.dll:00CE80AA lea     eax, [eax+edx*4]

engine.dll:00CE80AD mov     ecx, [eax+10h]   <----- we copy that value from memory

engine.dll:00CE80B0 jecxz   0CE80C8h          <----- and compare it this way - jecxz is a nice instruction :)

engine.dll:00CE80B2 mov     ecx, [ecx+1ch]    <----- even if some other thread resets dword ptr [eax+10h], we have still copy in ecx

engine.dll:00CE80B5 test    ecx, 0x4000000

engine.dll:00CE80BB jne     0CE80C8h

engine.dll:00CE80BD nop

engine.dll:00CE80BE nop           <----- we saved 4 bytes :)

engine.dll:00CE80BF nop

engine.dll:00CE80C0 nop

engine.dll:00CE80C1 mov     dword ptr [eax+10h], 0

 

There are two occurrences of this bug in engine.dll, to fix them both replace following (in unpacked engine.dll ofc)

 

old: 83 78 10 00 74 15 8B 48 10 8B 49 1C F7 C1 00 00 00 04 75 07

new: 8B 48 10 E3 16 8B 49 1C F7 C1 00 00 00 04 75 0B 90 90 90 90

 

old: 83 78 10 00 74 13 8B 48 10 F7 41 1C 00 00 00 04 75 07

new: 8B 48 10 E3 14 F7 41 1C 00 00 00 04 75 0b 90 90 90 90

 

Enjoy ;)

 

do you even know what that code is doing ? originaly , just setting to zero wouldnt explain the crash.

[eressea]

do you even know what that code is doing ? originaly , just setting to zero wouldnt explain the crash.

 

Not in the global scope, but for example the first one is in UGameEngine::LoadMapThread.

They call something like

 

UObjectLoader *res = UObject::GetLoader(something);

if (res->var84h[something2]->var10h) {

    here they access res->var84h[something2]->var10h->var1c

    which is totally wrong if you don't have mutex here

}

 

so my fix does simply this:

 

void *someptr = res->var84h[something2]->var10h;

if (*someptr) {

    now i work with someptr which is copy

}

 

the best solution would be to add mutexes, but I don't have the source code :))

[LightFusion]

Not in the global scope, but for example the first one is in UGameEngine::LoadMapThread.

They call something like

 

UObjectLoader *res = UObject::GetLoader(something);

if (res->var84h[something2]->var10h) {

    here they access res->var84h[something2]->var10h->var1c

    which is totally wrong if you don't have mutex here

}

 

so my fix does simply this:

 

void *someptr = res->var84h[something2]->var10h;

if (*someptr) {

    now i work with someptr which is copy

}

 

the best solution would be to add mutexes, but I don't have the source code :))

well thats acctualy explains better, a check if exist pattern 

[UgliBiBagli]

is that the god of optimization we were all looking for?