Skip to content

Null-guard EVP_AEAD_CTX_cleanup for fork safety with shared memory#3280

Closed
WillChilds-Klein wants to merge 7 commits into
aws:mainfrom
WillChilds-Klein:fix/aead-cleanup-null-guard
Closed

Null-guard EVP_AEAD_CTX_cleanup for fork safety with shared memory#3280
WillChilds-Klein wants to merge 7 commits into
aws:mainfrom
WillChilds-Klein:fix/aead-cleanup-null-guard

Conversation

@WillChilds-Klein

@WillChilds-Klein WillChilds-Klein commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Issues:

Resolves #P425555771

Description of changes:

Summary

Applications that use fork() with MAP_SHARED memory for TLS connection state can trigger a SIGSEGV in EVP_AEAD_CTX_cleanup. This occurs because DEFINE_METHOD_FUNCTION lazily initializes AEAD vtables via CRYPTO_once into .bss storage. After fork(), each process has an independent .bss copy. If a child process initializes the vtable (by performing TLS) and stores an EVP_AEAD_CTX in shared memory, the parent process's vtable remains zeroed. When the parent calls EVP_AEAD_CTX_cleanup, it dereferences ctx->aead->cleanup
which is NULL, crashing the process.

Call-outs:

n/a

Testing:

Adds aead_fork_test, an isolated test binary that verifies EVP_AEAD_CTX_cleanup tolerates an uninitialized vtable after fork(). The test is a separate executable (like rand_isolated_test) because it requires that AEAD vtables are NOT initialized before the test runs. The FIPS power-on self-test and other tests in crypto_test trigger vtable initialization as a side effect.


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

Applications that use fork() with MAP_SHARED memory for TLS connection
state can trigger a SIGSEGV in EVP_AEAD_CTX_cleanup. This occurs because
DEFINE_METHOD_FUNCTION lazily initializes AEAD vtables via CRYPTO_once
into .bss storage. After fork(), each process has an independent .bss
copy. If a child process initializes the vtable (by performing TLS) and
stores an EVP_AEAD_CTX in shared memory, the parent process's vtable
remains zeroed. When the parent calls EVP_AEAD_CTX_cleanup, it
dereferences ctx->aead->cleanup which is NULL, crashing the process.

This commit adds a null-check on ctx->aead->cleanup before calling it.
This is safe because:
- All AES-GCM cleanup functions are no-ops (empty function body)
- aead_chacha20_poly1305_cleanup is also a no-op
- In the cross-process cleanup scenario, skipping cleanup prevents a
  crash with no resource leak (the shared memory pool manages lifetime)

Also adds aead_fork_test, an isolated test binary that verifies
EVP_AEAD_CTX_cleanup tolerates an uninitialized vtable after fork().

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread crypto/cipher_extra/aead_fork_test.cc Outdated
Comment thread crypto/cipher_extra/aead_fork_test.cc Outdated
Comment thread crypto/cipher_extra/aead_fork_test.cc Outdated
@codecov-commenter

codecov-commenter commented Jun 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.12821% with 35 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.15%. Comparing base (5df320b) to head (fed6775).

Files with missing lines Patch % Lines
crypto/cipher_extra/aead_fork_test.cc 55.12% 31 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3280      +/-   ##
==========================================
- Coverage   78.16%   78.15%   -0.02%     
==========================================
  Files         693      694       +1     
  Lines      123955   124033      +78     
  Branches    17213    17217       +4     
==========================================
+ Hits        96887    96934      +47     
- Misses      26149    26175      +26     
- Partials      919      924       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

WillChilds-Klein and others added 3 commits June 3, 2026 13:09
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@WillChilds-Klein

WillChilds-Klein commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of PR #3336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants