From 7d21afe83b40d1f9406fc2f54de52bc8aa392138 Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Fri, 1 Feb 2013 22:58:29 +0100 Subject: Fix unsafe accesses to driver data from win32 fd/vanilla/spawn drivers --- erts/emulator/sys/win32/sys.c | 66 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 13 deletions(-) (limited to 'erts/emulator/sys') diff --git a/erts/emulator/sys/win32/sys.c b/erts/emulator/sys/win32/sys.c index fe66dbd83b..307ca4866f 100755 --- a/erts/emulator/sys/win32/sys.c +++ b/erts/emulator/sys/win32/sys.c @@ -57,11 +57,13 @@ extern void _dosmaperr(DWORD); #define __argv e_argv #endif +typedef struct driver_data DriverData; + static void init_console(); static int get_and_remove_option(int* argc, char** argv, const char* option); static char *get_and_remove_option2(int *argc, char **argv, const char *option); -static int init_async_io(struct async_io* aio, int use_threads); +static int init_async_io(DriverData *dp, struct async_io* aio, int use_threads); static void release_async_io(struct async_io* aio, ErlDrvPort); static void async_read_file(struct async_io* aio, LPVOID buf, DWORD numToRead); static int async_write_file(struct async_io* aio, LPVOID buf, DWORD numToWrite); @@ -96,7 +98,7 @@ static erts_smp_atomic_t pipe_creation_counter; static int driver_write(long, HANDLE, byte*, int); static int create_file_thread(struct async_io* aio, int mode); #ifdef ERTS_SMP -static void close_active_handle(ErlDrvPort, HANDLE handle); +static void close_active_handle(DriverData *, HANDLE handle); static DWORD WINAPI threaded_handle_closer(LPVOID param); #endif static DWORD WINAPI threaded_reader(LPVOID param); @@ -440,6 +442,8 @@ typedef struct async_io { DWORD bytesTransferred; /* Bytes read or write in the last operation. * Valid only when DF_OVR_READY is set. */ + DriverData *dp; /* Pointer to driver data struct which + this struct is part of */ } AsyncIo; @@ -458,7 +462,7 @@ static BOOL (WINAPI *fpSetHandleInformation)(HANDLE,DWORD,DWORD); * none of the file handles. */ -typedef struct driver_data { +struct driver_data { int totalNeeded; /* Total number of bytes needed to fill * up the packet header or packet. */ int bytesInBuffer; /* Number of bytes read so far in @@ -476,7 +480,8 @@ typedef struct driver_data { AsyncIo in; /* Control block for overlapped reading. */ AsyncIo out; /* Control block for overlapped writing. */ int report_exit; /* Do report exit status for the port */ -} DriverData; + erts_atomic32_t refc; /* References to this struct */ +}; /* Driver interfaces */ static ErlDrvData spawn_start(ErlDrvPort, char*, SysDriverOpts*); @@ -581,6 +586,26 @@ struct erl_drv_entry vanilla_driver_entry = { stop_select }; +static ERTS_INLINE void +refer_driver_data(DriverData *dp) +{ +#ifdef DEBUG + erts_aint32_t refc = erts_atomic32_inc_read_nob(&dp->refc); + ASSERT(refc > 1); +#else + erts_atomic32_inc_nob(&dp->refc); +#endif +} + +static ERTS_INLINE void +unrefer_driver_data(DriverData *dp) +{ + erts_aint32_t refc = erts_atomic32_dec_read_mb(&dp->refc); + ASSERT(refc >= 0); + if (refc == 0) + driver_free(dp); +} + /* * Initialises a DriverData structure. * @@ -604,6 +629,7 @@ new_driver_data(ErlDrvPort port_num, int packet_bytes, int wait_objs_required, i * any more, since driver_select() can't fail. */ + erts_atomic32_init_nob(&dp->refc, 1); dp->bytesInBuffer = 0; dp->totalNeeded = packet_bytes; dp->inBufSize = PORT_BUFSIZ; @@ -616,9 +642,9 @@ new_driver_data(ErlDrvPort port_num, int packet_bytes, int wait_objs_required, i dp->port_num = port_num; dp->packet_bytes = packet_bytes; dp->port_pid = INVALID_HANDLE_VALUE; - if (init_async_io(&dp->in, use_threads) == -1) + if (init_async_io(dp, &dp->in, use_threads) == -1) goto async_io_error1; - if (init_async_io(&dp->out, use_threads) == -1) + if (init_async_io(dp, &dp->out, use_threads) == -1) goto async_io_error2; return dp; @@ -662,7 +688,7 @@ release_driver_data(DriverData* dp) dp->in.fd = INVALID_HANDLE_VALUE; DEBUGF(("Waiting for the in event thingie")); if (WaitForSingleObject(dp->in.ov.hEvent,timeout) == WAIT_TIMEOUT) { - close_active_handle(dp->port_num, dp->in.ov.hEvent); + close_active_handle(dp, dp->in.ov.hEvent); dp->in.ov.hEvent = NULL; timeout = 0; } @@ -673,7 +699,7 @@ release_driver_data(DriverData* dp) dp->out.fd = INVALID_HANDLE_VALUE; DEBUGF(("Waiting for the out event thingie")); if (WaitForSingleObject(dp->out.ov.hEvent,timeout) == WAIT_TIMEOUT) { - close_active_handle(dp->port_num, dp->out.ov.hEvent); + close_active_handle(dp, dp->out.ov.hEvent); dp->out.ov.hEvent = NULL; } DEBUGF(("...done\n")); @@ -719,7 +745,7 @@ release_driver_data(DriverData* dp) * the exit thread. */ - driver_free(dp); + unrefer_driver_data(dp); } #ifdef ERTS_SMP @@ -727,11 +753,12 @@ release_driver_data(DriverData* dp) struct handles_to_be_closed { HANDLE handles[MAXIMUM_WAIT_OBJECTS]; unsigned cnt; + DriverData *dp; }; static struct handles_to_be_closed* htbc_curr = NULL; CRITICAL_SECTION htbc_lock; -static void close_active_handle(ErlDrvPort port_num, HANDLE handle) +static void close_active_handle(DriverData *dp, HANDLE handle) { struct handles_to_be_closed* htbc; int i; @@ -745,6 +772,10 @@ static void close_active_handle(ErlDrvPort port_num, HANDLE handle) sizeof(*htbc)); htbc->handles[0] = CreateAutoEvent(FALSE); htbc->cnt = 1; + htbc->dp = dp; + refer_driver_data(dp); /* Need to keep driver data until we have + closed the event; outstanding operation + might write into it.. */ thread = (HANDLE *) _beginthreadex(NULL, 0, threaded_handle_closer, htbc, 0, &tid); CloseHandle(thread); } @@ -797,6 +828,7 @@ threaded_handle_closer(LPVOID param) } LeaveCriticalSection(&htbc_lock); CloseHandle(htbc->handles[0]); + unrefer_driver_data(htbc->dp); erts_free(ERTS_ALC_T_DRV_TAB, htbc); DEBUGF(("threaded_handle_closer %p terminating\r\n", htbc)); return 0; @@ -863,8 +895,9 @@ reuse_driver_data(DriverData *dp, HANDLE ifd, HANDLE ofd, int read_write, ErlDrv */ static int -init_async_io(AsyncIo* aio, int use_threads) +init_async_io(DriverData *dp, AsyncIo* aio, int use_threads) { + aio->dp = dp; aio->flags = 0; aio->thread = (HANDLE) -1; aio->fd = INVALID_HANDLE_VALUE; @@ -1282,12 +1315,15 @@ create_file_thread(AsyncIo* aio, int mode) { DWORD tid; /* Id for thread. */ + refer_driver_data(aio->dp); aio->thread = (HANDLE) _beginthreadex(NULL, 0, (mode & DO_WRITE) ? threaded_writer : threaded_reader, aio, 0, &tid); - - return aio->thread != (HANDLE) -1; + if (aio->thread != (HANDLE) -1) + return 1; + unrefer_driver_data(aio->dp); + return 0; } /* @@ -2073,6 +2109,7 @@ threaded_reader(LPVOID param) if (aio->flags & DF_EXIT_THREAD) break; } + unrefer_driver_data(aio->dp); return 0; } @@ -2152,6 +2189,7 @@ threaded_writer(LPVOID param) } CloseHandle(aio->fd); aio->fd = INVALID_HANDLE_VALUE; + unrefer_driver_data(aio->dp); return 0; } @@ -2361,6 +2399,8 @@ stop(ErlDrvData data) */ HANDLE thread; DWORD tid; + + /* threaded_exiter implicitly takes over refc from us... */ thread = (HANDLE *) _beginthreadex(NULL, 0, threaded_exiter, dp, 0, &tid); CloseHandle(thread); } -- cgit v1.2.3