Page 1 of 1

[FIXED]Traits - World Crash

Posted: Tue Jan 17, 2012 7:24 am
by John Adams
Hmm, the Worlds are crashing a helluva lot more than they should be lately. Hopefully some of this is just my bad LogWrite data ;)

Crash:
*** buffer overflow detected ***: /home/eq2dev/bin/world/current_world terminate d
======= Backtrace: =========
/lib/tls/i686/cmov/libc.so.6(__fortify_fail+0x48)[0xb7bff008]
/lib/tls/i686/cmov/libc.so.6[0xb7bfe040]
/lib/tls/i686/cmov/libc.so.6[0xb7bfd36a]
/home/eq2dev/bin/world/current_world(_ZN15MasterTraitList18GetTraitListPacketEP6 Client+0x1124)[0x823b250]
/home/eq2dev/bin/world/current_world(_ZN21ClientPacketFunctions13SendTraitListEP 6Client+0x1a)[0x813401a]
/home/eq2dev/bin/world/current_world(_ZN6Client12SendCharInfoEv+0xa35)[0x8117ce7 ]
/home/eq2dev/bin/world/current_world(_ZN6Client12HandlePacketEP19EQApplicationPa cket+0x1b4a)[0x8121512]
/home/eq2dev/bin/world/current_world(_ZN6Client7ProcessEb+0x106)[0x81244b8]
/home/eq2dev/bin/world/current_world(_ZN10ZoneServer13ClientProcessEv+0x109)[0x8 2b6f25]
/home/eq2dev/bin/world/current_world(_ZN10ZoneServer7ProcessEv+0x46d)[0x82b8ae5]
/home/eq2dev/bin/world/current_world(_Z8ZoneLoopPv+0x120)[0x82b8f71]
/lib/tls/i686/cmov/libpthread.so.0[0xb7c6880e]
/lib/tls/i686/cmov/libc.so.6(clone+0x5e)[0xb7beaa0e]

Linux bt:

Code: Select all

(gdb) bt
#0  0xb7fe2430 in __kernel_vsyscall ()
#1  0xb7b484d1 in raise () from /lib/tls/i686/cmov/libc.so.6
#2  0xb7b4b932 in abort () from /lib/tls/i686/cmov/libc.so.6
#3  0xb7b7efc5 in ?? () from /lib/tls/i686/cmov/libc.so.6
#4  0xb7bff008 in __fortify_fail () from /lib/tls/i686/cmov/libc.so.6
#5  0xb7bfe040 in __chk_fail () from /lib/tls/i686/cmov/libc.so.6
#6  0xb7bfd36a in __strcat_chk () from /lib/tls/i686/cmov/libc.so.6
#7  0x0823b250 in strcat (this=0x834d234, client=0xb12026e8) at /usr/include/bits/string3.h:145
#8  MasterTraitList::GetTraitListPacket (this=0x834d234, client=0xb12026e8) at Traits/Traits.cpp:160
#9  0x0813401a in ClientPacketFunctions::SendTraitList (client=0xb12026e8) at ClientPacketFunctions.cpp:110
#10 0x08117ce7 in Client::SendCharInfo (this=0xb12026e8) at client.cpp:504
#11 0x08121512 in Client::HandlePacket (this=0xb12026e8, app=0xe32cc20) at client.cpp:960
#12 0x081244b8 in Client::Process (this=0xb12026e8, zone_process=true) at client.cpp:1775
#13 0x082b6f25 in ZoneServer::ClientProcess (this=0xb1146460) at zoneserver.cpp:1919
#14 0x082b8ae5 in ZoneServer::Process (this=0xb1146460) at zoneserver.cpp:889
#15 0x082b8f71 in ZoneLoop (tmp=0xb1146460) at zoneserver.cpp:4010
#16 0xb7c6880e in start_thread () from /lib/tls/i686/cmov/libpthread.so.0
#17 0xb7beaa0e in clone () from /lib/tls/i686/cmov/libc.so.6

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 7:35 am
by reefcrazed
I think mine has been crashing, cannot remember if it is just me not doing a & after the executable. I remembered to run it as eq2emu & last night and the world was still going when I got up. I plan on working on quest all day today so I will let you know. Well, let's just say I am getting organized to do quest, I am still in learning mode right now. LUA will take a short while to get used to.

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 3:24 pm
by John Adams
K, Scat, how about this one? This is not logging, but I think the functions Jabantiz used that may be Windows specific.

Code: Select all

				strcpy(sTrait, "trait");
				itoa(count, temp, 10);
				strcat(sTrait, temp);
Works on Windows, crashes Linux world when you attempt to log in.

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 4:40 pm
by Scatman
I need to see the definitions for sTrait and count.

Edit: Nevermind I found it.

Not sure why it's crashing. itoa() is not an ANSI standard function so C/C++ compilers are not required to support it and g++ may have a different variation than Visual Studio does. Instead I would use

Code: Select all

sprintf(temp, "%i", count);

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 5:17 pm
by Jabantiz
Is it itoa that is causing the crash? I found this code in MiscFunctions.cpp

Code: Select all

#ifndef WIN32
const char * itoa(int num) {
		static char temp[_ITOA_BUFLEN];
		memset(temp,0,_ITOA_BUFLEN);
		snprintf(temp,_ITOA_BUFLEN,"%d",num);
		return temp;
}


const char * itoa(int num, char* a,int b) {
		static char temp[_ITOA_BUFLEN];
		memset(temp,0,_ITOA_BUFLEN);
		snprintf(temp,_ITOA_BUFLEN,"%d",num);
		return temp;
		return temp;
}
#endif
so itoa should work on linux as well right?

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 5:30 pm
by John Adams
I only suspected strcat() because nowhere in the entire project is that used. I assumed LE avoided it for a reason (cuz isn't that really old school C?)

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 5:33 pm
by Scatman
Ah, didn't know we had that in there. Ouch, the second function is a very poor implementation of itoa, that's for sure. It should be stuffing the resulting string into a and returning a;

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 5:35 pm
by Scatman
strcat() is fine, it's just dangerous because it has no buffer overflow checking. In the context it's used, it shouldn't be an issue at all.

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 5:42 pm
by John Adams
Scat, to reproduce this error you just need to source in the Traits data (thus waking up the MasterTraitsList) on a Linux box. All the data Zcoretri provided is good, but you have to update the Tier to 1 for all the tables.

Code: Select all

UPDATE spell_traits SET tier = 1;
UPDATE spell_tiers SET tier = 1 WHERE spell_id >= 1000000;
UPDATE spell_display_effects SET tier = 1 WHERE spell_id >= 1000000;
UPDATE spell_data SET tier = 1 WHERE spell_id >= 1000000;
This should make it crash on your linux box, too. Hopefully.

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 6:04 pm
by Scatman
I just realized the reason this is crashing is because of the improper implementation of itoa(). Here's what's happening.

The second itoa() is being used.
This implementation completely ignores the 2nd and 3rd parameter. The 2nd parameter is possible output so this IS BAD.
Since the second parameter is never being touched (and Jab is using this function correctly since it should be modified), he's trying to strcat() whatever random memory is being placed into temp.

Implementing a proper itoa() will fix this crash.

Re: Traits - World Crash

Posted: Tue Jan 17, 2012 7:35 pm
by Scatman
I've committed a (hopefully) proper version of itoa() for Linux.

Re: Traits - World Crash

Posted: Wed Jan 18, 2012 7:13 am
by John Adams
If someone will, log in to EQ2 DB server and see if it crashes. I have replaced the Traits data, but will not be able to log in myself til later this afternoon. Otherwise, I'll get it when I get home.

Something I was thinking about last night, Scatman.... how is EQEmu functioning with this malformed itoa on linux? Far as I know, they have no problems at all. I run their server on a linux box, too.

Re: Traits - World Crash

Posted: Wed Jan 18, 2012 10:01 am
by TILT
I logged onto EQ2Emu DB server and so far it hasn't crashed.

Edit: My original post was off topic, so I deleted it. I'll post it again in the correct topic.

Re: Traits - World Crash

Posted: Wed Jan 18, 2012 10:13 am
by Scatman
There are two ways you can use itoa(). You pass in the value with a buffer that's large enough to fit the string version of value into result. itoa() will also return a pointer to result. Why does it do that? So you can use itoa() as a paramter to functions too

Code: Select all

char * itoa(int value, char *result, int base)
So the first way is to use the return value of itoa()

Code: Select all

char unused[128];
int number = 23;
printf("I'm using the return value of itoa() to turn %i into %s\n", number, itoa(number, unused, 10));
Or you can use it how Jab was using it and use the out parameter, which is how you should use it if you're not using itoa() as a parameter to a function

Code: Select all

char result[128];
int number = 23;
itoa(number, result, 10);
printf("I'm using the out parameter of itoa() to turn %i into %s\n", number, result);
Basically EQEmu's implementation was ignoring the second example and only returning a statically allocated string, as if the 2nd two parameters never even existed. I have no idea why they did this and my only assumption can be that they always used the return value of itoa() and not the out parameter.