Skip to content

Don't set security descriptor if the mutex is not owned/new - #120075

Merged
ericstj merged 1 commit into
dotnet:mainfrom
jozkee:mutex_sec
Sep 25, 2025
Merged

Don't set security descriptor if the mutex is not owned/new#120075
ericstj merged 1 commit into
dotnet:mainfrom
jozkee:mutex_sec

Conversation

@jozkee

@jozkeejozkee commented Sep 24, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Fixes#116014.
Fixes#110454.

CopilotAI review requested due to automatic review settings September 24, 2025 23:03
CopilotAI reviewed Sep 24, 2025
View reviewed changes

CopilotAI 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.

Pull Request Overview

This PR fixes a mutex access control issue by only setting security permissions when the mutex is newly created. The change prevents unnecessary access control modifications on existing mutexes.

  • Only sets mutex access control when createdNew is true
  • Adds conditional check around the existing security configuration code
  • Maintains the same security model but applies it more selectively

@github-actionsgithub-actionsBot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 24, 2025
@jozkee
jozkee requested a review from ericstjSeptember 24, 2025 23:04
@jkotasjkotas added area-System.Diagnostics.EventLog and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 24, 2025
@dotnet-policy-service

dotnet-policy-serviceBot commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-diagnostics-performancecounter
See info in area-owners.md if you want to be subscribed.

jozkee
jozkee commented Sep 25, 2025
View reviewed changes
Comment threadsrc/libraries/Common/src/System/Diagnostics/NetFrameworkUtils.cs
@jozkeejozkee changed the title Don't set access control if the mutex is not initially ownedDon't set security descriptor if the mutex is not owned/newSep 25, 2025
ericstj
ericstj approved these changes Sep 25, 2025
View reviewed changes

@ericstjericstj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As discussed - you've done manual testing involving multiple users to ensure this is fixed. I imagine we cannot cover this easily in CI.

@ericstj
ericstj merged commit bf486c8 into dotnet:mainSep 25, 2025
96 checks passed
@ericstj

ericstj commented Sep 25, 2025

Copy link
Copy Markdown
Member

/backport to release/10.0

@github-actions

github-actionsBot commented Sep 25, 2025

Copy link
Copy Markdown
Contributor

Started backporting to release/10.0: https://github.com/dotnet/runtime/actions/runs/18021213909

@github-actionsgithub-actionsBot mentioned this pull request Sep 25, 2025
Merged
4 tasks
@jozkee
jozkee deleted the mutex_sec branch September 25, 2025 22:58
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Oct 26, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@ericstjericstjericstj approved these changes

Assignees

@jozkeejozkee

Labels

area-System.Diagnostics.PerformanceCounter

Projects

None yet

Milestone

No milestone

4 participants

@jozkee@ericstj@jkotas