[pycrypto] Crypto.Random crashes due to unaligned access

[Dwayne Litzenberger]

Hi Greg!

What version/build of GCC is this?  Does "setup.py test" crash for you as well?

I'd rather figure out how to fix the problem than to start making copies of the key.

Greg Price <gnprice at gmail.com> wrote:
>I get the following crash in a PyCrypto built from the current master,
>af058ee (aka v2.6.1-136-gaf058ee):
>
>>>> import Crypto.Random
>>>> Crypto.Random.new().read(1)
>Segmentation fault (core dumped)
>
>This is on i686.  I compiled with GCC 4.6.3 (or "Ubuntu/Linaro
>4.6.3-1ubuntu5".)
>
>GDB shows the crash is here:
>
>Program received signal SIGSEGV, Segmentation fault.
>aes_key_setup_enc (keylen=32, cipherKey=
>0x841b1bc
>"L\fB2\244\225\235\206^\242\305\305b\201\200\335ņ{d\240\343\262;m\361\243\276u~\337&",
>rk=
>    0x84900a8) at src/AESNI.c:122
>122            rk[0] = _mm_loadu_si128((const __m128i*) cipherKey);
>
>at which the instruction is
>
>(gdb) x/i $pc
>=> 0xb78f2600 <ALGnew+2160>: movdqa %xmm0,0x40(%esi)
>
>This is an aligned store.  The documentation of MOVDQA says it should
>be 16-byte aligned.  The value of rk (aka %esi + 0x40) is only 8-byte
>aligned:
>
>(gdb) p rk
>$5 = (__m128i *) 0x84900a8
>(gdb) p/x $esi
>$9 = 0x8490068
>
>It's not clear to me why GCC generated an aligned instruction here --
>in fact, the definition of _mm_loadu_si128 in my emmintrin.h appears
>to be
>
>extern __inline __m128i __attribute__((__gnu_inline__,
>__always_inline__, __artificial__))
>_mm_loadu_si128 (__m128i const *__P)
>{
>  return (__m128i) __builtin_ia32_loaddqu ((char const *)__P);
>}
>
>and the name of that builtin sure sounds more like MOVDQU than MOVDQA.
> Perhaps GCC somehow decides that it can prove the pointer is aligned
>here.
>
>I don't know why GCC makes this mistake, or (since it's never the
>compiler's fault) which code is lying to it about something being
>aligned. Anyone know how to investigate this kind of question?
>
>A workaround would be to make sure that the cipherKey argument to
>aes_key_setup_enc() in src/AESNI.c is always 16-byte aligned.  At
>present, that argument comes straight from the first Python-level
>argument to _AESNI.new(); see the PyArg_ParseTupleAndKeywords() call
>in src/block_template.c.  I guess to implement this workaround we'd
>copy the key to a new, aligned buffer if it's not aligned.
>
>I can send a patch for that workaround if it seems like the best
>approach.  Happy to hear alternatives, and of course it'd be most
>satisfying if we can understand why the compiler is emitting this
>output in the first place.
>
>Greg

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.