On Sun, Dec 23, 2018 at 10:12:27AM +0100, John Paul Adrian Glaubitz wrote: > On 12/23/18 10:03 AM, wf...@niif.hu wrote: > > Thanks to everybody for their spot-on comments, and especially to Steve > > for his detailed analysis in the bug report. Meanwhile I reproduced the > > error on the sparc64 porterbox, but I couldn't really make head or tail > > of the issue at the source level, so that additional input is most > > appreciated. We'll probably have to replace all nswap64 occurences in > > the code (at least). > > I just ran the binary through gdb: > > (gdb) bt > #0 0x000001000000d0e0 in encode_oauth_token_gcm (server_name=0x7feffffebd8 > "blackdow.carleon.gov", etoken=0x7feffffe4b8, key=0x7feffffe100, > dtoken=0x7feffffdbf0, > nonce0=0x7feffffeb78 "h4j3k2l2n4b5") at src/ns_turn_defs.h:98 > #1 0x0000010000003778 in check_oauth () at > src/apps/rfc5769/rfc5769check.c:146 > #2 main (argc=<optimized out>, argv=<optimized out>) at > src/apps/rfc5769/rfc5769check.c:568 > (gdb) > > Looking at the code, it's very obvious where the unaligned access > comes from. It's in src/ns_turn_defs.h:
Have to admit it is not so obvious to me; more below. > static inline u64bits _ioa_ntoh64(u64bits v) > { > #if BYTE_ORDER == LITTLE_ENDIAN > u08bits *src = (u08bits*) &v; > u08bits* dst = src + 7; > while (src < dst) { > u08bits vdst = *dst; > *(dst--) = *src; > *(src++) = vdst; > } > #elif BYTE_ORDER == BIG_ENDIAN > /* OK */ > #else > #error WRONG BYTE_ORDER SETTING > #endif > return v; > } It's casting to a (unsigned) char pointer (assuming that is what u08bits is typedefed to) and reading and writing through that. A char can have any alignment so there is no alignment issue in this code. The code above is an arsed-about-way of doing byte reversal. On a register-based arch the code will force the argument passed in a register to be written to the stack, byte order reversed by reading and writing bytes, then re-reading from the stack to pass back the result in the return register. That's assuming the compiler isn't smart enough to work out what the the hell the programmer intends; but if it is it might simplify it down to register based logical and shift operations, thereby avoiding any memory accesses. This suggestion is somewhat concerning: > The copying here needs to be done with memcpy(). Which will fail to reverse the order of bytes and thus break the code. The code above is *not* doing a straight memory copy. > Just casting pointers > and using plain assignment is a guarantee for unaligned access. Untrue. It is not a guarantee for unaligned access. There are other conditions that must be met for the gaurantee, and I don't see why the code above---despite being written by a neophyte--- would cause unaligned accessess. Cheers, Michael.