microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2k stars 84 forks source link

CsWin32 ignores `Out` attribute #1007

Closed zbalkan closed 10 months ago

zbalkan commented 11 months ago

Actual behavior

The signature generated by CsWin32:

public static unsafe Windows.Win32.Foundation.BOOL GetKernelObjectSecurity(
SafeHandle Handle,
uint RequestedInformation,
Windows.Win32.Security.PSECURITY_DESCRIPTOR pSecurityDescriptor,
uint nLength,
out uint lpnLengthNeeded)

The [Out] attribute of pSecurityDescriptor is ignored, which makes GetKernelObjectSecurity unusable.

Expected behavior

public static unsafe Windows.Win32.Foundation.BOOL GetKernelObjectSecurity(
SafeHandle Handle,
uint RequestedInformation,
out Windows.Win32.Security.PSECURITY_DESCRIPTOR pSecurityDescriptor,
uint nLength,
out uint lpnLengthNeeded)

The way to handle GetKernelObjectSecurity using conventional P/Invoke calls looks like this:

[DllImport("advapi32.dll", SetLastError = true)]
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
internal static extern bool GetKernelObjectSecurity(
IntPtr Handle,
int RequestedInformation,
[Out] byte[] pSecurityDescriptor,
uint nLength,
out uint lpnLengthNeeded);

The CsWin32 should look and behave similar, too.

Repro steps

  1. NativeMethods.txt content:

    GetKernelObjectSecurity
  2. NativeMethods.json content (if present):

    {
    "$schema": "https://aka.ms/CsWin32.schema.json",
    "public": true
    }
  3. Any of your own code that should be shared?

Context

AArnott commented 11 months ago

The [Out] attribute of pSecurityDescriptor is ignored, which makes GetKernelObjectSecurity unusable.

Can you help me understand why it's unusable? Have you tried it?

The [Out] modifier on the pSecurityDescriptor parameter merely indicates the party that is expected to initialize the buffer pointed to by the one field inside PSECURITY_DESCRIPTOR. That field itself is initialized by the caller, to point at an existing buffer, which the callee is expected to initialize. The [Out] modifier, while useful as a SAL annotation, doesn't change the runtime behavior at all in this case AFAIK.

It is possible that in your proposed form, [Out] is necessary because passing in byte[] requires the CLR marshaler to get involved, and the modifier allows it to know which direction to copy data. At least in some cases, that's required. But in the declaration that CsWin32 generates, the marshaler doesn't get involved so the attribute isn't needed.

AArnott commented 10 months ago

Closing due to no response.