#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 Jul 24, 2019 at 10:00:11 PM by Joel Sherrill

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

comment:2 Changed on Jul 24, 2019 at 10:38:11 PM by Jeffrey Hill

Component: adminnetwork/libbsd

comment:3 Changed on Jul 25, 2019 at 12:13:25 AM 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 Jul 25, 2019 at 12:27:27 AM 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 Jul 25, 2019 at 1:48:40 AM by Chris Johns

Component: network/libbsdnetwork/legacy

comment:6 Changed on Jul 25, 2019 at 8:00:43 PM 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 Jul 31, 2019 at 7:32:12 PM 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;
Note: See TracTickets for help on using tickets.