[pycrypto] Crypto.Random crashes due to unaligned access

[Paul_Koning at Dell.com]

On Oct 24, 2013, at 4:10 PM, Andrew Cooper <andyhhp at gmail.com> wrote:

> 
> On 24/10/2013 17:59, Dwayne Litzenberger wrote:
>> 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);
>>> }
> 
> http://gcc.gnu.org/onlinedocs/gcc/X86-Built_002din-Functions.html  would
> suggest that __builtin_ia32_loaddqu is specified to generate movdqu
> rather than movdqa.
> 
> There is certainly nothing I can see in the code which would allow GCC
> to presume that the address is 16  byte aligned.
> 
> ~Andrew

Indeed.  You can certainly tell GCC about alignment (with the __aligned__ attribute) but that is not specified here.  The fact that the machinery is willing to generate an instruction that requires alignment, without marking the addresses in question as such, is a type error…

	paul