From 6f73eeccfea0849d6de687e4163c05f075598ec7 Mon Sep 17 00:00:00 2001 From: Chris Varnz Date: Fri, 19 Feb 2021 00:51:07 +0000 Subject: [PATCH] Fix crash in H5TS_win32_thread_exit Fix crash caused on thread exits when the following events occur: 1. HDF5 library is loaded 2. A thread is created 3. No HDF5 API's are called (so H5TS_win32_process_enter is never called, and H5TS_errstk_key_g and H5TS_funcstk_key_g are uninitialized and happen to be zero) 4. Some other library erroneously calls TlsSetValue with an index of zero (the same as our uninitialized H5TS_errstk_key_g and H5TS_funcstk_key_g keys) 5. The thread exits, calling H5TS_win32_thread_exit, which calls TlsGetValue with the uninitialized (zero) keys, gets a non-zero value back because of the other library previously calling TlsSetValue with that key, calls LocalFree on it and explodes with a heap error. The fix is simply to initialize the keys to some known invalid value (TLS_OUT_OF_INDEXES) and only cleanup if they were ever actually allocated with a call to TlsAlloc. --- src/H5TS.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/H5TS.c b/src/H5TS.c index 7d46e954434..54156bef884 100644 --- a/src/H5TS.c +++ b/src/H5TS.c @@ -31,12 +31,15 @@ typedef struct H5TS_cancel_struct { /* Global variable definitions */ #ifdef H5_HAVE_WIN_THREADS H5TS_once_t H5TS_first_init_g; +H5TS_key_t H5TS_errstk_key_g = TLS_OUT_OF_INDEXES; +H5TS_key_t H5TS_funcstk_key_g = TLS_OUT_OF_INDEXES; +H5TS_key_t H5TS_cancel_key_g = TLS_OUT_OF_INDEXES; #else /* H5_HAVE_WIN_THREADS */ H5TS_once_t H5TS_first_init_g = PTHREAD_ONCE_INIT; -#endif /* H5_HAVE_WIN_THREADS */ H5TS_key_t H5TS_errstk_key_g; H5TS_key_t H5TS_funcstk_key_g; H5TS_key_t H5TS_cancel_key_g; +#endif /* H5_HAVE_WIN_THREADS */ /*-------------------------------------------------------------------------- @@ -422,10 +425,16 @@ H5TS_win32_process_exit(void) DeleteCriticalSection(&H5_g.init_lock.CriticalSection); /* Clean up per-process thread local storage */ - TlsFree(H5TS_errstk_key_g); + if (H5TS_errstk_key_g != TLS_OUT_OF_INDEXES) + { + TlsFree(H5TS_errstk_key_g); + } #ifdef H5_HAVE_CODESTACK - TlsFree(H5TS_funcstk_key_g); + if (H5TS_funcstk_key_g != TLS_OUT_OF_INDEXES) + { + TlsFree(H5TS_funcstk_key_g); + } #endif /* H5_HAVE_CODESTACK */ return; @@ -461,14 +470,22 @@ H5TS_win32_thread_exit(void) */ /* Clean up per-thread thread local storage */ - lpvData = TlsGetValue(H5TS_errstk_key_g); - if(lpvData) - LocalFree((HLOCAL)lpvData); + if (H5TS_errstk_key_g != TLS_OUT_OF_INDEXES) + { + if (lpvData = TlsGetValue(H5TS_errstk_key_g)) + { + LocalFree((HLOCAL)lpvData); + } + } #ifdef H5_HAVE_CODESTACK - lpvData = TlsGetValue(H5TS_funcstk_key_g); - if(lpvData) - LocalFree((HLOCAL)lpvData); + if (H5TS_funcstk_key_g != TLS_OUT_OF_INDEXES) + { + if (lpvData = TlsGetValue(H5TS_funcstk_key_g)) + { + LocalFree((HLOCAL)lpvData); + } + } #endif /* H5_HAVE_CODESTACK */ return ret_value;