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

False positives in constant time tests using MSan with Clang 16 #9921

Open
gilles-peskine-arm opened this issue Jan 22, 2025 · 0 comments · May be fixed by #9942 or Mbed-TLS/TF-PSA-Crypto#170
Open

False positives in constant time tests using MSan with Clang 16 #9921

gilles-peskine-arm opened this issue Jan 22, 2025 · 0 comments · May be fixed by #9942 or Mbed-TLS/TF-PSA-Crypto#170
Labels
bug component-test Test framework and CI scripts size-xs Estimated task size: extra small (a few hours at most)

Comments

@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Jan 22, 2025

As discovered by @davidhorstmann-arm, our constant-flow testing leveraging MSan (MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN) fails on Clang 16 and above.

This is due to a change in the memory sanitizer. It 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.

The fix is to pass the extra command line option -fno-sanitize-memory-param-retval when building with MBEDTLS_TEST_CONSTANT_FLOW_MEMSAN with Clang ≥16.

This is currently not a problem on the CI because we use an older image. But it tends to come up on typical developer machines nowadays, and it's one of many things that we'll need to fix before we upgrade our CI images.

@gilles-peskine-arm gilles-peskine-arm added bug component-test Test framework and CI scripts size-xs Estimated task size: extra small (a few hours at most) labels Jan 22, 2025
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jan 22, 2025
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 Mbed-TLS#9921

Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jan 22, 2025
Support Clang >=16 in component_test_memsan_constant_flow and
component_test_memsan_constant_flow_psa. Modern versions of Clang require an
extra compiler option to avoid false positives.

Fixes Mbed-TLS#9921.

Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jan 29, 2025
Support Clang >=16 in component_test_memsan_constant_flow and
component_test_memsan_constant_flow_psa. Modern versions of Clang require an
extra compiler option to avoid false positives.

Fixes Mbed-TLS#9921.

Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jan 30, 2025
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 Mbed-TLS#9921

Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm added a commit to gilles-peskine-arm/mbedtls that referenced this issue Jan 30, 2025
Support Clang >=16 in component_test_memsan_constant_flow. Modern versions
of Clang require an extra compiler option to avoid false positives.

Fixes Mbed-TLS#9921.

Signed-off-by: Gilles Peskine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug component-test Test framework and CI scripts size-xs Estimated task size: extra small (a few hours at most)
Projects
Status: No status
1 participant