[FIXED]Traits - World Crash

Old bugs stored here for reference.
Locked
User avatar
John Adams
Retired
Posts: 9684
Joined: Thu Jul 26, 2007 6:27 am
EQ2Emu Server: EQ2Emulator Test Center
Characters: John
Location: Arizona
Contact:

[FIXED]Traits - World Crash

Post by John Adams » Tue Jan 17, 2012 7:24 am

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

User avatar
reefcrazed
Posts: 72
Joined: Tue May 10, 2011 10:22 am
EQ2Emu Server: Dragons of Mist
Characters: Provocating

Re: Traits - World Crash

Post by reefcrazed » Tue Jan 17, 2012 7:35 am

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.
aka Provocating

Client Version 6118L

User avatar
John Adams
Retired
Posts: 9684
Joined: Thu Jul 26, 2007 6:27 am
EQ2Emu Server: EQ2Emulator Test Center
Characters: John
Location: Arizona
Contact:

Re: Traits - World Crash

Post by John Adams » Tue Jan 17, 2012 3:24 pm

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.

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Tue Jan 17, 2012 4:40 pm

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);

Jabantiz
Lead Developer
Posts: 2912
Joined: Wed Jul 25, 2007 2:52 pm
Location: California

Re: Traits - World Crash

Post by Jabantiz » Tue Jan 17, 2012 5:17 pm

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?

User avatar
John Adams
Retired
Posts: 9684
Joined: Thu Jul 26, 2007 6:27 am
EQ2Emu Server: EQ2Emulator Test Center
Characters: John
Location: Arizona
Contact:

Re: Traits - World Crash

Post by John Adams » Tue Jan 17, 2012 5:30 pm

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?)

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Tue Jan 17, 2012 5:33 pm

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;

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Tue Jan 17, 2012 5:35 pm

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.

User avatar
John Adams
Retired
Posts: 9684
Joined: Thu Jul 26, 2007 6:27 am
EQ2Emu Server: EQ2Emulator Test Center
Characters: John
Location: Arizona
Contact:

Re: Traits - World Crash

Post by John Adams » Tue Jan 17, 2012 5:42 pm

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.

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Tue Jan 17, 2012 6:04 pm

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.

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Tue Jan 17, 2012 7:35 pm

I've committed a (hopefully) proper version of itoa() for Linux.

User avatar
John Adams
Retired
Posts: 9684
Joined: Thu Jul 26, 2007 6:27 am
EQ2Emu Server: EQ2Emulator Test Center
Characters: John
Location: Arizona
Contact:

Re: Traits - World Crash

Post by John Adams » Wed Jan 18, 2012 7:13 am

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.

User avatar
TILT
Posts: 114
Joined: Thu Oct 13, 2011 12:03 am
Location: > /dev/null

Re: Traits - World Crash

Post by TILT » Wed Jan 18, 2012 10:01 am

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.
"Hell, there are no rules here - we're trying to accomplish something." Thomas A. Edison

User avatar
Scatman
Retired
Posts: 1688
Joined: Wed Apr 16, 2008 5:44 am
EQ2Emu Server: Scatman's Word
Characters: Scatman
Location: New Jersey

Re: Traits - World Crash

Post by Scatman » Wed Jan 18, 2012 10:13 am

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.

Locked

Who is online

Users browsing this forum: No registered users and 0 guests