intel / tinycrypt

tinycrypt is a library of cryptographic algorithms with a focus on small, simple implementation.
Other
446 stars 156 forks source link

GNUisms in ecc_dh.h #32

Closed EtchedPixels closed 5 years ago

EtchedPixels commented 6 years ago

The rest of the code goes to great pains to use _set() but ecc_dh instead dives into GNUisms, not only that but it dives into non archicture portable GNUisms.

mped-oticon commented 6 years ago

@EtchedPixels You must mean the asm volatile lines following memset, right?

mped-oticon commented 6 years ago

To explain: The 3 memsets are supposed to clear the stack because p2, tmp and _private contain sensitive information. The function is about to return so the function called next could simply read the old stack contents -- hence the need to memset. However, GCC is intelligent enough to realize that functionally the function is about to return and nobody will need p2, tmp and _private, so why bother clearing them and thus wants to optimize the memset away. Hence the need for the memory barrier.

mped-oticon commented 6 years ago

The right solution is to do an abstraction: replace memset+asm with 1 call to a secure_memset. This function is then allowed to be platform-specific.

EtchedPixels commented 6 years ago

Yes - agreed and it's the only file that hasn't done this. I noticed it because I was trying (and failing) to get tinycrypt to run on a 16bit microcontroller.