[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: coturn - bus error on armhf



On Sun, Dec 23, 2018 at 10:12:27AM +0100, John Paul Adrian Glaubitz wrote:
> On 12/23/18 10:03 AM, wferi@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.


Reply to: