dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

GetCurrentProcess should return valid handle #302

Closed NN--- closed 1 year ago

NN--- commented 7 years ago

Kernel32.GetCurrentProces returns SafeObjectHandle which treates '-1' as invalid:

    public override bool IsInvalid => this.handle == INVALID_HANDLE_VALUE || this.handle == IntPtr.Zero;

GetCurrentProcess doesn't return a real handle, it returns pseudo handle with value of '-1' .

The fact 'Kernel32.GetCurrentProcess().IsInvalid == false' should be true.

AArnott commented 7 years ago

Interesting case. Not sure what to do about it. -1 isn't a valid handle anywhere else, so we can't change SafeObjectHandle. And we can't change the handle type that GetCurrentProcess returns without it being a breaking change.

vbfox commented 7 years ago

Maybe a possibility is to have GetCurrentProcess be an helper function that call DuplicateHandle after the real GetCurrentProcess to get the non-pseudo value.

NN--- commented 7 years ago

In my project I have SafeObjectHandle which receives Boolean for treating -1 as invalid and it is true by default.

In SafeProcessHandle derived class I pass "false" value to treat -1 as valid.

NN--- commented 7 years ago

This is my implementation of SafeObjectHandle <- SafeWaitHandle <- SafeProcessHandle Other wait handles such as SafeThreadHandle just derive from SafeWaitHandle passing default values.

[SecurityCritical]
public abstract class SafeObjectHandle : SafeHandle
{
    private readonly bool minusOneIsInvalid;

    protected SafeObjectHandle() : this(true, IntPtr.Zero, true)
    {
    }

    protected SafeObjectHandle(bool minusOneIsInvalid, IntPtr existingHandle, bool ownsHandle)
        : base(IntPtr.Zero, ownsHandle)
    {
        this.minusOneIsInvalid = minusOneIsInvalid;

        SetHandle(existingHandle);
    }

    [SecurityCritical]
    protected override bool ReleaseHandle()
    {
        // Do not call CloseHandle on invalid handle value.
        if (handle != Kernel32.INVALID_HANDLE_VALUE)
        {
            return Kernel32.CloseHandle(handle);
        }
        else
        {
            return true;
        }
    }

    public override bool IsInvalid
    {
        [SecurityCritical]
        get
        {
            if (minusOneIsInvalid)
            {
                return handle == IntPtr.Zero || handle == Kernel32.INVALID_HANDLE_VALUE;
            }
            else
            {
                return handle == IntPtr.Zero;
            }
        }
    }
}

Next is SafeWaitHandle which takes care of -1:

[SecurityCritical] 
public abstract class SafeWaitableHandle : SafeObjectHandle
{
    protected SafeWaitableHandle(): this(true, IntPtr.Zero, true)
    {
    }

    protected SafeWaitableHandle(bool minusOneIsInvalid, IntPtr existingHandle, bool ownsHandle) 
        : base(minusOneIsInvalid, existingHandle, ownsHandle)
    {
    }
}

And finally SafeProcessHandle which just passes 'false' in first argument for minusOneIsInvalid:

[SecurityCritical]
public class SafeProcessHandle : SafeWaitableHandle
{
    public SafeProcessHandle() : this(IntPtr.Zero)
    {
    }

    public SafeProcessHandle(IntPtr existingHandle) : this(existingHandle, true)
    {
    }

    public SafeProcessHandle(IntPtr existingHandle, bool ownsHandle) : base(false, existingHandle, ownsHandle)
    {
    }
}

Sorry, I don't have time for PR but hope the idea is clear and you can implement it.

NN--- commented 7 years ago

@AArnott I started making PR for this, and I think I miss the meaning of SafeObjectHandle in PInvoke library. In my project I used SafeObjectHandle as a base class for other safe handles to reuse code, Since, here it doesn't have this function, I suspect that this class is unnecessary and specific safe handles should be introduces, SafeFileHandle, SafeTokenHandle and so on.

AArnott commented 7 years ago

It looks like fixing this will require a SafeObjectHandle-derived type that overrides IsInvalid so that it allows -1.

fraser125 commented 6 years ago

I would add that CreateFile also can return -1 in certain cases. The scenario I experienced was an improper serial port name ("\\.\COM3", ....) is correct I tried with (@"\\.\COM3", ....) and got the -1 error.

ghost commented 6 years ago

BTW DPI_AWARENESS_CONTEXT handles can also be valid with values -1, -2, -3 etc.

NN--- commented 6 years ago

@vatsanm -2 is not a problem. The issue is solely with -1.