Skip to content

Commit

Permalink
win32/win32sck.c: dont close() a freed socket os handle
Browse files Browse the repository at this point in the history
This patch is in RT as [perl #120091] but also fixes [perl #118059].

Because the MS C lib, doesn't support sockets natively, Perl uses
open_osfhandle, to wrap a socket into CRT fd. Sockets handles must be
closed with closesocket, not CloseHandle (which CRT close calls).
Undefined behavior will happen otherwise according to MS. Swap the now
closed socket handle in the CRT to INVALID_HANDLE_VALUE. The CRT will
not call CloseHandle on INVALID_HANDLE_VALUE and returns success if it
sees INVALID_HANDLE_VALUE. CloseHandle will never be called on a socket
handle with this patch.

In #118059, a race condition was reported, where accept() failed with
ENOTSOCK, when a psuedofork was done, and connection from the child fake
perl process to parent fake perl process was done. The race condition's
effects occur inside the user mode code in mswsock.dll!_WSPAccept in the
parent perl, which winds up having a kernel handle of an unknown type
in it that is invalid. The invalid handle is passed to kernel calls, which
fail, because the handle is invalid, and the error is translated to
ENOTSOCK.  The exact mechanism of the bug and 100% reproducabilty of the
race were never established, since mswsock.dll is closed source.

Another benefit of this patch is making it easier to use C debuggers on
a Win32 Perl because of less debugger-only bad handle exceptions
(NtGlobalFlag FLG_ENABLE_HANDLE_EXCEPTIONS/0xC0000008 STATUS_INVALID_HANDLE).

This commit reverts parts of commit 9b1f181
"Get rid of PERL_MSVCRT_READFIX" and parts of commit 46e77f1
"Don't use the PERL_MSVCRT_READFIX when using VC++ 7.x onwards." and
contains additional changes not found in those 2 commits. The method for
selecting the definition of struct ioinfo isn't perfect. It will break if
VC > 6 is changed to use the older msvcrt.dll. For some versions of the
CRT, like 2005/8.0, it is impossible to know the definition of ioinfo
struct at C compile time, since the struct increased in size a number of
times with higher build numbers of v8.0 CRT. SxS and security updates make
that same perl binary will run with different v8.0 CRTs at different times.
For the case when ioinfo can not be hard coded, introduce
WIN32_DYN_IOINFO_SIZE. With WIN32_DYN_IOINFO_SIZE, the size of the ioinfo
struct is determined on Perl process startup using _mize. Since VC 2013
is a brand new product at the time of this patch, even though its struct
is known, and 2008 through 2012 have been stable so far, don't assume at
this time 2013's ioinfo will be stable. Therefore, VC 2003 and older
(including Mingw's v6.0), 2008 to 2012, are hard coded. 2013 is a candidate
one day to be hard coded. VC 2005 can never be hard coded.

All non-WIN32_DYN_IOINFO_SIZE compilers will work with
WIN32_DYN_IOINFO_SIZE. Non-WIN32_DYN_IOINFO_SIZE mode is simply more
efficient.

For future compatibility concerns, 3 forms of protection are offered.
If __pioinfo isn't exported anymore, the Perl build will break. In
WIN32_DYN_IOINFO_SIZE mode, if __pioinfo isn't heap memory anymore or the
start of a memory block, the runtime panic will happen. In with and without
WIN32_DYN_IOINFO_SIZE, only on a DEBUGGING build, the handle returned by
CRT's _get_osfhandle, which is the authentic handle in the CRT, is compared
to the handle found by accessing the ioinfo struct directly. If they don't
match, the handle swapping code is broken and the assert fails.
  • Loading branch information
bulk88 authored and steve-m-hay committed Nov 2, 2013
1 parent 50e4f4d commit b47a847
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 4 deletions.
15 changes: 15 additions & 0 deletions win32/win32.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,9 @@ static void win32_csighandler(int sig);
START_EXTERN_C
HANDLE w32_perldll_handle = INVALID_HANDLE_VALUE;
char w32_module_name[MAX_PATH+1];
#ifdef WIN32_DYN_IOINFO_SIZE
Size_t w32_ioinfo_size;/* avoid 0 extend op b4 mul, otherwise could be a U8 */
#endif
END_EXTERN_C

static OSVERSIONINFO g_osver = {0, 0, 0, 0, 0, ""};
Expand Down Expand Up @@ -4415,6 +4418,18 @@ Perl_win32_init(int *argcp, char ***argvp)
g_osver.dwOSVersionInfoSize = sizeof(g_osver);
GetVersionEx(&g_osver);

#ifdef WIN32_DYN_IOINFO_SIZE
{
Size_t ioinfo_size = _msize((void*)__pioinfo[0]);;
if((SSize_t)ioinfo_size <= 0) { /* -1 is err */
fprintf(stderr, "panic: invalid size for ioinfo\n"); /* no interp */
exit(1);
}
ioinfo_size /= IOINFO_ARRAY_ELTS;
w32_ioinfo_size = ioinfo_size;
}
#endif

ansify_path();
}

Expand Down
94 changes: 94 additions & 0 deletions win32/win32.h
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,100 @@ void win32_wait_for_children(pTHX);
# define PERL_WAIT_FOR_CHILDREN win32_wait_for_children(aTHX)
#endif

#ifdef PERL_CORE
/* C doesn't like repeat struct definitions */
#if defined(__MINGW32__) && (__MINGW32_MAJOR_VERSION>=3)
#undef _CRTIMP
#endif
#ifndef _CRTIMP
#define _CRTIMP __declspec(dllimport)
#endif


/* VV 2005 has multiple ioinfo struct definitions through VC 2005's release life
* VC 2008-2012 have been stable but do not assume future VCs will have the
* same ioinfo struct, just because past struct stability. If research is done
* on the CRTs of future VS, the version check can be bumped up so the newer
* VC uses a fixed ioinfo size.
*/
#if ! (_MSC_VER < 1400 || (_MSC_VER >= 1500 && _MSC_VER <= 1700) \
|| defined(__MINGW32__))
/* size of ioinfo struct is determined at runtime */
# define WIN32_DYN_IOINFO_SIZE
#endif

#ifndef WIN32_DYN_IOINFO_SIZE
/*
* Control structure for lowio file handles
*/
typedef struct {
intptr_t osfhnd;/* underlying OS file HANDLE */
char osfile; /* attributes of file (e.g., open in text mode?) */
char pipech; /* one char buffer for handles opened on pipes */
int lockinitflag;
CRITICAL_SECTION lock;
/* this struct defintion breaks ABI compatibility with
* not using, cl.exe's native VS version specitfic CRT. */
# if _MSC_VER >= 1400 && _MSC_VER < 1500
# error "This ioinfo struct is incomplete for Visual C 2005"
# endif
/* VC 2005 CRT has atleast 3 different definitions of this struct based on the
* CRT DLL's build number. */
# if _MSC_VER >= 1500
# ifndef _SAFECRT_IMPL
/* Not used in the safecrt downlevel. We do not define them, so we cannot
* use them accidentally */
char textmode : 7;/* __IOINFO_TM_ANSI or __IOINFO_TM_UTF8 or __IOINFO_TM_UTF16LE */
char unicode : 1; /* Was the file opened as unicode? */
char pipech2[2]; /* 2 more peak ahead chars for UNICODE mode */
__int64 startpos; /* File position that matches buffer start */
BOOL utf8translations; /* Buffer contains translations other than CRLF*/
char dbcsBuffer; /* Buffer for the lead byte of dbcs when converting from dbcs to unicode */
BOOL dbcsBufferUsed; /* Bool for the lead byte buffer is used or not */
# endif
# endif
} ioinfo;
#else
typedef intptr_t ioinfo;
#endif

/*
* Array of arrays of control structures for lowio files.
*/
EXTERN_C _CRTIMP ioinfo* __pioinfo[];

/*
* Definition of IOINFO_L2E, the log base 2 of the number of elements in each
* array of ioinfo structs.
*/
#define IOINFO_L2E 5

/*
* Definition of IOINFO_ARRAY_ELTS, the number of elements in ioinfo array
*/
#define IOINFO_ARRAY_ELTS (1 << IOINFO_L2E)

/*
* Access macros for getting at an ioinfo struct and its fields from a
* file handle
*/
#ifdef WIN32_DYN_IOINFO_SIZE
# define _pioinfo(i) ((intptr_t *) \
(((Size_t)__pioinfo[(i) >> IOINFO_L2E])/* * to head of array ioinfo [] */\
/* offset to the head of a particular ioinfo struct */ \
+ (((i) & (IOINFO_ARRAY_ELTS - 1)) * w32_ioinfo_size)) \
)
/* first slice of ioinfo is always the OS handle */
# define _osfhnd(i) (*(_pioinfo(i)))
#else
# define _pioinfo(i) (__pioinfo[(i) >> IOINFO_L2E] + ((i) & (IOINFO_ARRAY_ELTS - 1)))
# define _osfhnd(i) (_pioinfo(i)->osfhnd)
#endif

/* since we are not doing a dup2(), this works fine */
# define _set_osfhnd(fh, osfh) (void)(_osfhnd(fh) = (intptr_t)osfh)
#endif /* PERL_CORE */

/* IO.xs and POSIX.xs define PERLIO_NOT_STDIO to 1 */
#if defined(PERL_EXT_IO) || defined(PERL_EXT_POSIX)
#undef PERLIO_NOT_STDIO
Expand Down
16 changes: 12 additions & 4 deletions win32/win32sck.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ static struct servent* win32_savecopyservent(struct servent*d,

static int wsock_started = 0;

#ifdef WIN32_DYN_IOINFO_SIZE
EXTERN_C Size_t w32_ioinfo_size;
#endif

EXTERN_C void
EndSockets(void)
{
Expand Down Expand Up @@ -689,8 +693,10 @@ int my_close(int fd)
int err;
err = closesocket(osf);
if (err == 0) {
(void)close(fd); /* handle already closed, ignore error */
return 0;
assert(_osfhnd(fd) == osf); /* catch a bad ioinfo struct def */
/* don't close freed handle */
_set_osfhnd(fd, INVALID_HANDLE_VALUE);
return close(fd);
}
else if (err == SOCKET_ERROR) {
err = get_last_socket_error();
Expand All @@ -717,8 +723,10 @@ my_fclose (FILE *pf)
win32_fflush(pf);
err = closesocket(osf);
if (err == 0) {
(void)fclose(pf); /* handle already closed, ignore error */
return 0;
assert(_osfhnd(win32_fileno(pf)) == osf); /* catch a bad ioinfo struct def */
/* don't close freed handle */
_set_osfhnd(win32_fileno(pf), INVALID_HANDLE_VALUE);
return fclose(pf);
}
else if (err == SOCKET_ERROR) {
err = get_last_socket_error();
Expand Down

0 comments on commit b47a847

Please sign in to comment.