From 49fb598b0e78d09d6a2a42679ee0649df482090e Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 11 Apr 2017 18:56:55 +0100 Subject: [PATCH] Add automatic type-checking to GET_WINDOWS_FUNCTION. This gives me an extra safety-check against having mistyped one of the function prototypes that we load at run time from DLLs: we verify that the typedef we defined based on the prototype in our source code matches the type of the real function as declared in the Windows headers. This was an idea I had while adding a pile of further functions using this mechanism. It didn't catch any errors (either in the new functions or in the existing collection), but that's no reason not to keep it anyway now that I've thought of it! In VS2015, this automated type-check works for most functions, but a couple manage to break it. SetCurrentProcessExplicitAppUserModelID in winjump.c can't be type-checked, because including where that function is declared would also bring in a load of other stuff that conflicts with the painful manual COM declarations in winjump.c. (That stuff could probably be removed now we're on an up-to-date Visual Studio, on the other hand, but that's a separate chore.) And gai_strerror, used in winnet.c, does _have_ an implementation in a DLL, but the header files like to provide an inline version with a different calling convention, which defeats this error-checking trick. And in the older VS2003 that we still precautionarily build with, several more type-checks have to be #ifdeffed out because the functions they check against just aren't there at all. --- windows/wingss.c | 5 +++++ windows/winhsock.c | 10 +++++++++- windows/winjump.c | 8 +++++++- windows/winmisc.c | 7 +++++++ windows/winnet.c | 14 ++++++++++++-- windows/winstuff.h | 21 +++++++++++++++------ 6 files changed, 55 insertions(+), 10 deletions(-) diff --git a/windows/wingss.c b/windows/wingss.c index f7cc43eb..ef056167 100644 --- a/windows/wingss.c +++ b/windows/wingss.c @@ -79,7 +79,12 @@ struct ssh_gss_liblist *ssh_gss_setup(Conf *conf) if (!kernel32_module) { kernel32_module = load_system32_dll("kernel32.dll"); } +#if defined _MSC_VER && _MSC_VER < 1900 + /* Omit the type-check because older MSVCs don't have this function */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK(kernel32_module, AddDllDirectory); +#else GET_WINDOWS_FUNCTION(kernel32_module, AddDllDirectory); +#endif list->libraries = snewn(3, struct ssh_gss_library); list->nlibraries = 0; diff --git a/windows/winhsock.c b/windows/winhsock.c index 1a4ee4d7..e5f0fa4f 100644 --- a/windows/winhsock.c +++ b/windows/winhsock.c @@ -282,7 +282,15 @@ static char *sk_handle_peer_info(Socket s) if (!kernel32_module) { kernel32_module = load_system32_dll("kernel32.dll"); - GET_WINDOWS_FUNCTION(kernel32_module, GetNamedPipeClientProcessId); +#if defined _MSC_VER && _MSC_VER < 1900 + /* For older Visual Studio, this function isn't available in + * the header files to type-check */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK( + kernel32_module, GetNamedPipeClientProcessId); +#else + GET_WINDOWS_FUNCTION( + kernel32_module, GetNamedPipeClientProcessId); +#endif } /* diff --git a/windows/winjump.c b/windows/winjump.c index 85963a32..d0dea863 100644 --- a/windows/winjump.c +++ b/windows/winjump.c @@ -728,7 +728,13 @@ BOOL set_explicit_app_user_model_id() if (!shell32_module) { shell32_module = load_system32_dll("Shell32.dll"); - GET_WINDOWS_FUNCTION(shell32_module, SetCurrentProcessExplicitAppUserModelID); + /* + * We can't typecheck this function here, because it's defined + * in , which we're not including due to clashes + * with all the manual-COM machinery above. + */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK( + shell32_module, SetCurrentProcessExplicitAppUserModelID); } if (p_SetCurrentProcessExplicitAppUserModelID) diff --git a/windows/winmisc.c b/windows/winmisc.c index 308f0ea5..85fa3c95 100644 --- a/windows/winmisc.c +++ b/windows/winmisc.c @@ -177,7 +177,14 @@ void dll_hijacking_protection(void) if (!kernel32_module) { kernel32_module = load_system32_dll("kernel32.dll"); +#if defined _MSC_VER && _MSC_VER < 1900 + /* For older Visual Studio, this function isn't available in + * the header files to type-check */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK( + kernel32_module, SetDefaultDllDirectories); +#else GET_WINDOWS_FUNCTION(kernel32_module, SetDefaultDllDirectories); +#endif } if (p_SetDefaultDllDirectories) { diff --git a/windows/winnet.c b/windows/winnet.c index 98753237..7ceca486 100644 --- a/windows/winnet.c +++ b/windows/winnet.c @@ -268,7 +268,10 @@ void sk_init(void) GET_WINDOWS_FUNCTION(winsock_module, getaddrinfo); GET_WINDOWS_FUNCTION(winsock_module, freeaddrinfo); GET_WINDOWS_FUNCTION(winsock_module, getnameinfo); - GET_WINDOWS_FUNCTION(winsock_module, gai_strerror); + /* This function would fail its type-check if we did one, + * because the VS header file provides an inline definition + * which is __cdecl instead of WINAPI. */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK(winsock_module, gai_strerror); } else { /* Fall back to wship6.dll for Windows 2000 */ wship6_module = load_system32_dll("wship6.dll"); @@ -279,7 +282,8 @@ void sk_init(void) GET_WINDOWS_FUNCTION(wship6_module, getaddrinfo); GET_WINDOWS_FUNCTION(wship6_module, freeaddrinfo); GET_WINDOWS_FUNCTION(wship6_module, getnameinfo); - GET_WINDOWS_FUNCTION(wship6_module, gai_strerror); + /* See comment above about type check */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK(winsock_module, gai_strerror); } else { #ifdef NET_SETUP_DIAGNOSTICS logevent(NULL, "No IPv6 support detected"); @@ -310,7 +314,13 @@ void sk_init(void) GET_WINDOWS_FUNCTION(winsock_module, getservbyname); GET_WINDOWS_FUNCTION(winsock_module, inet_addr); GET_WINDOWS_FUNCTION(winsock_module, inet_ntoa); +#if defined _MSC_VER && _MSC_VER < 1900 + /* Older Visual Studio doesn't know about this function at all, so + * can't type-check it */ + GET_WINDOWS_FUNCTION_NO_TYPECHECK(winsock_module, inet_ntop); +#else GET_WINDOWS_FUNCTION(winsock_module, inet_ntop); +#endif GET_WINDOWS_FUNCTION(winsock_module, connect); GET_WINDOWS_FUNCTION(winsock_module, bind); GET_WINDOWS_FUNCTION(winsock_module, setsockopt); diff --git a/windows/winstuff.h b/windows/winstuff.h index 87706f32..007889e4 100644 --- a/windows/winstuff.h +++ b/windows/winstuff.h @@ -128,15 +128,24 @@ struct FontSpec *fontspec_new(const char *name, * * (DECL_WINDOWS_FUNCTION works with both these variants.) */ -#define DECL_WINDOWS_FUNCTION(linkage, rettype, name, params) \ - typedef rettype (WINAPI *t_##name) params; \ +#define TYPECHECK(to_check, to_return) \ + (sizeof(to_check) ? to_return : to_return) +#define DECL_WINDOWS_FUNCTION(linkage, rettype, name, params) \ + typedef rettype (WINAPI *t_##name) params; \ linkage t_##name p_##name #define STR1(x) #x #define STR(x) STR1(x) -#define GET_WINDOWS_FUNCTION_PP(module, name) \ - (p_##name = module ? (t_##name) GetProcAddress(module, STR(name)) : NULL) -#define GET_WINDOWS_FUNCTION(module, name) \ - (p_##name = module ? (t_##name) GetProcAddress(module, #name) : NULL) +#define GET_WINDOWS_FUNCTION_PP(module, name) \ + TYPECHECK((t_##name)NULL == name, \ + (p_##name = module ? \ + (t_##name) GetProcAddress(module, STR(name)) : NULL)) +#define GET_WINDOWS_FUNCTION(module, name) \ + TYPECHECK((t_##name)NULL == name, \ + (p_##name = module ? \ + (t_##name) GetProcAddress(module, #name) : NULL)) +#define GET_WINDOWS_FUNCTION_NO_TYPECHECK(module, name) \ + (p_##name = module ? \ + (t_##name) GetProcAddress(module, #name) : NULL) /* * Global variables. Most modules declare these `extern', but