Skip to content

[release/8.0-staging] Fix thread safety in SafeEvpPKeyHandle.DuplicateHandle#124618

Open
bartonjs wants to merge 2 commits intodotnet:release/8.0-stagingfrom
bartonjs:124485_80
Open

[release/8.0-staging] Fix thread safety in SafeEvpPKeyHandle.DuplicateHandle#124618
bartonjs wants to merge 2 commits intodotnet:release/8.0-stagingfrom
bartonjs:124485_80

Conversation

@bartonjs
Copy link
Member

Manual backport of #124485 to release/8.0-staging

Customer Impact

  • Customer reported
  • Found internally

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

  • Yes
  • No

Testing

A dedicated test is added with this change.

Risk

Low. DangerousAddRef/DangerousRelease is a standard treatment for SafeHandle atomicity.

@bartonjs bartonjs self-assigned this Feb 19, 2026
Copilot AI review requested due to automatic review settings February 19, 2026 19:52
@bartonjs bartonjs added Servicing-consider Issue for next servicing release review area-System.Security labels Feb 19, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 DuplicateHandle with DangerousAddRef/DangerousRelease to prevent concurrent Dispose from zeroing the handle field during duplication
  • Adds a regression test that verifies concurrent DuplicateHandle and Dispose calls 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

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @bartonjs, @vcsjones, @dotnet/area-system-security
See info in area-owners.md if you want to be subscribed.

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

Labels

area-System.Security Servicing-consider Issue for next servicing release review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments