Pass alloc/free functions to MemoryLoadLibraryEx

[joankaradimov]

This is driven by a problem I solved recently. And MemoryModule helped me a lot. It works great, it's simple, it's well written and does what it should. So thanks for the great library!

The problem I was solving - I have some legacy x86 code that I want to call from an executable that I'm writing. When I say "legacy" - I mean I only have binaries. The challenge is that the code is not relocatable. That's a bit of a problem on a modern Windows. I came to the conclusion that it's virtually impossible to consistently get the base address that I want at runtime.

With some compiler/linker magic I was able to reserve a section in the image at the offset I want. However, this does not play well with the current interface of MemoryModule. The MemoryLoadLibrary function wants to allocate the memory itself and would not work with my memory chunk.

What I did was pass alloc and free functions to MemoryLoadLibraryEx and use those instead. The default implementations just call VirtualAlloc/VirtualFree. My implementation sets memory permissions on the pre-allocated memory. Here is a rough example of what I'm doing:

static LPVOID _MyAlloc(LPVOID address, SIZE_T size, DWORD allocationType, DWORD protect, void* userdata) {
    if (address < LEGACY_IMAGE_BASE || address >= LEGACY_IMAGE_BASE) {
        return VirtualAlloc(address, size, allocationType, protect);
    }
    DWORD old_protect;
    BOOL success = VirtualProtect(address, size, protect, &old_protect);
    if (success) {
        return address;
    }
    else {
        return nullptr;
    }
}

/* some more code.... and then: */
MemoryLoadLibraryEx(address, size, _MyAlloc, _MyFree, _LoadLibrary, _GetProcAddress, _FreeLibrary, NULL);

I hope it's not an issue that this changes the interface of MemoryLoadLibraryEx. My rationale behind it is that this is not a public API. At least I didn't find any references to it in the documentation at http://www.joachim-bauch.de/tutorials/loading-a-dll-from-memory/

TL;DR
This pull request allows for MemoryLoadLibraryEx to work with memory allocations different from calls to VirtualAlloc.

[fancycode]

Nice, thanks. You should also use the custom function to release the memory in MemoryFreeLibrary (https://github.com/joankaradimov/MemoryModule/blob/custom-alloc-and-free/MemoryModule.c#L744)

[joankaradimov]

Ah, yes. Sorry, I missed that one. I'll fix it later today and I'll update the pull request.

By the way - would you like me to try and write some tests for that?

[fancycode]

Sure, tests for new features are always welcome to prevent regressions. Although this here doesn't really change DLL loading functionality, so I'll leave it up to you...

[joankaradimov]

I've fixed the memory releasing that I initially overlooked.

As for the tests - things are becoming a bit ugly. MemoryLoadLibraryEx expects all the custom functions to be passed. However the default implementations are not visible from outside MemoryModule.c. Seeing how they are static, this is intentional and I don't want to change that.

I was thinking - would it be acceptable to make some of the custom callbak params optional. I was thinking something along these lines in MemoryLoadLibraryEx:

    if (!allocMemory) allocMemory = _VirtualAlloc;
    if (!freeMemory) freeMemory = _VirtualFree;
    if (!loadLibrary) loadLibrary = _LoadLibrary;
    if (!getProcAddress) getProcAddress = _GetProcAddress;
    if (!freeLibrary) freeLibrary = _FreeLibrary;

So calling with a NULL would fallback to a default implementation. That would allow for calls that look like this:

MemoryLoadLibraryEx(address, size, _MockAlloc, _MockFree, NULL, NULL, NULL, NULL);

[fancycode]

Hmm, I'm not sure about having that in MemoryLoadLibraryEx because if somebody passes NULL as the wrong parameter, he ends up having undefined behaviour (due to the default being used), instead of just crashing.

You could add a MockMemoryLoadLibraryEx function in your tests and support NULL and some default implementations there.

[joankaradimov]

[...] if somebody passes NULL as the wrong parameter, he ends up having undefined behaviour [...]

But what if someone intentionally wants to use the default behavior for some of the parameter functions? That's not possible at the moment without copy pasting the implementations of _LoadLibrary, _GetProcAddress, and _FreeLibrary. They are all static and can not be used from outside code.

Maybe those can be made non-static and included in the header?

[fancycode]

Ok, I see your point. I exposed the default functions in the referenced commit.

You will need to rebase your merge request though...

[joankaradimov]

Great, thanks!

I'll rebase and I'll let you know when I have the tests.

[joankaradimov]

Thanks for waiting for this.

There is some difference in the behaviour between Visual Studio and gcc cross-compiled code. I'll resolve it and write again.

[joankaradimov]

The CI seems to be good now.

About the tests - I went with black box testing. I tried to do something similar to mock objects. It's not perfect and I'm open to suggestions on how to improve it.

I have some notes on the tests I did:

• I could shorten the code if I put the code for LoadFromMemory and TestCustomAllocAndFree together. It didn't seem appropriate, though.
• I didn't put TestCustomAllocAndFree in a separate file either. Maybe I should have.
• I'm not using C++ in the tests (although DllLoader has a cpp extenstion). Some things could have been shorter, but I wanted to be consistent with you code style.
• I've only tested 3 scenarios.