On Dec 5, 2014, at 3:12 PM, Solar Designer <[email protected]> wrote:
> On Sat, Dec 06, 2014 at 01:47:52AM +0300, Solar Designer wrote: >> Another suspect is C strict aliasing rules violations, which I think are >> present in scrypt's current -nosse and -sse code. I was able to trigger >> misbehavior in -nosse with either unusual gcc options (aggressive >> function inlining) or slight source code changes. I was not able to >> trigger misbehavior in -sse, but per my understanding the violations are >> in there. Another compiler might expose these. (I fixed both in yescrypt.) > > BTW, when you fix this, beware of icc 14.0.0 (other versions?) bug where > it sometimes preloads elements of a union into registers despite of > writes into other elements, at least when the union consists of __m128i > and uint32_t fields. union is a standard way to meet C strict aliasing > rules, yet there's this bug to trap us. Adding volatile works around > it, and also it does not show up with the accesses being in different > functions (would show up with more aggressive function inlining anyway? > not sure). > > Miscompiles are a thing. This is a reason why I think runtime self-test > of the full scrypt is desirable. To say nothing of the fact that one might want to compile on one machine and run the resulting binary on a different machine. A the risk of stating the (what should be) obvious, silent failures can be catastrophic in security code. rg
