Skip to content

Commit 8552f09

Browse files
committed
win32: inject a socket-aware version of CloseHandle() into the CRT
_close() on an fd calls CloseHandle(), which is illegal if the fd contains a socket handle. We previously worked around this by having our own close(), which called closesocket() before calling _close() (e601c43). Amusingly, the author of that solution thought it's just a temporary workaround: /* * close RTL fd while respecting sockets * added as temporary measure until PerlIO has real * Win32 native layer * -- BKS, 11-11-2000 */ To make it thread-safe, we had to manipulate the internals of file descriptors, which kept changing (b47a847). Unfortunately, the C runtime has been rewritten and it no longer exposes them at all. We had to disable the thread-safety fix in Visual C++ 2015 builds (1f664ef). It also wouldn't work with MinGW configured to use UCRT. This commit introduces a new solution: we inject a socket-aware version of CloseHandle() into the C runtime library. Hopefully, this will be less fragile. This also fixes a few issues that the original solution didn't: - Closing a busy pipe doesn't cause a deadlock (fixes #19963) - _dup2 properly closes an overwritten socket (fixes #20920)
1 parent 8a548d1 commit 8552f09

File tree

4 files changed

+182
-173
lines changed

4 files changed

+182
-173
lines changed

pod/perldelta.pod

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,19 @@ L</Modules and Pragmata> section.
365365

366366
=over 4
367367

368-
=item XXX-some-platform
368+
=item Windows
369369

370-
XXX
370+
=over 4
371+
372+
=item *
373+
374+
C<POSIX::dup2> no longer creates broken sockets. [GH #20920]
375+
376+
=item *
377+
378+
Closing a busy pipe could cause Perl to hang. [GH #19963]
379+
380+
=back
371381

372382
=back
373383

win32/win32.c

Lines changed: 170 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#include <io.h>
3939
#include <signal.h>
4040
#include <winioctl.h>
41+
#include <winternl.h>
4142

4243
/* #include "config.h" */
4344

@@ -156,9 +157,6 @@ static void translate_to_errno(void);
156157
START_EXTERN_C
157158
HANDLE w32_perldll_handle = INVALID_HANDLE_VALUE;
158159
char w32_module_name[MAX_PATH+1];
159-
#ifdef WIN32_DYN_IOINFO_SIZE
160-
Size_t w32_ioinfo_size;/* avoid 0 extend op b4 mul, otherwise could be a U8 */
161-
#endif
162160
END_EXTERN_C
163161

164162
static OSVERSIONINFO g_osver = {0, 0, 0, 0, 0, ""};
@@ -3313,7 +3311,7 @@ win32_freopen(const char *path, const char *mode, FILE *stream)
33133311
DllExport int
33143312
win32_fclose(FILE *pf)
33153313
{
3316-
return my_fclose(pf); /* defined in win32sck.c */
3314+
return fclose(pf);
33173315
}
33183316

33193317
DllExport int
@@ -3913,13 +3911,10 @@ win32_open(const char *path, int flag, ...)
39133911
return open(PerlDir_mapA(path), flag, pmode);
39143912
}
39153913

3916-
/* close() that understands socket */
3917-
extern int my_close(int); /* in win32sck.c */
3918-
39193914
DllExport int
39203915
win32_close(int fd)
39213916
{
3922-
return my_close(fd);
3917+
return _close(fd);
39233918
}
39243919

39253920
DllExport int
@@ -5172,6 +5167,172 @@ ansify_path(void)
51725167
win32_free(wide_path);
51735168
}
51745169

5170+
/* This hooks a function that is imported by the specified module. The hook is
5171+
* local to that module. */
5172+
static bool
5173+
win32_hook_imported_function_in_module(
5174+
HMODULE module, LPCSTR fun_name, FARPROC hook_ptr
5175+
)
5176+
{
5177+
ULONG_PTR image_base = (ULONG_PTR)module;
5178+
PIMAGE_DOS_HEADER dos_header = (PIMAGE_DOS_HEADER)image_base;
5179+
PIMAGE_NT_HEADERS nt_headers
5180+
= (PIMAGE_NT_HEADERS)(image_base + dos_header->e_lfanew);
5181+
PIMAGE_OPTIONAL_HEADER opt_header = &nt_headers->OptionalHeader;
5182+
5183+
PIMAGE_DATA_DIRECTORY data_dir = opt_header->DataDirectory;
5184+
DWORD data_dir_len = opt_header->NumberOfRvaAndSizes;
5185+
5186+
BOOL is_idt_present = data_dir_len > IMAGE_DIRECTORY_ENTRY_IMPORT
5187+
&& data_dir[IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress != 0;
5188+
5189+
if (!is_idt_present)
5190+
return FALSE;
5191+
5192+
BOOL found = FALSE;
5193+
5194+
/* Import Directory Table */
5195+
PIMAGE_IMPORT_DESCRIPTOR idt = (PIMAGE_IMPORT_DESCRIPTOR)(
5196+
image_base + data_dir[IMAGE_DIRECTORY_ENTRY_IMPORT].VirtualAddress
5197+
);
5198+
5199+
for (; idt->Name != 0; ++idt) {
5200+
/* Import Lookup Table */
5201+
PIMAGE_THUNK_DATA ilt
5202+
= (PIMAGE_THUNK_DATA)(image_base + idt->OriginalFirstThunk);
5203+
/* Import Address Table */
5204+
PIMAGE_THUNK_DATA iat
5205+
= (PIMAGE_THUNK_DATA)(image_base + idt->FirstThunk);
5206+
5207+
ULONG_PTR address_of_data;
5208+
for (; address_of_data = ilt->u1.AddressOfData; ++ilt, ++iat) {
5209+
/* Ordinal imports are quite rare, so skipping them will most likely
5210+
* not cause any problems. */
5211+
BOOL is_ordinal
5212+
= address_of_data >> ((sizeof(address_of_data) * 8) - 1);
5213+
5214+
if (is_ordinal)
5215+
continue;
5216+
5217+
LPCSTR name = (
5218+
(PIMAGE_IMPORT_BY_NAME)(image_base + address_of_data)
5219+
)->Name;
5220+
5221+
if (strEQ(name, fun_name)) {
5222+
DWORD old_protect = 0;
5223+
BOOL succ = VirtualProtect(
5224+
&iat->u1.Function, sizeof(iat->u1.Function), PAGE_READWRITE,
5225+
&old_protect
5226+
);
5227+
if (!succ)
5228+
return FALSE;
5229+
5230+
iat->u1.Function = (ULONG_PTR)hook_ptr;
5231+
found = TRUE;
5232+
5233+
VirtualProtect(
5234+
&iat->u1.Function, sizeof(iat->u1.Function), old_protect,
5235+
&old_protect
5236+
);
5237+
break;
5238+
}
5239+
}
5240+
}
5241+
5242+
return found;
5243+
}
5244+
5245+
typedef NTSTATUS (NTAPI *pNtQueryInformationFile_t)(HANDLE, PIO_STATUS_BLOCK, PVOID, ULONG, ULONG);
5246+
pNtQueryInformationFile_t pNtQueryInformationFile = NULL;
5247+
5248+
typedef BOOL (WINAPI *pCloseHandle)(HANDLE h);
5249+
static pCloseHandle CloseHandle_orig;
5250+
5251+
/* CloseHandle() that supports sockets. CRT uses mutexes during file operations,
5252+
* so the lack of thread safety in this function isn't a problem. */
5253+
static BOOL WINAPI
5254+
my_CloseHandle(HANDLE h)
5255+
{
5256+
/* In theory, passing a non-socket handle to closesocket() is fine. It
5257+
* should return a WSAENOTSOCK error, which is easy to recover from.
5258+
* However, we should avoid doing that because it's not that simple in
5259+
* practice. For instance, it can deadlock on a handle to a stuck pipe (see:
5260+
* https://github.com/Perl/perl5/issues/19963).
5261+
*
5262+
* There's no foolproof way to tell if a handle is a socket (mostly because
5263+
* of the non-IFS sockets), but in some cases we can tell if a handle
5264+
* is definitely *not* a socket.
5265+
*/
5266+
5267+
/* GetFileType() always returns FILE_TYPE_PIPE for sockets. */
5268+
BOOL maybe_socket = (GetFileType(h) == FILE_TYPE_PIPE);
5269+
5270+
if (maybe_socket && pNtQueryInformationFile) {
5271+
IO_STATUS_BLOCK isb;
5272+
struct {
5273+
ULONG name_len;
5274+
WCHAR name[100];
5275+
} volume = {0};
5276+
5277+
/* There are many ways to tell a named pipe from a socket, but almost
5278+
* all of them can deadlock on a handle to a stuck pipe (like in the
5279+
* bug ticket mentioned above). According to my tests,
5280+
* FileVolumeNameInfomation is the only relevant function that doesn't
5281+
* suffer from this problem.
5282+
*
5283+
* It's undocumented and it requires Windows 10, so on older systems
5284+
* we always pass pipes to closesocket().
5285+
*/
5286+
NTSTATUS s = pNtQueryInformationFile(
5287+
h, &isb, &volume, sizeof(volume), 58 /* FileVolumeNameInformation */
5288+
);
5289+
if (NT_SUCCESS(s)) {
5290+
maybe_socket = (_wcsnicmp(
5291+
volume.name, L"\\Device\\NamedPipe", C_ARRAY_LENGTH(volume.name)
5292+
) != 0);
5293+
}
5294+
}
5295+
5296+
if (maybe_socket)
5297+
if (closesocket((SOCKET)h) == 0)
5298+
return TRUE;
5299+
else if (WSAGetLastError() != WSAENOTSOCK)
5300+
return FALSE;
5301+
5302+
return CloseHandle_orig(h);
5303+
}
5304+
5305+
/* Hook CloseHandle() inside CRT so its functions like _close() or
5306+
* _dup2() can close sockets properly. */
5307+
static void
5308+
win32_hook_closehandle_in_crt()
5309+
{
5310+
/* Get the handle to the CRT module basing on the address of _close()
5311+
* function. */
5312+
HMODULE crt_handle;
5313+
BOOL succ = GetModuleHandleExA(
5314+
GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS
5315+
| GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT, (LPCSTR)_close,
5316+
&crt_handle
5317+
);
5318+
if (!succ)
5319+
return;
5320+
5321+
CloseHandle_orig = (pCloseHandle)GetProcAddress(
5322+
GetModuleHandleA("kernel32.dll"), "CloseHandle"
5323+
);
5324+
if (!CloseHandle_orig)
5325+
return;
5326+
5327+
win32_hook_imported_function_in_module(
5328+
crt_handle, "CloseHandle", (FARPROC)my_CloseHandle
5329+
);
5330+
5331+
pNtQueryInformationFile = (pNtQueryInformationFile_t)GetProcAddress(
5332+
GetModuleHandleA("ntdll.dll"), "NtQueryInformationFile"
5333+
);
5334+
}
5335+
51755336
void
51765337
Perl_win32_init(int *argcp, char ***argvp)
51775338
{
@@ -5208,17 +5369,7 @@ Perl_win32_init(int *argcp, char ***argvp)
52085369
g_osver.dwOSVersionInfoSize = sizeof(g_osver);
52095370
GetVersionEx(&g_osver);
52105371

5211-
#ifdef WIN32_DYN_IOINFO_SIZE
5212-
{
5213-
Size_t ioinfo_size = _msize((void*)__pioinfo[0]);;
5214-
if((SSize_t)ioinfo_size <= 0) { /* -1 is err */
5215-
fprintf(stderr, "panic: invalid size for ioinfo\n"); /* no interp */
5216-
exit(1);
5217-
}
5218-
ioinfo_size /= IOINFO_ARRAY_ELTS;
5219-
w32_ioinfo_size = ioinfo_size;
5220-
}
5221-
#endif
5372+
win32_hook_closehandle_in_crt();
52225373

52235374
ansify_path();
52245375

win32/win32.h

Lines changed: 0 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,6 @@ DllExport void win32_get_child_IO(child_IO_table* ptr);
426426
DllExport HWND win32_create_message_window(void);
427427
DllExport int win32_async_check(pTHX);
428428

429-
extern int my_fclose(FILE *);
430429
extern char * win32_get_privlib(WIN32_NO_REGISTRY_M_(const char *pl) STRLEN *const len);
431430
extern char * win32_get_sitelib(const char *pl, STRLEN *const len);
432431
extern char * win32_get_vendorlib(const char *pl, STRLEN *const len);
@@ -566,83 +565,6 @@ void win32_wait_for_children(pTHX);
566565
# define PERL_WAIT_FOR_CHILDREN win32_wait_for_children(aTHX)
567566
#endif
568567

569-
/* The following ioinfo struct manipulations had been removed but were
570-
* reinstated to fix RT#120091/118059. However, they do not work with
571-
* the rewritten CRT in VS2015 so they are removed once again for VS2015
572-
* onwards, which will therefore suffer from the reintroduction of the
573-
* close socket bug. */
574-
#if (!defined(_MSC_VER)) || (defined(_MSC_VER) && _MSC_VER < 1900)
575-
576-
#ifdef PERL_CORE
577-
578-
/* C doesn't like repeat struct definitions */
579-
#if defined(__MINGW32__) && (__MINGW32_MAJOR_VERSION>=3)
580-
# undef _CRTIMP
581-
#endif
582-
#ifndef _CRTIMP
583-
# define _CRTIMP __declspec(dllimport)
584-
#endif
585-
586-
#ifndef __MINGW32__
587-
/* size of ioinfo struct is determined at runtime */
588-
# define WIN32_DYN_IOINFO_SIZE
589-
#endif
590-
591-
#ifndef WIN32_DYN_IOINFO_SIZE
592-
/*
593-
* Control structure for lowio file handles
594-
*/
595-
typedef struct {
596-
intptr_t osfhnd;/* underlying OS file HANDLE */
597-
char osfile; /* attributes of file (e.g., open in text mode?) */
598-
char pipech; /* one char buffer for handles opened on pipes */
599-
int lockinitflag;
600-
CRITICAL_SECTION lock;
601-
} ioinfo;
602-
#else
603-
typedef intptr_t ioinfo;
604-
#endif
605-
606-
/*
607-
* Array of arrays of control structures for lowio files.
608-
*/
609-
EXTERN_C _CRTIMP ioinfo* __pioinfo[];
610-
611-
/*
612-
* Definition of IOINFO_L2E, the log base 2 of the number of elements in each
613-
* array of ioinfo structs.
614-
*/
615-
#define IOINFO_L2E 5
616-
617-
/*
618-
* Definition of IOINFO_ARRAY_ELTS, the number of elements in ioinfo array
619-
*/
620-
#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)
621-
622-
/*
623-
* Access macros for getting at an ioinfo struct and its fields from a
624-
* file handle
625-
*/
626-
#ifdef WIN32_DYN_IOINFO_SIZE
627-
# define _pioinfo(i) ((intptr_t *) \
628-
(((Size_t)__pioinfo[(i) >> IOINFO_L2E])/* * to head of array ioinfo [] */\
629-
/* offset to the head of a particular ioinfo struct */ \
630-
+ (((i) & (IOINFO_ARRAY_ELTS - 1)) * w32_ioinfo_size)) \
631-
)
632-
/* first slice of ioinfo is always the OS handle */
633-
# define _osfhnd(i) (*(_pioinfo(i)))
634-
#else
635-
# define _pioinfo(i) (__pioinfo[(i) >> IOINFO_L2E] + ((i) & (IOINFO_ARRAY_ELTS - 1)))
636-
# define _osfhnd(i) (_pioinfo(i)->osfhnd)
637-
#endif
638-
639-
/* since we are not doing a dup2(), this works fine */
640-
#define _set_osfhnd(fh, osfh) (void)(_osfhnd(fh) = (intptr_t)osfh)
641-
642-
#endif /* PERL_CORE */
643-
644-
#endif /* !defined(_MSC_VER) || _MSC_VER<1900 */
645-
646568
/* IO.xs and POSIX.xs define PERLIO_NOT_STDIO to 1 */
647569
#if defined(PERL_EXT_IO) || defined(PERL_EXT_POSIX)
648570
#undef PERLIO_NOT_STDIO

0 commit comments

Comments
 (0)