win32/win32sck.c: dont close() a freed socket os handle
authorDaniel Dragan <bulk88@hotmail.com>
Sat, 2 Nov 2013 03:04:35 +0000 (23:04 -0400)
committerSteve Hay <steve.m.hay@googlemail.com>
Sat, 2 Nov 2013 19:35:43 +0000 (19:35 +0000)
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 9b1f18150a
"Get rid of PERL_MSVCRT_READFIX" and parts of commit 46e77f1118
"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.

win32/win32.c
win32/win32.h
win32/win32sck.c

index 53962b2..bf91b76 100644 (file)
@@ -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, ""};
@@ -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();
 }
 
index 19dcbf7..a521a41 100644 (file)
@@ -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
index 5f98910..674add2 100644 (file)
@@ -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)
 {
@@ -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();
@@ -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();