Tests: verify that CRYPTOPP_ALIGN_DATA delivers the requested alignment - #1844
Open
bernardladenthin wants to merge 1 commit into
Open
Tests: verify that CRYPTOPP_ALIGN_DATA delivers the requested alignment#1844bernardladenthin wants to merge 1 commit into
bernardladenthin wants to merge 1 commit into
Conversation
The macro expands to nothing for compilers that are neither MSVC nor GCC/Clang, and the SIMD backends issue aligned loads against buffers declared with it, so a silent expansion to nothing would fault at run time. Nothing verified this so far. Add TC_IS_ALIGNED next to the existing alignment constants and check both of them from the platform self-test suites. The probe buffers stay below the alignment under test because the ABI already aligns larger objects by itself, which would make the check pass vacuously. The check is skipped in the Windows driver: a failing self-test there reaches TC_BUG_CHECK and would bugcheck the machine.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an alignment check to both platform self-test suites:
src/Volume/EncryptionTest.cpp—EncryptionTest::TestAlignment(), called fromTestAll()src/Common/Tests.c—TestBufferAlignment(), called fromDoAutoTestAlgorithms()src/Common/Volumes.h—TC_IS_ALIGNED(address, alignment), next to the existing alignment constantsWhy
Around 79 declarations across the code base rely on
CRYPTOPP_ALIGN_DATA, and the SIMD backends issue aligned loads (_mm_load_si128/_mm_store_si128, ~66 sites) against buffers declared with it. If the macro ever expands to nothing — it has an empty fallback branch for compilers that are neither MSVC nor GCC/Clang — those loads fault. In kernel mode that is a bugcheck.Nothing verified this so far.
Design notes
Probe size. The probe buffers are deliberately smaller than the alignment under test. The x86-64 ABI already aligns objects of 16 bytes or more on its own, so a large probe would pass even with the macro removed, making the test vacuous. This was measured, not assumed:
Driver excluded. The check is skipped under
TC_WINDOWS_DRIVER. A failing self-test in a boot-start driver reachesTC_BUG_CHECK(Ntdriver.c), which would leave a system-encrypted machine unbootable — too high a price for a check whose failure mode the SIMD code would surface anyway. The guard wraps both the call and the function definition, the latter because an unreferenced static function trips C4505 and the driver builds withTreatWarningAsError.Verification
The test was confirmed to actually fail when the macro is broken. Neutering
CRYPTOPP_ALIGN_DATAin a scratch build produces:Unmodified, the suite passes. Additionally:
/W4and kernel-mode/W4 /WX: clean,Tests.cwarning-freeveracrypt --text --test: passLimitation
Only the Linux side runs automatically — the repository has no Windows CI job. The
Tests.chalf covers the MSVC branch of the macro but executes only when Format/Mount/ExpandVolume/FormatDLL starts.