Summary:	ASTERISK-17310: [patch] Alignment issue cause multiple failures with 1.8 on ARM
Reporter:	radael (radael)	Labels:
Date Opened:	2011-01-28 18:41:09.000-0600	Date Closed:	2011-11-02 16:34:15
Priority:	Major	Regression?	No
Status:	Closed/Complete	Components:	Core/Portability
Versions:		Frequency of Occurrence
Related Issues:	is related to ASTERISK-17946 AST_STRING_FIELD new pointer overlap the last string on MIPS
Environment:		Attachments:	( 0) issueA17310-no_misalign_autoconf.patch ( 1) patch.patch ( 2) trace.txt ( 3) utils.c.patch
Description:	Also seen on the Dockstar and other Kirkwood platforms. The Kirkwood [all ARM cpus?] require addresses to be word, half-word or byte aligned, depending on the type being stored into memory. Asterisk dynamic string management uses a half-word value preceding the string storage location to track the string length.  When storing this length, if the string is not half-word aligned, the length value will also not be aligned and will trample on the terminating '\0' of the previous string (adding an unprintable char to the end of the string).  In addition, setting the first string in a buffer could potentially corrupt some other memory. The result of this string mangling can be variety of errors.  Since users generally test their devices and connections before moving on to other functionality, some of the more common "errors" presenting are: In SIP:    * "ERROR[23208]: rtp_engine.c:325 ast_rtp_instance_new: No RTP engine was found. Do you have one loaded?      [Nov 9 13:06:32] ERROR[23208]: chan_sip.c :8078 process_sdp: Got SDP but have no RTP session allocated."    * The inability to use a secret.    * Issues with the deny/permit IP address ranges In IAX:    * Failure to validate with RSA keys (could not find the keys)    * Failure to find the context and number when dialing into the pbx from outside.    * Inability to use deny/permit to restrict by IP address The actual error takes place in include/asterisk/stringfields.h Line 304: #define AST_STRING_FIELD_ALLOCATION(x) *((ast_string_field_allocation *) (x - sizeof(ast_string_field_allocation))) ****** STEPS TO REPRODUCE ****** Since the errors which presents are dependent upon the order and length of the strings in the configuration, it is difficult to reproduce.  Hopefully, the attached trace document will be sufficient to demonstrate the issue. ****** ADDITIONAL INFORMATION ****** The issue has already been resolved for the SPARC platform with code in main/utils.c.   To resolve the issue for ARM platforms, the two relevant pre-compiler statements need to look for that platform (see attached patch), and the variable __arm__ must be set in the make environment. For those individuals seeking to resolve the issue before a fix is applied to the tree, the patch can be applied, and the needed variable can also be defined in the same file: #define __arm__ 1
Comments:	By: radael (radael) 2011-01-28 18:53:29.000-0600 Check the patch, I'm not sure of the correct format.  And, to repeat, it's not complete unless __arm__ is defined. By: S Adrian (d3xt3r01.tk) 2011-10-26 14:33:55.656-0500 It does work. the 2 lines that look like main/utils.c:#ifdef __sparc__ should be main/utils.c:#if defined(__sparc__) || defined(__arm__) include/asterisk/compat.h:#ifdef __sparc__ becomes include/asterisk/compat.h:#if defined(__sparc__) || defined(__arm__) include/asterisk/unaligned.h:#elif defined(SOLARIS) && defined(__sparc__) becomes include/asterisk/unaligned.h:#elif (defined(SOLARIS) && defined(__sparc__)) || defined(__arm__) each of these 3 files should also include some #define __arm__ 1 at the top of the file/defines section :) Just tested and they work. By: Walter Doekes (wdoekes) 2011-10-26 15:30:29.930-0500 I was thinking something along the lines of this: an autoconf test to test if unaligned access is allowed. You'll need to run ./bootstrap.sh before running ./configure Note that I'm in favor of removing the #ifdef in main/utils.c completely. That int16 should be aligned in x86 as well. By: Tzafrir Cohen (tzafrir) 2011-10-26 18:28:06.689-0500 Isn't there a performance hit from mis-alignment on other architectures even if they don't panic? What about x86? By: Walter Doekes (wdoekes) 2011-10-27 02:02:28.928-0500 From the experience I had with misalignment recently: - some systems cause a SIGBUS to be raised (sparc) - some systems fail silently by giving the wrong number (arm) - some systems warn -- warnings on stderr (or /dev/console), don't ask me how that works -- but work (ia64) - some systems only have a performance penalty (x86) In my case, I had the choice between doing extra memcpy's to align things. In this case, these extra memcpy's were far more expensive than the misalignment cost. But in the case we're discussing here, it wouldn't matter either way: - you're not reading/writing the int16 that often, so you won't ever notice this - losing an extra byte in memory by aligning it is cheap too My suggestions: - kill the #ifdef in main/utils.c => everyone should get an aligned int16 - use the autoconf test for unaligned.h By: S Adrian (d3xt3r01.tk) 2011-10-27 13:41:42.849-0500 Unfortunately, I'm not such a great C coder so I don't know how I could help more. Found out later that defining __arm__ isn't necessary since gnu C defines it.