From e0dde79a806f46790df6104a6bacdbddaa938fd6 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 24 Sep 2009 10:49:11 +0200 Subject: bugfix: potential segfault in stream writer on destruction Most severely affected omfile. The problem was that some buffers were freed before the asynchronous writer thread was shut down. So the writer thread accessed invalid data, which may even already be overwritten. Symptoms (with omfile) were segfaults, grabled data and files with random names placed around the file system (most prominently into the root directory). Special thanks to Aaron for helping to track this down. --- ChangeLog | 8 ++++++++ runtime/stream.c | 14 +++++++++----- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index a12f258e..7c6985b7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ --------------------------------------------------------------------------- Version 4.5.4 [v4-beta] (rgerhards), 2009-09-?? +- bugfix: potential segfault in stream writer on destruction + Most severely affected omfile. The problem was that some buffers were + freed before the asynchronous writer thread was shut down. So the + writer thread accessed invalid data, which may even already be + overwritten. Symptoms (with omfile) were segfaults, grabled data + and files with random names placed around the file system (most + prominently into the root directory). Special thanks to Aaron for + helping to track this down. - bugfix: potential race in object loader (obj.c) during use/release of object interface - bugfixes: potential problems in out file zip writer. Problems could diff --git a/runtime/stream.c b/runtime/stream.c index 32467082..9e88eca1 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -685,11 +685,6 @@ dbgprintf("XXX: destruct stream %p\n", pThis); if(pThis->fd != -1) strmCloseFile(pThis); - free(pThis->pszDir); - free(pThis->pZipBuf); - free(pThis->pszCurrFName); - free(pThis->pszFName); - if(pThis->bAsyncWrite) { stopWriter(pThis); pthread_mutex_destroy(&pThis->mut); @@ -702,6 +697,15 @@ dbgprintf("XXX: destruct stream %p\n", pThis); } else { free(pThis->pIOBuf); } + + /* IMPORTANT: we MUST free this only AFTER the ansyncWriter has been stopped, else + * we get random errors... + */ + free(pThis->pszDir); + free(pThis->pZipBuf); + free(pThis->pszCurrFName); + free(pThis->pszFName); + ENDobjDestruct(strm) -- cgit