SHA256_MB_JOB_MGR::lens lacks alignment, causes access violation

[gh-andre]

Flushing submitted jobs when computing SHA256 ends up calling sha256_mb_mgr_flush_avx2 on my computer, which internally uses instructions like these:

00007FF6EA656D2C C5 F9 6F 81 80 02 00 00 vmovdqa     xmm0,xmmword ptr [rcx+280h]  
00007FF6EA656D34 C5 F9 6F 89 90 02 00 00 vmovdqa     xmm1,xmmword ptr [rcx+290h]  

After some debugging, I can see that the top one loads SHA256_MB_JOB_MGR::lens, which may end up placed at the address like 0x000002e9afbe1688, which is not 16-bytes aligned, so Visual Studio reports a cryptic error like this:

Exception thrown at 0x00007FF6EA656D2C in fit.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF.

I was able to work it around in my code by explicitly aligning SHA256_HASH_CTX_MGR within the structure holding it, which only works because SHA256_MB_ARGS_X16 just happens to be 640 bytes and naturally props SHA256_MB_JOB_MGR::lens to a 16/64-byte boundary, but this will likely break as these structures change going forward.

All data members participating in SSE/AVX operations, such as SHA256_MB_JOB_MGR::lens, should be aligned within their data structures explicitly and I only see result_digest aligned for SHA256, for example.

[pablodelara]

Hi! Thanks for flagging this issue. I will look into it shortly.

[gh-andre]

Thank you. In my case it was happening in some threads and not others, so it might be tricky to reproduce this with one thread, although you can probably just force it to see the effect. Let me know if you need an app that blows up most of the time if alignment is removed.

[pablodelara]

As you can see in the tests, SHA256_HASH_CTX_MGR memory is always aligned to 16 bytes, so as long as this is done, there shouldn't be any alignment issues. I will add a comment about it in the documentation to make it clear.

[gh-andre]

This assumes that SHA256_MB_ARGS_X16 declared prior to lens it will always prop lens to the correct alignment boundary. If you go the commenting route, perhaps new comments should mention alignment requirements around all data members participating in SSE/AVX instructions, such as lens, not only for the context manager.

[pablodelara]

Right, I am also working on adding alignment within the structure.

[gh-andre]

Thank you for your work. isa-l_crypto is a fantastic library.