[release/8.0-staging] Fix thread safety in SafeEvpPKeyHandle.DuplicateHandle#124618
Closed
bartonjs wants to merge 2 commits intodotnet:release/8.0-stagingfrom
Closed
[release/8.0-staging] Fix thread safety in SafeEvpPKeyHandle.DuplicateHandle#124618bartonjs wants to merge 2 commits intodotnet:release/8.0-stagingfrom
bartonjs wants to merge 2 commits intodotnet:release/8.0-stagingfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR backports a thread safety fix for SafeEvpPKeyHandle.DuplicateHandle from the main branch to release/8.0-staging. The issue occurs when DuplicateHandle and Dispose are called concurrently on the same handle, potentially resulting in a duplicated handle that tracks IntPtr.Zero instead of the actual native handle, causing memory leaks and invalid handle operations.
Changes:
- Wraps the critical section of
DuplicateHandlewithDangerousAddRef/DangerousReleaseto prevent concurrentDisposefrom zeroing the handle field during duplication - Adds a regression test that verifies concurrent
DuplicateHandleandDisposecalls don't produce invalid handles
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs | Fixes thread safety race condition in DuplicateHandle by using DangerousAddRef/DangerousRelease to prevent concurrent disposal |
| src/libraries/System.Security.Cryptography.OpenSsl/tests/SafeEvpPKeyHandleTests.cs | Adds regression test that verifies concurrent DuplicateHandle and Dispose operations don't produce invalid handles |
...s/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs
Outdated
Show resolved
Hide resolved
Contributor
|
Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security |
vcsjones
approved these changes
Feb 19, 2026
Member
Author
|
Servicing review found this to be below the bar. |
Member
Author
|
Servicing review found this to be below the bar. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Manual backport of #124485 to release/8.0-staging
Customer Impact
Parallel calls to Dispose and DuplicateHandle on SafeEvpPKeyHandle can result in a state where the native handle gets up-ref'd, but the SafeHandle returned from DuplicateHandle is tracking the zero-valued handle, so the underlying free never gets called (and interacting with the returned SafeHandle doesn't do what you want, either).
Regression
Testing
A dedicated test is added with this change.
Risk
Low. DangerousAddRef/DangerousRelease is a standard treatment for SafeHandle atomicity.