Opened on 07/24/19 at 21:54:50
Last modified on 07/31/19 at 19:32:12
#3772 new defect
missaligned pointer dereference in cpukit/libnetworking/libc/res_query.c:131
Reported by: | Jeffrey Hill | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | network/legacy | Version: | 4.11 |
Severity: | normal | Keywords: | missaligned libnetworking |
Cc: | Blocked By: | ||
Blocking: |
Description
nios2-rtems4.11-gcc (GCC) 4.9.3 20150626 (RTEMS 4.11, RSB no-repo, Newlib 2.2.0.20150423)
configure --target=nios2-rtems4.11 --prefix=/ade/rtems/install/rtems-4-11 --disable-itron --disable-tests --enable-posix --enable-cxx --enable-rtemsbsp=altera-sys-config-S43X-TDAQ-dev --enable-networking
This is occurring in 4.11.1.99.
I don't suggest that this fix is a great design, but its what we are currently using here to get our regression tests to pass w/o causing a misaligned pointer exception. This change probably requires at least gcc 4.6 to properly align a char buffer on the stack this way.
git diff /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/res_query.c diff --git a/cpukit/libnetworking/libc/res_query.c b/cpukit/libnetworking/libc/res_query.c index b742c30..e06f70f 100644 --- a/cpukit/libnetworking/libc/res_query.c +++ b/cpukit/libnetworking/libc/res_query.c @@ -113,7 +113,7 @@ res_query( u_char *answer, /* buffer to put answer */ int anslen) /* size of answer buffer */ { - u_char buf[MAXPACKET]; + u_char buf[MAXPACKET] __attribute ((aligned(4))); HEADER *hp = (HEADER *) answer; int n;
#0 _Thread_Do_dispatch (cpu_self=<optimized out>, level=<optimized out>, level@entry=1) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/score/src/threaddispatch.c:132 #1 0x0807a6d4 in _Thread_Dispatch_enable (cpu_self=<optimized out>) at ../../cpukit/../../../altera-sys-config-llrf-fcm-diacrode/lib/include/rtems/score/threaddispatch.h:313 #2 _Thread_Enable_dispatch_body () at ../../cpukit/../../../altera-sys-config-llrf-fcm-diacrode/lib/include/rtems/score/threaddispatch.h:343 #3 _Thread_Enable_dispatch () at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/score/src/threadenabledispatch.c:30 #4 0x08075f3c in _Objects_Put (the_object=<optimized out>) at ../../cpukit/../../../altera-sys-config-llrf-fcm-diacrode/lib/include/rtems/score/objectimpl.h:975 #5 rtems_task_suspend (id=id@entry=0) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/rtems/src/tasksuspend.c:37 #6 0x0813d808 in _Nios2_CPU_exception_handler_last_resort (pefr=0x81d498c, ctx=<optimized out>) at /ade/rtems/release/rtems-4-11/c/src/lib/libcpu/nios2/shared/except/except-last-resort.c:64 #7 0x0809d9ac in _Nios2_Exception_handler_high_level (pefr=0x81d498c) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/score/cpu/nios2/nios2-except.c:119 #8 0x0809d86c in _Nios2_Exception_handler () at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/score/cpu/nios2/nios2-except-low-level.S:96 #9 0x08085980 in CPU_swap_u16 (value=10412) at ../../cpukit/../../../altera-sys-config-llrf-fcm-diacrode/lib/include/rtems/score/cpu.h:402 #10 htons (_x=10412) at ../../cpukit/../../../altera-sys-config-llrf-fcm-diacrode/lib/include/rtems/endian.h:83 #11 __res_mkquery (op=op@entry=0, dname=dname@entry=0x81d4aa8 "localhost.lcs.net", class=class@entry=1, type=type@entry=1, data=data@entry=0x0, datalen=datalen@entry=0, newrr_in=newrr_in@entry=0x0, buf=buf@entry-11/c/src/../../cpukit/libnetworking/libc/res_mkquery.c:122 #12 0x08058bcc in __res_query (anslen=<optimized out>, answer=0x81d5318 "stupwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzpe=1, class=1, name=0x81d4aa8 "localhost.lcs.net") at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/res_query.c:131 #13 __res_querydomain (name=name@entry=0x81d576c "localhost", domain=<optimized out>, class=class@entry=1, type=type@entry=1, answer=answer@entry=0x81d5318 "stupwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghij"..., anslen=anslen@entry=1024) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/res_query.c:368 #14 0x080590e0 in __res_search (name=0x81d576c "localhost", class=1, type=1, answer=0x81d5318 "stupwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdwxyzabcdefghij"..., anslen=1024) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/res_query.c:242 #15 0x080808b8 in _gethostbydnsname (name=name@entry=0x81d576c "localhost", af=af@entry=2) at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/gethostbydns.c:588 #16 0x08056c58 in gethostbyname2 (type=2, name=0x81d576c "localhost") at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/gethostnamadr.c:153 #17 gethostbyname (name=name@entry=0x81d576c "localhost") at /ade/rtems/release/rtems-4-11/c/src/../../cpukit/libnetworking/libc/gethostnamadr.c:133 #18 0x08045198 in hostToIPAddr (pHostName=pHostName@entry=0x81d576c "localhost", pIPA=pIPA@entry=0x81d5984) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/osi/os/posix/osdSock.c:170 #19 0x0803990c in aToIPAddr (pAddrString=pAddrString@entry=0x81d59b0 "localhost", defaultPort=defaultPort@entry=0, pIP=pIP@entry=0x81d5a30) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/misc/aToIPAddr.c:168 #20 0x0804ed80 in envGetInetAddrConfigParam (pParam=pParam@entry=0x81a723c <EPICS_IOC_LOG_INET>, pAddr=pAddr@entry=0x81d5a50) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/env/envSubr.c:258 #21 0x08036b2c in getConfig (pserver_port=<synthetic pointer>, pserver_addr=0x81d5a50) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/log/iocLog.c:57 #22 iocLogClientInit () at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/log/iocLog.c:88 #23 iocLogInit () at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/log/iocLog.c:116 #24 0x08008d60 in testLogPrefix () at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/test/epicsErrlogTest.c:402 #25 epicsErrlogTest () at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/test/epicsErrlogTest.c:324 #26 0x0803c51c in runTestFunc (name=name@entry=0x8189cf4 "epicsErrlogTest", func=0x80085e4 <epicsErrlogTest>) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/misc/epicsUnitTest.c:262 #27 0x08018b74 in epicsRunLibComTests () at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/test/epicsRunLibComTests.c:66 #28 0x0802854c in main (argc=<optimized out>, argv=<optimized out>) at /ade/epics/supTop/base/DAR3.15.1.1/src/libCom/test/rtemsTestHarness.c:16 (
(gdb) print &buf $9 = (u_char (*)[1024]) 0x81d4ea9 (gdb) list - 116 u_char buf[MAXPACKET]; 117 HEADER *hp = (HEADER *) answer; 118 int n; 119 120 hp->rcode = NOERROR; /* default */ 121 122 if ((_res.options & RES_INIT) == 0 && res_init() == -1) { 123 h_errno = NETDB_INTERNAL; 124 return (-1); 125 }
(gdb) print &hp->id $3 = (unsigned int *) 0x81d4ea9 (gdb) print &hp $4 = (HEADER **) 0x81d4aa0 (gdb) print *hp $5 = {id = 0, rd = 0, tc = 0, aa = 0, opcode = 0, qr = 0, rcode = 0, cd = 0, ad = 0, unused = 0, ra = 0, qdcount = 0, ancount = 0, nscount = 0, arcount = 0} (gdb) print &hp->id $6 = (unsigned int *) 0x81d4ea9 (gdb) print hp $7 = (HEADER *) 0x81d4ea9 (gdb) list 117 */ 118 if ((buf == NULL) || (buflen < HFIXEDSZ)) 119 return (-1); 120 memset(buf, 0, HFIXEDSZ); 121 hp = (HEADER *) buf; 122 hp->id = htons(++_res.id); 123 hp->opcode = op; 124 hp->rd = (_res.options & RES_RECURSE) != 0; 125 hp->rcode = NOERROR;
Change History (7)
comment:1 Changed on 07/24/19 at 22:00:11 by Joel Sherrill
comment:2 Changed on 07/24/19 at 22:38:11 by Jeffrey Hill
Component: | admin → network/libbsd |
---|
comment:3 Changed on 07/25/19 at 00:13:25 by Jeffrey Hill
Are you compiling with any type of strict alignment flags?
Not that I know of, this is what we are using, and probably nothing additional relevant in the specs file.
-O3 -ggdb -Wall -Wmissing-prototypes -Wimplicit-function-declaration -Wstrict-prototypes -Wnested-externs -MT serial/libserialio_a-mc68681.o -MD -MP -MF
A union requires more modification of the software I think.
That would be more portable. The cplusplus 11 has alignment features but C doesn't yet at this time probably?
Are we sure 4 is a portable answer.
The particular alignment issue causing the failure is probably a two byte one, based only on use of htons in that place in the code, but I definitely haven't looked around in this code to find the worst case alignment issue. Furthermore, I suspect there are additional instances of these char aligned buffers getting casted alarmingly into random data types therein ...
Is the stack itself properly aligned initially?
I suppose that we _are_ an unusual BSP/ARCH, so there could be some suspicion there. I am looking at the _CPU_Context_Initialize we are using, which probably is mostly unmodified from stock RTEMS. The comment about the broken stack allocate functionality is concerning. I don't recall noticing it before. Nevertheless, wouldn't we see some fireworks if the stack pointer wasn't initially native word aligned?
void _CPU_Context_Initialize( Context_Control *context, void *stack_area_begin, size_t stack_area_size, uint32_t new_level, void (*entry_point)( void ), bool is_fp, void *tls_area ) { const Nios2_MPU_Configuration *mpu_config = _Nios2_MPU_Get_configuration(); uint32_t stack = (uint32_t) stack_area_begin + stack_area_size - 4; memset(context, 0, sizeof(*context)); context->fp = stack; context->sp = stack; context->ra = (uint32_t) entry_point; /* * must init status field in the context before * calling _NIOS2_CPU_context_isr_set_level */ context->status = NIOS2_STATUS_PIE; _NIOS2_CPU_context_isr_set_level ( context, new_level ); if ( mpu_config != NULL ) { Nios2_MPU_Region_descriptor desc = { .index = mpu_config->data_index_for_stack_protection, /* FIXME: Brocken stack allocator */ .base = (void *) ((int) stack_area_begin & ~((1 << mpu_config->data_region_size_log2) - 1)), .end = (char *) stack_area_begin + stack_area_size, .perm = NIOS2_MPU_DATA_PERM_SVR_READWRITE_USER_NONE, .data = true, .cacheable = mpu_config->enable_data_cache_for_stack, .read = false, .write = true }; bool ok = _Nios2_MPU_Setup_region_registers( mpu_config, &desc, &context->stack_mpubase, &context->stack_mpuacc ); if ( !ok ) { /* The task stack allocator must ensure that the stack area is valid */ _Terminate( INTERNAL_ERROR_CORE, false, 0xdeadbeef ); } } }
comment:4 Changed on 07/25/19 at 00:27:27 by Jeffrey Hill
I suppose that if 4 bytes _is_ the correct alignment then buf could be array of native words, as a less unsightly fix.
The harvard risc nios2 is passing parameters in registers so perhaps gcc has less stringent stack alignment constraints?
comment:5 Changed on 07/25/19 at 01:48:40 by Chris Johns
Component: | network/libbsd → network/legacy |
---|
comment:6 Changed on 07/25/19 at 20:00:43 by Joel Sherrill
Per https://www.intel.com/content/dam/www/programmable/us/en/pdfs/literature/hb/nios2/n2cpu_nii51016.pdf it looks like the stack only requires 32-bit alignment. If RTEMS meets that, then the only solution is to somehow properly align the buffer.
comment:7 Changed on 07/31/19 at 19:32:12 by Gedare Bloom
Probably the 'buf' should be aligned to the structure alignment since it gets cast to HEADER struct. You can at least use CPU_STRUCTURE_ALIGNMENT macro, or else create a union, something like this might work
union { struct { HEADER header; u_char payload[RTEMS_ZERO_LENGTH_ARRAY]; }; u_char buffer[MAXPACKET]; } x; u_char *buf = x.buffer;
Are you compiling with any type of strict alignment flags?
Is the stack itself properly aligned initially?
I agree that it's an ugly solution but it is clear that you can't assume proper alignment of a byte buffer without either a union, compiler magic, or an attribute. A union requires more modification of the software I think. RTEMS has this macro to help avoid compiler dependencies. I am almost OK with using it and the CPU_ALIGNMENT constant as the alignment. Are we sure 4 is a portable answer. What's the first field of HEADER?
include/rtems/score/basedefs.h: #define RTEMS_ALIGNED( _alignment ) attribute((aligned(_alignment)))