Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix false positives in constant time tests using MSan with Clang 16 #170

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jan 29, 2025

Fix false positives of constant-flow testing when using MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN on Clang 16 and above. Fixes Mbed-TLS/mbedtls#9921.

This isn't needed for the CI (in the short to medium term) because still we do this testing on an old platform with an old Clang, but is needed on typical developer machines nowadays.

PR checklist

In the documentation of `MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN`, note that since
Clang 16, an extra command line option `-fsanitize-memory-param-retval` is
required.

As documented in the release notes
https://releases.llvm.org/16.0.0/tools/clang/docs/ReleaseNotes.html#sanitizers
since Clang 16, MSan forbids passing "uninitialized" values in and out of
functions. In constant-flow testing, "uninitialized" values are actually
secrets that must be manipulated with a data-independent flow, and it's
perfectly fine to pass these in and out of functions.

Fix #9921

Signed-off-by: Gilles Peskine <[email protected]>
New CMake build types CFMemSan, CFMemSanDbg to take care of differing
compiler command lines with Clang <15 and Clang >=16.

Signed-off-by: Gilles Peskine <[email protected]>
Add some basic checks of constant flow sanitizers. In particular, detect the
specific way in which Clang 16 broke our constant-flow testing (by default,
"uninitialized" values may not be passed to or returned from functions).

Signed-off-by: Gilles Peskine <[email protected]>
Copy link
Contributor

@waleed-elmelegy-arm waleed-elmelegy-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-test Test framework and CI scripts needs-review Every commit must be reviewed by at least two team members size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: In Development
Development

Successfully merging this pull request may close these issues.

False positives in constant time tests using MSan with Clang 16
2 participants