newhopecrypto / newhope

Software of the NIST Post-Quantum submission NewHope
https://newhopecrypto.org
43 stars 9 forks source link

ref implementation speed test stack smashing #7

Closed kammoh closed 3 years ago

kammoh commented 4 years ago

I'm trying to measure the execution speed of the reference implementation on ARMv8. Running speed_cpakem512, speed_cpakem1024, etc I receive the following message at the end of each execution:

*** stack smashing detected ***: <unknown> terminated
Aborted (core dumped)

This happens with the original code even with just replacing the cpucycles function with a dummy return 0:

index e6803ce..ef7c391 100644
--- a/ref/cpucycles.c
+++ b/ref/cpucycles.c
@@ -2,8 +2,5 @@

 long long cpucycles(void)
 {
-  unsigned long long result;
-  asm volatile(".byte 15;.byte 49;shlq $32,%%rdx;orq %%rdx,%%rax"
-    : "=a" (result) ::  "%rdx");
-  return result;
+  return 0;
 }

It also happen with all gcc the optimization flags removed or O0. running under gdb:


decaps:  
median: 0
average: 0

*** stack smashing detected ***: <unknown> terminated

Program received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.

(gdb) where
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x0000fffff7eab8b4 in __GI_abort () at abort.c:79
#2  0x0000fffff7ee465c in __libc_message (action=do_abort, fmt=fmt@entry=0xfffff7fa46f8 "*** %s ***: %s terminated\n")
    at ../sysdeps/posix/libc_fatal.c:181
#3  0x0000fffff7f5976c in __GI___fortify_fail_abort (need_backtrace=need_backtrace@entry=false, 
    msg=msg@entry=0xfffff7fa46d0 "stack smashing detected") at fortify_fail.c:33
#4  0x0000fffff7f59720 in __stack_chk_fail () at stack_chk_fail.c:29
#5  0x0000aaaaaaaae61c in main () at speed.c:119
(gdb) 

gcc (Ubuntu/Linaro 7.5.0-3ubuntu1~18.04) 7.5.0 Ubuntu 18.04.4 LTS CPU: ARM Cortex-A53 (BCM2835, Raspberry Pi 3)

lucasperin commented 4 years ago

Hey @kammoh, I think that you could try to increase stack size, but have you tryied to set #define NTESTS 10000 to something smaller? Say 100-1000?

cryptojedi commented 3 years ago

This was a very stupid bug (reversing arguments in encaps). Should be fixed now.