Search code examples
c#pinvoke

Why would I need `DangerousAddRef`/`DangerousRelease` around `DangerousGetHandle`?


I've been looking into the Microsoft.Windows.CsWin32 NuGet package and comparing the P/Invoke code it generates to what I would write by hand. In some cases it uses SafeHandle.DangerousAddRef/SafeHandle.DangerousRelease when I wouldn't have. I'm trying to understand why, and haven't come up with anything.

Here's an example for the Win32 SetInformationJobObject function:

[DllImport("KERNEL32.dll", ExactSpelling = true, SetLastError = true)]
[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
[SupportedOSPlatform("windows5.1.2600")]
internal static extern unsafe winmdroot.Foundation.BOOL SetInformationJobObject(winmdroot.Foundation.HANDLE hJob, winmdroot.System.JobObjects.JOBOBJECTINFOCLASS JobObjectInformationClass, void* lpJobObjectInformation, uint cbJobObjectInformationLength);

/// <inheritdoc cref="AssignProcessToJobObject(winmdroot.Foundation.HANDLE, winmdroot.Foundation.HANDLE)"/>
[SupportedOSPlatform("windows5.1.2600")]
internal static unsafe winmdroot.Foundation.BOOL AssignProcessToJobObject(SafeHandle hJob, SafeHandle hProcess)
{
    bool hJobAddRef = false;
    bool hProcessAddRef = false;
    try
    {
        winmdroot.Foundation.HANDLE hJobLocal;
        if (hJob is object)
        {
            hJob.DangerousAddRef(ref hJobAddRef);
            hJobLocal = (winmdroot.Foundation.HANDLE)hJob.DangerousGetHandle();  // <-- THIS
        }
        else
            throw new ArgumentNullException(nameof(hJob));
        winmdroot.Foundation.HANDLE hProcessLocal;
        if (hProcess is object)
        {
            hProcess.DangerousAddRef(ref hProcessAddRef);
            hProcessLocal = (winmdroot.Foundation.HANDLE)hProcess.DangerousGetHandle();
        }
        else
            throw new ArgumentNullException(nameof(hProcess));
        winmdroot.Foundation.BOOL __result = PInvoke.AssignProcessToJobObject(hJobLocal, hProcessLocal); // calls the `extern` overload
        return __result;
    }
    finally
    {
        if (hJobAddRef)
            hJob.DangerousRelease();
        if (hProcessAddRef)
            hProcess.DangerousRelease();
    }
}

I expected the second function to something like:

[DllImport("KERNEL32.dll", ExactSpelling = true, SetLastError = true)]
[SupportedOSPlatform("windows5.1.2600")]
internal static unsafe winmdroot.Foundation.BOOL AssignProcessToJobObject(SafeHandle hJob, SafeHandle hProcess)
{
    var localJob = (winmdroot.Foundation.HANDLE)hJob.DangerousGetHandle();
    var localProcess = (winmdroot.Foundation.HANDLE)hProcess.DangerousGetHandle();
    return (winmdroot.Foundation.BOOL)PInvoke.AssignProcessToJobObject(localJob, localProcess);
}

It seems to be making a defensive copy of the handle, but I don't see how that would help anything. Even if the handle held by the SafeHandle changes—which AFAIK it shouldn't—we'd still be passing some handle into the extern overload.

I tried checking out the source generator's code for hints, and found the commit that added the calls, but that doesn't tell me. I'm assuming that it was based on what the marshaler would have done with a SafeHandle.

Any ideas on why we need to increase the reference count here?


Solution

  • From the official docs:

    SafeHandle.DangerousGetHandle

    Using the DangerousGetHandle method can pose security risks because, if the handle has been marked as invalid with SetHandleAsInvalid, DangerousGetHandle still returns the original, potentially stale handle value. The returned handle can also be recycled at any point. At best, this means the handle might suddenly stop working. At worst, if the handle or the resource that the handle represents is exposed to untrusted code, this can lead to a recycling security attack on the reused or returned handle. For example, an untrusted caller can query data on the handle just returned and receive information for an entirely unrelated resource.

    [emphasis mine:] See the DangerousAddRef and the DangerousRelease methods for more information about using the DangerousGetHandle method safely.

    SafeHandle.DangerousAddRef

    The DangerousAddRef method prevents the common language runtime from reclaiming memory used by a handle (which occurs when the runtime calls the ReleaseHandle method). You can use this method to manually increment the reference count on a SafeHandle instance. [...].

    So this seems to be to make sure that the handle doesn't get invalidated while you use it.