Unify SECURITY_ATTRIBUTES code across the codebase

[Copilot]

Description

All places in the codebase that manually initialized SECURITY_ATTRIBUTES with nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES) duplicated the same boilerplate. This adds three factory methods to the struct itself to eliminate all such duplication.

Changes

• Interop.SECURITY_ATTRIBUTES.cs: Added three factory methods:

  • Create() — sets only nLength = sizeof(SECURITY_ATTRIBUTES), for callers that need no other fields initialized.
  • Create(void* securityDescriptor) — sets nLength and lpSecurityDescriptor, for callers that provide a security descriptor.
  • Create(bool inheritable) — sets nLength and bInheritHandle, for callers that only need to control handle inheritance. The parameter is bool (not FileShare) since SECURITY_ATTRIBUTES is not file-specific.

• SafeFileHandle.Windows.cs: Replaced 9-line if/else block with SECURITY_ATTRIBUTES.Create((share & FileShare.Inheritable) != 0). Also fixes a subtle correctness issue: previously when FileShare.Inheritable was not set, the struct was still pointer-passed to CreateFile with nLength = 0, which violates the Windows API contract.

• FileSystemAclExtensions.cs: Replaced manual struct initializer with SECURITY_ATTRIBUTES.Create((share & FileShare.Inheritable) != 0). Also changed from var to explicit type per coding conventions.

• MemoryMappedFile.Windows.cs: Deleted the GetSecAttrs(HandleInheritability) helper method entirely; both call sites now call Interop.Kernel32.SECURITY_ATTRIBUTES.Create(...) directly.

• PipeStream.Windows.cs: Simplified existing GetSecAttrs helper to a one-liner using Create(bool).

• FileSystem.DirectoryCreation.Windows.cs, EventWaitHandleAcl.cs, MutexAcl.cs, SemaphoreAcl.cs, EventWaitHandle.Windows.cs, Mutex.Windows.cs, Semaphore.Windows.cs: Replaced two-line Create() + secAttrs.lpSecurityDescriptor = ... pattern with a single Create(securityDescriptor) call. Also removed unnecessary = default initializations of securityAttributes local variables (IDE0059).

• RegistryKey.cs: Simplified to call Create(pSecurityDescriptor) unconditionally inside the fixed block — the null check was unnecessary since passing null is valid.

Testing

All affected libraries (System.IO.FileSystem.AccessControl, System.IO.Pipes, System.IO.MemoryMappedFiles, System.Threading.AccessControl, Microsoft.Win32.Registry, System.Private.CoreLib) build successfully with 0 errors. All projects that link Interop.SECURITY_ATTRIBUTES.cs have AllowUnsafeBlocks=true, so the new unsafe factory methods are compatible with all consumers.

Original prompt

---

This section details on the original issue you should resolve

<issue_title>Unify SECURITY_ATTRIBUTES code from FileSystemAclExtensions and SafeFileHandle</issue_title>
<issue_description>Follow up of the PR comment where this was suggested: #61297 (comment) (but wait for that PR to be merged).

Unify these two methods:

runtime/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs

Lines 294 to 308 in 57bfe47

	fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm())
	{
	var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
	{
	nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
	bInheritHandle = ((share & FileShare.Inheritable) != 0) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE,
	lpSecurityDescriptor = (IntPtr)pSecurityDescriptor
	};
	using (DisableMediaInsertionPrompt.Create())
	{
	handle = Interop.Kernel32.CreateFile(fullPath, (int)rights, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero);
	ValidateFileHandle(handle, fullPath);
	}
	}

runtime/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs

Lines 52 to 102 in 39ece73

	private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)
	{
	Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = default;
	if ((share & FileShare.Inheritable) != 0)
	{
	secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
	{
	nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
	bInheritHandle = Interop.BOOL.TRUE
	};
	}
	int fAccess =
	((access & FileAccess.Read) == FileAccess.Read ? Interop.Kernel32.GenericOperations.GENERIC_READ : 0) |
	((access & FileAccess.Write) == FileAccess.Write ? Interop.Kernel32.GenericOperations.GENERIC_WRITE : 0);
	// Our Inheritable bit was stolen from Windows, but should be set in
	// the security attributes class. Don't leave this bit set.
	share &= ~FileShare.Inheritable;
	// Must use a valid Win32 constant here...
	if (mode == FileMode.Append)
	{
	mode = FileMode.OpenOrCreate;
	}
	int flagsAndAttributes = (int)options;
	// For mitigating local elevation of privilege attack through named pipes
	// make sure we always call CreateFile with SECURITY_ANONYMOUS so that the
	// named pipe server can't impersonate a high privileged client security context
	// (note that this is the effective default on CreateFile2)
	flagsAndAttributes |= (Interop.Kernel32.SecurityOptions.SECURITY_SQOS_PRESENT | Interop.Kernel32.SecurityOptions.SECURITY_ANONYMOUS);
	SafeFileHandle fileHandle = Interop.Kernel32.CreateFile(fullPath, fAccess, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero);
	if (fileHandle.IsInvalid)
	{
	// Return a meaningful exception with the full path.
	// NT5 oddity - when trying to open "C:\" as a Win32FileStream,
	// we usually get ERROR_PATH_NOT_FOUND from the OS. We should
	// probably be consistent w/ every other directory.
	int errorCode = Marshal.GetLastPInvokeError();
	if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath!.Length == PathInternal.GetRootLength(fullPath))
	{
	errorCode = Interop.Errors.ERROR_ACCESS_DENIED;
	}
	throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath);
	}

Since they are both very similar.

cc @adamsitnik @jozkee </issue_description>

Comments on the Issue (you are @copilot in this section)

@ Tagging subscribers to this area: @dotnet/area-system-io See info in [area-owners.md](https://github.com/dotnet/runtime/blob/main/docs/area-owners.md) if you want to be subscribed.

Issue Details

---

Follow up of the PR comment where this was suggested: #61297 (comment)

Unify these two methods:

runtime/src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs

Lines 294 to 308 in 57bfe47

	fixed (byte* pSecurityDescriptor = security.GetSecurityDescriptorBinaryForm())
	{
	var secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
	{
	nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
	bInheritHandle = ((share & FileShare.Inheritable) != 0) ? Interop.BOOL.TRUE : Interop.BOOL.FALSE,
	lpSecurityDescriptor = (IntPtr)pSecurityDescriptor
	};
	using (DisableMediaInsertionPrompt.Create())
	{
	handle = Interop.Kernel32.CreateFile(fullPath, (int)rights, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero);
	ValidateFileHandle(handle, fullPath);
	}
	}

runtime/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs

Lines 52 to 102 in 39ece73

	private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options)
	{
	Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = default;
	if ((share & FileShare.Inheritable) != 0)
	{
	secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES
	{
	nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES),
	bInheritHandle = Interop.BOOL.TRUE
	};
	}
	int fAccess =
	((access & FileAccess.Read) == FileAccess.Read ? Interop.Kernel32.GenericOperations.GENERIC_READ : 0) |
	((access & FileAccess.Write) == FileAccess.Write ? Interop.Kernel32.GenericOperations.GENERIC_WRITE : 0);
	// Our Inheritable bit was stolen from Windows, but should be set in
	// the security attributes class. Don't leave this bit set.
	share &= ~FileShare.Inheritable;
	// Must use a valid Win32 constant here...
	if (mode == FileMode.Append)
	{
	mode = FileMode.OpenOrCreate;
	}
	int flagsAndAttributes = (int)options;
	// For mitigating local elevation of privilege attack through named pipes
	// make sure we always call CreateFile with SECURITY_ANONYMOUS so that the
	// named pipe server can't impersonate a high privileged client security context
	// (note that this is the effective default on CreateFile2)
	flagsAndAttributes |= (Interop.Kernel32.SecurityOptions.SECURITY_SQOS_PRESENT | Interop.Kernel32.SecurityOptions.SECURITY_ANONYMOUS);
	SafeFileHandle fileHandle = Interop.Kernel32.CreateFile(fullPath, fAccess, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero);
	if (fileHandle.IsInvalid)
	{
	// Return a meaningful exception with the full path.
	// NT5 oddity - when trying to open "C:\" as a Win32FileStream,
	// we usually get ERROR_PATH_NOT_FOUND from the OS. We should
	// probably be consistent w/ every other directory.
	int errorCode = Marshal.GetLastPInvokeError();
	if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath!.Length == PathInternal.GetRootLength(fullPath))
	{
	errorCode = Interop.Errors.ERROR_ACCESS_DENIED;
	}
	throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath);
	}

Since they are both very similar.

cc @adamsitnik @jozkee

Author:	carlossanlop
Assignees:	-
Labels:	area-System.IO
Milestone:	Future

• Fixes Unify SECURITY_ATTRIBUTES code from FileSystemAclExtensions and SafeFileHandle #64017

---

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

[danmoseley]

@copilot address feedback

[danmoseley]

@copilot do more of your thing ...address feedback

[adamsitnik]

@copilot fix following build failures:

##[error]src\libraries\System.Private.CoreLib\src\System\Threading\Semaphore.Windows.cs(58,50): error IDE0059: (NETCORE_ENGINEERING_TELEMETRY=Build) Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059)
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Threading\Semaphore.Windows.cs(58,50): error IDE0059: Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]
##[error]src\libraries\System.Private.CoreLib\src\System\Threading\Mutex.Windows.cs(43,50): error IDE0059: (NETCORE_ENGINEERING_TELEMETRY=Build) Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059)
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Threading\Mutex.Windows.cs(43,50): error IDE0059: Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]
##[error]src\libraries\System.Private.CoreLib\src\System\Threading\EventWaitHandle.Windows.cs(54,50): error IDE0059: (NETCORE_ENGINEERING_TELEMETRY=Build) Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059)
D:\a\_work\1\s\src\libraries\System.Private.CoreLib\src\System\Threading\EventWaitHandle.Windows.cs(54,50): error IDE0059: Unnecessary assignment of a value to 'securityAttributes' (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0059) [D:\a\_work\1\s\src\coreclr\System.Private.CoreLib\System.Private.CoreLib.csproj]

[Copilot]

@copilot fix following build failures:

Fixed in 11f93bc. Removed = default from the securityAttributes declaration in all three files — the initial default value was never observed since the variable is always assigned before use inside the if block.

[Copilot]

Pull request overview

This PR centralizes Win32 SECURITY_ATTRIBUTES initialization by introducing Interop.Kernel32.SECURITY_ATTRIBUTES.Create(...) helpers and updating multiple Windows-specific call sites to use them, reducing repeated struct setup code across threading primitives and I/O.

Changes:

• Add SECURITY_ATTRIBUTES.Create() / Create(void*) / Create(bool) factory helpers in the Kernel32 interop layer.
• Replace repeated SECURITY_ATTRIBUTES field assignments with the new helpers across wait handle, file, pipe, registry, directory, and memory-mapped file code paths.
• Remove now-redundant local helper(s) that previously built SECURITY_ATTRIBUTES.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/libraries/System.Threading.AccessControl/src/System/Threading/SemaphoreAcl.cs	Use SECURITY_ATTRIBUTES.Create(void*) when creating named semaphores with custom security.
src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs	Use SECURITY_ATTRIBUTES.Create(void*) when creating mutexes with custom security.
src/libraries/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs	Use SECURITY_ATTRIBUTES.Create(void*) when creating events with custom security.
src/libraries/System.Private.CoreLib/src/System/Threading/Semaphore.Windows.cs	Use SECURITY_ATTRIBUTES.Create(void*) for “current user only” named semaphore creation.
src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs	Use SECURITY_ATTRIBUTES.Create(void*) for “current user only” named mutex creation.
src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Windows.cs	Use SECURITY_ATTRIBUTES.Create(void*) for “current user only” named event creation.
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Use SECURITY_ATTRIBUTES.Create(bool) to control inheritability for file handles.
src/libraries/System.IO.Pipes/src/System/IO/Pipes/PipeStream.Windows.cs	Simplify inheritability SECURITY_ATTRIBUTES creation with Create(bool).
src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs	Switch to Create(bool) and remove old helper, but introduces a compile-breaking block/bracing issue.
src/libraries/System.IO.FileSystem.AccessControl/src/System/IO/FileSystemAclExtensions.cs	Use SECURITY_ATTRIBUTES.Create(bool) and continue setting descriptor when provided.
src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.cs	Use SECURITY_ATTRIBUTES.Create(void*) when creating subkeys (with optional security descriptor).
src/libraries/Common/src/System/IO/FileSystem.DirectoryCreation.Windows.cs	Use SECURITY_ATTRIBUTES.Create(void*) for directory creation security descriptor.
src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SECURITY_ATTRIBUTES.cs	Add SECURITY_ATTRIBUTES.Create(...) factory methods and remove unused using System;.

---

You can also share your feedback on Copilot code review. Take the survey.

[adamsitnik]

LGTM assuming the tests are going to pass.

[adamsitnik]

/ba-g the failure is unrelated (chrome failing to start)