Load handler DLL with load dir on the path#63773
Load handler DLL with load dir on the path#63773adityamandaleeka merged 1 commit intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with DLL loading in the ASP.NET Core Module V2 by modifying the library loading behavior to include the DLL's directory in the search path.
- Replaces
LoadLibrarywithLoadLibraryExusing specific search flags - Ensures the DLL load directory is included in the search path for dependencies
DeagleGross
left a comment
There was a problem hiding this comment.
Approving to unblock an urgent PR; I assume you've tested this running well on arm64.
| LOG_INFOF(L"Loading request handler: '%ls'", handlerDllPath.c_str()); | ||
|
|
||
| hRequestHandlerDll = LoadLibrary(handlerDllPath.c_str()); | ||
| hRequestHandlerDll = LoadLibraryEx(handlerDllPath.c_str(), nullptr, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR); |
There was a problem hiding this comment.
this will search 4 places -- current dll, app, System32 and any AddDllDirectory/SetDllDirectory. Do we need all those places?
I see in the code of this dll we may SetDllDirectory to the current directory. Is that relevant?
There was a problem hiding this comment.
I'm wondering whether it's well defined exactly where it should look and that's something more limited than this range. As far as security goes, I guess we trust app directory/dll directory. We do not necessarily trust current directory, in general.
There was a problem hiding this comment.
I believe the only behavioral difference introduced here is that we now search the directory where the handler DLL is as well. The other places (app dir, system32, etc.) are already being searched with the current LoadLibrary code.
The inprocessapplication.cpp code you linked to is not yet relevant at this point; that comes later.
|
/backport to release/10.0 |
|
Started backporting to release/10.0: https://github.com/dotnet/aspnetcore/actions/runs/17958188973 |
Fix #63772
See that issue for context.
This change updates the handler DLL load call from
LoadLibrarytoLoadLibraryExwith the flagsLOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIRThe handler DLL exports
CreateApplicationas a forwarded export to an architecture-specific DLL. The plainLoadLibrarydoes not search the handler’s directory when resolving that forwarder. As a result, the loader fails to locate the arch-specific DLL and the scenario breaks (the user gets a 500 error).