haskell-crypto / cryptonite

lowlevel set of cryptographic primitives for haskell
Other
226 stars 139 forks source link

Fix alignment in gfmul_generic (closes #334) #348

Closed robx closed 2 years ago

robx commented 3 years ago

I believe this is the root of #334, though it would be good to get confirmation.

This fixes a test-suite segfault on Darwin with -O0. Before this change:

$ cabal run -O0 test-cryptonite -- -p AE1
Segmentation fault: 11

with

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   test-cryptonite                 0x0000000108f7f61f gfmul_generic + 47
1   test-cryptonite                 0x0000000108f76f17 ghash_add + 71
2   test-cryptonite                 0x0000000108f743b4 cryptonite_aesni_gcm_encrypt128 + 2244
3   test-cryptonite                 0x0000000108f97f20 cryptonite_aes_gcm_encrypt + 96
4   test-cryptonite                 0x0000000108eeadf5 Lc8Pq_info + 197
robx commented 3 years ago

I should add that I don't know what I'm doing here. :)

vincenthz commented 3 years ago

hmm thanks for tracking this down, although this is a bit suspicious; IIRC (and some quick glance on the internet) tells me the stack should be 16 bytes aligned by the compiler. if you can still reproduce this, could you check what instruction is triggering this behavior ? it should be just about doing a disassemble in gdb when the SEGV happens.

robx commented 3 years ago

Yes, still segfaults. lldb log below, let me know if there's anything else I can do. Looks to me like the stack pointers themselves are 16-byte aligned, but _t isn't?

(lldb) run
Process 89844 launched: '/h/cryptonite/dist-newstyle/build/x86_64-osx/ghc-8.10.4/cryptonite-0.28/t/test-cryptonite/noopt/build/test-cryptonite/test-cryptonite' (x86_64)
Process 89844 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
    frame #0: 0x00000001003ad01f test-cryptonite`gfmul_generic + 47
test-cryptonite`gfmul_generic:
->  0x1003ad01f <+47>: movdqa %xmm0, (%rax)
    0x1003ad023 <+51>: movq   -0x38(%rbp), %rsi
    0x1003ad027 <+55>: callq  0x10039ca70               ; cryptonite_aes_generic_gf_mul
    0x1003ad02c <+60>: leaq   -0x48(%rbp), %rax
Target 0: (test-cryptonite) stopped.
(lldb) register read rbp rsp rax xmm0
     rbp = 0x00007ffeefbfaa60
     rsp = 0x00007ffeefbfaa10
     rax = 0x00007ffeefbfaa18
    xmm0 = {0x7f 0x21 0x05 0x5a 0xe5 0xf8 0x1c 0x3f 0xc8 0xb1 0x10 0xdf 0x8b 0x00 0x4a 0x22}
(lldb) dis
test-cryptonite`gfmul_generic:
    0x1003acff0 <+0>:  pushq  %rbp
    0x1003acff1 <+1>:  movq   %rsp, %rbp
    0x1003acff4 <+4>:  subq   $0x50, %rsp
    0x1003acff8 <+8>:  movdqa %xmm0, -0x30(%rbp)
    0x1003acffd <+13>: movq   %rdi, -0x38(%rbp)
    0x1003ad001 <+17>: leaq   -0x48(%rbp), %rdi
    0x1003ad005 <+21>: movq   %rdi, %rax
    0x1003ad008 <+24>: movdqa -0x30(%rbp), %xmm0
    0x1003ad00d <+29>: movq   %rax, -0x10(%rbp)
    0x1003ad011 <+33>: movdqa %xmm0, -0x20(%rbp)
    0x1003ad016 <+38>: movdqa -0x20(%rbp), %xmm0
    0x1003ad01b <+43>: movq   -0x10(%rbp), %rax
->  0x1003ad01f <+47>: movdqa %xmm0, (%rax)
    0x1003ad023 <+51>: movq   -0x38(%rbp), %rsi
    0x1003ad027 <+55>: callq  0x10039ca70               ; cryptonite_aes_generic_gf_mul
    0x1003ad02c <+60>: leaq   -0x48(%rbp), %rax
    0x1003ad030 <+64>: movq   %rax, -0x8(%rbp)
    0x1003ad034 <+68>: movq   -0x8(%rbp), %rax
    0x1003ad038 <+72>: movdqa (%rax), %xmm0
    0x1003ad03c <+76>: movdqa %xmm0, -0x30(%rbp)
    0x1003ad041 <+81>: movdqa -0x30(%rbp), %xmm0
    0x1003ad046 <+86>: addq   $0x50, %rsp
    0x1003ad04a <+90>: popq   %rbp
    0x1003ad04b <+91>: retq   
    0x1003ad04c <+92>: nop    
    0x1003ad04d <+93>: nop    
    0x1003ad04e <+94>: nop    
    0x1003ad04f <+95>: nop    
(lldb) dis -s  0x10039ca70
test-cryptonite`cryptonite_aes_generic_gf_mul:
    0x10039ca70 <+0>:  pushq  %rbp
    0x10039ca71 <+1>:  movq   %rsp, %rbp
    0x10039ca74 <+4>:  subq   $0x30, %rsp
    0x10039ca78 <+8>:  movq   %rdi, -0x8(%rbp)
    0x10039ca7c <+12>: movq   %rsi, -0x10(%rbp)
    0x10039ca80 <+16>: leaq   -0x20(%rbp), %rdi
    0x10039ca84 <+20>: callq  0x10039c950               ; block128_zero
    0x10039ca89 <+25>: movl   $0xf, -0x24(%rbp)
robx commented 2 years ago

@vincenthz any chance to get this merged? I'd love to not have to keep building against a fork