Repository navigation
Conversation
SHA-2, BLAKE2s, Twofish and Kuznyechik read and write words in caller buffers directly on little-endian hosts, Streebog's stage2() reads 64-bit words from the input, and the Unix XTS code XORs whitening values through uint64 pointers. On strict-alignment CPUs (ARMv5-v7 for LDRD/LDM, MIPS, mips64) this raises SIGBUS when a caller passes an unaligned buffer; real volume operations use aligned buffers, but test vectors and future callers need not. - SHA-2, BLAKE2s, Twofish and Kuznyechik use the VcLoad*/VcStore* helpers on all hosts, not only on big-endian ones. - On GCC/Clang little-endian targets the helpers copy with memcpy, which compiles to single loads and stores where unaligned access is allowed, so the C paths keep their speed. MSVC targets keep native accesses, as before. - Streebog takes the direct path only for 8-byte-aligned input; other input goes through the existing copy into CTX->buffer. - XTS (Unix, without the x64 SSE path) XORs the whitening values through memcpy. Output is unchanged: known-answer comparisons for every offset from 0 to 15 match on x86_64, armv7, mips, mipsel and mips64, and an alignment-checking (UBSan) build is clean. Signed-off-by: Ville Takio <ville+git@takio.fi>
| /* Message word i of the block at p (big-endian). p is the caller's buffer | ||
| * and may be unaligned: load it byte-wise (compilers merge this into a | ||
| * single load and byte swap where unaligned accesses are allowed). */ | ||
| #define SHA2_LOAD_BE32(p, i) VcLoadBE32((const uint8 *) (p) + 4 * (i)) | ||
| #define SHA2_LOAD_BE64(p, i) VcLoadBE64((const uint8 *) (p) + 8 * (i)) |
There was a problem hiding this comment.
Please limit this change to targets that need the alignment fix. We should keep the existing loads and byte swaps on unaffected targets, regardless of compiler. We don't need to change the established x86 and x64 paths to fix the faults on ARMv7 or MIPS.
On affected little endian targets, only the input load needs to change: read through VcLoadLE32 or VcLoadLE64, then keep the existing bswap_32 or bswap_64. The alignment problem doesn't require removing the byte swaps. The existing big endian path can stay as it is.
There's a measurable cost to making this change everywhere. With MSVC 19.29 and 19.44 using _UEFI and /O2, my local SHA512 measurements showed about 2% to 5% more time per block on x64, and about 4% on x86. Clang 21.1 targeting x86 with -O2 also showed about 4% more time for the portable SHA256 and SHA512 transforms. These measurements used aligned input.
Even keeping the swaps around the new helpers still left about a 2.4% slowdown in the x64 SHA512 transform with MSVC 19.44. I'd preserve the original expressions on unaffected targets instead of relying on every compiler to recover the previous code.
Please make sure the target condition accounts for 64 bit accesses. ARMv7's support for some unaligned word loads doesn't make these 64 bit loads safe. If the condition uses CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS, it needs the correction from #1908 and must also account for MSVC ARM64, which that macro doesn't currently recognize.
VeraCrypt DCS EFI bootloader compiles this file and selects the portable SHA implementations through _UEFI, so this affects code used during password derivation. Please narrow the description's bootloader exclusion to the legacy BIOS build.
2 participants
Follow-up to #1908, which fixed Camellia and Whirlpool on 32-bit ARM through
CRYPTOPP_ALLOW_UNALIGNED_DATA_ACCESS. While checking whether anything else has the same problem, I ran every cipher, hash, PBKDF2 and XTS on buffers shifted by 0 to 15 bytes under qemu-user (which, like hardware without kernel fixups, faults on unaligned accesses). These still did unaligned word accesses on strict-alignment CPUs:EncryptionModeXTS.cpp, Unix)stage2)Real volume operations are not affected: headers, salts, keys, passwords and FUSE sector buffers are all malloc- or page-aligned, and Linux fixes up such accesses on ARMv7 and MIPS hardware (slowly). But the self-tests already hit this once (#1908), and any caller with an odd offset would.
Changes:
VcLoad*/VcStore*helpers (from Crypto: fix portable C code on big-endian CPUs #1899) on all hosts, not only on big-endian ones.memcpy, which compiles to single loads/stores where unaligned access is allowed, so the portable C paths keep their speed (byte-wise stores cost up to 10% in Twofish/Kuznyechik). MSVC targets keep native accesses as before; the Windows boot loader does not compile any of the changed code (TC_MINIMIZE_CODE_SIZE).CTX->buffer.memcpy.Testing:
-fsanitize=alignmentbuild on x86_64 is clean (master: first error at the BLAKE2s output store).veracrypt --text --testpasses on those architectures, and the x86_64 object code of the XTS file is unchanged.