search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.45k stars 4.76k forks source link

Calling a Vararg P/Invoke from another assembly fails to call underlying native function #87188

Open jkoritzinsky opened 1 year ago

jkoritzinsky commented 1 year ago

Description

When calling a vararg P/Invoke defined in another assembly, the runtime tries to look up the metadata from the wrong module. This can cause a variety of issues, from throwing a BadImageFormatException to emitting an incorrect stub signature.

Reproduction Steps

Define the following P/Invoke in assembly A:

[DllImport("msvcrt.dll", CharSet = CharSet.Ansi, CallingConvention = CallingConvention.Cdecl)]
public static unsafe extern int _scprintf(string format, __arglist);

Call it from another assembly (Assembly B):

_scprintf("%d\n", __arglist(42));

Expected behavior

"42" is printed out to the console.

Actual behavior

BadImageFormatException if the method token for _scprintf is higher than the highest valid method token in assembly B.

Stack corruption if the method token for _scprintf in Assembly A corresponds to a method in Assembly B without the preservesig metadata bit set.

Regression?

Based on code inspection, this issue has likely been present since .NET Framework.

Known Workarounds

Move the call to the P/Invoke to the same assembly as the caller (or vice versa).

Configuration

.NET 7.0.3 Windows x86

Other information

The failure is due to this line using m_pModule instead of m_pMetadataModule:

https://github.com/dotnet/runtime/blob/10222f94e5d89b19959117dd9e5b8576ef63f878/src/coreclr/vm/dllimport.cpp#L4267

fkelava commented 1 year ago

Hi! Figured I'd chime in here, seeing as I ran into this issue.

I originally reported this issue through Discord and Jeremy kindly helped me discover what it was and filed it. Thanks again!

Although the workaround given is fine for the time being, I'd just like to let you know there is interest in seeing this fixed at some point. I know it's low priority, but I hope it's more a few months than a few years from now. Maybe in .NET 9, time permitting?

If there is no time to deal with it at present, that's fine too. It's a minor inconvenience at most; I'm just commenting on the very naive assumption that the fix truly is a one-liner or at least not too drastic.