eressea Posted June 10, 2016 Share Posted June 10, 2016 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 ;) Quote Link to comment Share on other sites More sharing options...
svipben Posted June 13, 2016 Share Posted June 13, 2016 Send you a PM, please read it! Quote Link to comment Share on other sites More sharing options...
LightFusion Posted June 13, 2016 Share Posted June 13, 2016 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. Quote Link to comment Share on other sites More sharing options...
eressea Posted June 13, 2016 Author Share Posted June 13, 2016 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 :)) Quote Link to comment Share on other sites More sharing options...
LightFusion Posted June 13, 2016 Share Posted June 13, 2016 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 Quote Link to comment Share on other sites More sharing options...
UgliBiBagli Posted June 13, 2016 Share Posted June 13, 2016 is that the god of optimization we were all looking for? Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.
Note: Your post will require moderator approval before it will be visible.