SecureArray.SetProcessWorkingSetSizeEx unbalances stack

[pukovec]

When I use Argon2 in Release on Win10 x64bit, the following exception is thrown: (seems like a mismatch in function signature - wrong params or their types)

Additional information: A call to PInvoke function 
'Isopoh.Cryptography.SecureArray!Isopoh.Cryptography.SecureArray.SecureArray::SetProcessWorkingSetSizeEx' has unbalanced the stack. This is likely because the managed PInvoke signature does not match the unmanaged target signature. Check that the calling convention and parameters of the PInvoke signature match the target unmanaged signature.

[mheyman]

Hmm, that doesn't happen on my Win10 64-bit.

The calling convention issue I think is a red herring because the default is WinApi and that call is in kernel32.dll which is about as WinApi as you can get.

I'm thinking it might be a 32-bit vs 64-bit issue which you can get on 64-bit systems. I tried to reproduce the problem and failed.

You forced me to realize that there was a latent bug for Win10 32-bit systems. I added code to fix that but it probably won't make a difference for you (but may). I cannot test that code (similarly, I cannot test the MacOS code).

If you pull the 1.0.3 version of SecureArray from NuGet, you will get the updated code that could fix your problem. Until I can reproduce your problem, I'm going to have a hard time validating any fix.

On Fri, Sep 8, 2017 at 12:18 PM, pukovec notifications@github.com wrote:

When I use Argon2 in Release on Win10 x64bit, the following happens:

Additional information: A call to PInvoke function 'Isopoh.Cryptography. SecureArray!Isopoh.Cryptography.SecureArray.SecureArray::SetProcessWorkingSetSizeEx' has unbalanced the stack. This is likely because the managed PInvoke signature does not match the unmanaged target signature. Check that the calling convention and parameters of the PInvoke signature match the target unmanaged signature.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mheyman/Isopoh.Cryptography.Argon2/issues/6, or mute the thread https://github.com/notifications/unsubscribe-auth/ABM_p3OaqMKz3jEiNM6NL69Jvd19fUnRks5sgWjJgaJpZM4PRZe_ .

[paulhickman]

How about making it injectable:

1. Declare an interface ISecureArray with the public methods of SecureArray
2. Add a constructor for the Argon2 class that takes an Func<int,ISecureArray> factory function as an argument.
3. Have the default constructor pass BestSecuryArray as the factory function.

That way, uses can replace SecureArray with their own implementation if there are issues on a particular platform, and you can inject a mock array in test functions if that helps.

[mheyman]

That was my first idea to architect the system when I added Linux (an the untested OSX) support. Unfortunately, ISecureArray would be a generic interface and, while C# lets you do something like:

    public int Foo<T>(T arg)
    {
        return 0;
    }
    public void Bar()
    {
        Func<int, int> fooFunc = Foo<int>;
        fooFunc(7);
    }

It doesn't let you do: public void Bar() { Func<T, int> fooFunc = Foo; fooFunc(7); }

The Argon2/Blake2b code only uses ISecureArray and ISecureArray, so for that code passing in two Func<>s wouldn't be too bad, I decided the cleaner implementation was to split out the platform-specific operations inside SecureArray. But, you have given me the idea of injecting a type that has the three functions SecureArray uses (zero, lock, and unlock memory). I'll work on that when I get a chance.

On Sat, Sep 23, 2017 at 10:53 AM, paulhickman notifications@github.com wrote:

How about making it injectable:

1. Declare an interface ISecureArray with the public methods of SecureArray
2. Add a constructor for the Argon2 class that takes an Func<int,ISecureArray> factory function as an argument. 2, Have the default constructor pass BestSecuryArray as the factory function.

That way, uses can replace SecureArray with their own implementation if there are issues on a particular platform, and you can inject a mock array in test functions if that helps.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mheyman/Isopoh.Cryptography.Argon2/issues/6#issuecomment-331640704, or mute the thread https://github.com/notifications/unsubscribe-auth/ABM_pwyC_M_SNjS01GV0PLQAqf2G8GWLks5slRuBgaJpZM4PRZe_ .

[mheyman]

I have added a SecureArrayCall class that holds the methods used by SecureArray to actually secure the array.

There is a SecureArray.DefaultCall property that will get populated with the basic calls for Windows/Linux/OSx. In a unit test, I create a SecureArrayCall that wraps calls in SecureArray.DefaultCall to track locks (and I found a situation where I didn't free a lock - fixed in 1.0.5).

The Argon2 calls (should) now all have an optional parameter so they can use a non-default SecureArrayCall to get non-default behavior.

[thomasd3]

Same message here:

Managed Debugging Assistant 'PInvokeStackImbalance' : 'A call to PInvoke function 'Isopoh.Cryptography.SecureArray!Isopoh.Cryptography.SecureArray.SecureArray::SetProcessWorkingSetSizeEx64' has unbalanced the stack. This is likely because the managed PInvoke signature does not match the unmanaged target signature. Check that the calling convention and parameters of the PInvoke signature match the target unmanaged signature.'

That's with .NET 4.7 "AnyCpu" on Windows Server 2016

[mheyman]

I think I can get access to a WinServer 2016 VM. I'll see if I can reproduce this as soon as I can.

[mheyman]

It was a bug! I had flipped 32/64-bit P/Invokes and I never hit it until I added a .Net Framework test (which I had removed a long time ago as painful to maintain). The testing here is woefully incomplete because of the multitude of platforms and configurations it theoretically runs on...

1.0.6 version at https://www.nuget.org/packages/Isopoh.Cryptography.Argon2/ has the fix.