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

ci: Re-enable asan and ubsan for fuzz tests #4840

Merged
merged 6 commits into from
Oct 15, 2024

Conversation

jouho
Copy link
Contributor

@jouho jouho commented Oct 11, 2024

Resolved issues:

Asan and ubsan were disabled for fuzz testing due to memory issues in CI after transitioning from the Make build system to CMake. These sanitizers are critical for detecting memory-related issues, and re-enabling them will enhance fuzz coverage.

Memory issue reproduced in this CodeBuild job

The likely cause of the memory issues was not disabling MADV_DONTDUMP as required, which is documented here:

s2n-tls/utils/s2n_mem.c

Lines 103 to 112 in 5a2fdf6

/*
** We disable MAD_DONTDUMP when fuzz-testing or using the address sanitizer because
** both need to be able to dump pages to function. It's how they map heap output.
*/
#if defined(MADV_DONTDUMP) && !defined(S2N_ADDRESS_SANITIZER) && !defined(S2N_FUZZ_TESTING)
if (madvise(*ptr, *allocated, MADV_DONTDUMP) != 0) {
POSIX_GUARD(s2n_mem_free_no_mlock_impl(*ptr, *allocated));
POSIX_BAIL(S2N_ERR_MADVISE);
}
#endif

This PR re-enables asan and ubsan for better test coverage.

Description of changes:

Add asan and ubsan option to fuzz tests, and add a compile definition for fuzz test to disable MAD_DONTDUMP

Testing:

  • s2nFuzzBatch is running without exhausting its memory.
  • successfully running scheduled fuzz batch for a reduced time (60 secs) against this change: Link to CodeBuild job

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Oct 11, 2024
@jouho jouho changed the title ci: add asan and ubsan to fuzz test ci: Re-enable asan and ubsan for fuzz tests Oct 11, 2024
@jouho jouho mentioned this pull request Oct 11, 2024
9 tasks
@jouho jouho marked this pull request as ready for review October 11, 2024 20:33
@jouho jouho requested review from jmayclin, dougch and goatgoose and removed request for jmayclin October 11, 2024 20:34
CMakeLists.txt Outdated Show resolved Hide resolved
@jouho jouho enabled auto-merge (squash) October 15, 2024 17:03
@jouho jouho merged commit 254199a into aws:main Oct 15, 2024
38 checks passed
@jouho jouho deleted the add-asan-ubsan-to-fuzz branch October 15, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants