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(bindings): prevent temp connection free after panic #5067

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jmayclin
Copy link
Contributor

@jmayclin jmayclin commented Jan 28, 2025

Description of changes:

We should never be freeing the temporary connection in with_context, even during an unwind. To prevent this make the connection ManuallyDrop during initialization.

Secondly, with_context may be invoked concurrently, so we should use context not context_mut.
Moved to #5701

Testing:

I added a unit test which confirms that the connection is not freed during a panic. I confirmed that the old behavior causes this test to fail.

~/workspace/s2n-tls/bindings/rust/extended$ cargo test panic_does_not_free_connection -- --nocapture

running 1 test
thread 'callbacks::tests::panic_does_not_free_connection' panicked at 'a panic in customer code', s2n-tls/src/callbacks.rs:121:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'callbacks::tests::panic_does_not_free_connection' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `2`', s2n-tls/src/callbacks.rs:141:9
error: test failed, to rerun pass '-p s2n-tls --lib'

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 Jan 28, 2025
@jmayclin jmayclin requested a review from lrstewart January 30, 2025 00:26
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