summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRainer Gerhards <rgerhards@adiscon.com>2009-09-24 10:49:11 +0200
committerRainer Gerhards <rgerhards@adiscon.com>2009-09-24 10:49:11 +0200
commite0dde79a806f46790df6104a6bacdbddaa938fd6 (patch)
tree25a0401099cfd9bec8e2d342a40d14e2ec981bcb
parent44844b8af3be216cec340c8ff83fdc6f1d6f1fff (diff)
downloadrsyslog-e0dde79a806f46790df6104a6bacdbddaa938fd6.tar.gz
rsyslog-e0dde79a806f46790df6104a6bacdbddaa938fd6.tar.xz
rsyslog-e0dde79a806f46790df6104a6bacdbddaa938fd6.zip
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.
-rw-r--r--ChangeLog8
-rw-r--r--runtime/stream.c14
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)