From 7b456ce548030ff362d3a2be04b1e5c2c89e0dcb Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 14 Aug 2008 11:19:02 +0200 Subject: bugfix: imfile could cause a segfault upon rsyslogd HUP and termination Thanks to lperr for an excellent bug report that helped detect this problem. --- ChangeLog | 3 +++ configure.ac | 2 +- plugins/imfile/imfile.c | 9 +++++++-- threads.c | 12 ++++++++---- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8933f88a..ff45137b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ --------------------------------------------------------------------------- Version 3.18.3 (rgerhards), 2008-08-?? +- bugfix: imfile could cause a segfault upon rsyslogd HUP and termination + Thanks to lperr for an excellent bug report that helped detect this + problem. - enhanced ommysql to support custom port to connect to server Port can be set via new $ActionOmmysqlServerPort config directive Note: this was a very minor change and thus deemed appropriate to be diff --git a/configure.ac b/configure.ac index caffe01e..b5427ab9 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[3.18.3-Test3],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.18.3-Test4],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([syslogd.c]) AC_CONFIG_HEADERS([config.h]) diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c index 75e54f04..6b7f516e 100644 --- a/plugins/imfile/imfile.c +++ b/plugins/imfile/imfile.c @@ -182,8 +182,9 @@ static void pollFileCancelCleanup(void *pArg) /* poll a file, need to check file rollover etc. open file if not open */ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) { - DEFiRet; cstr_t *pCStr = NULL; + int bMustPopCleanup = 0; + DEFiRet; ASSERT(pbHadFileData != NULL); @@ -192,6 +193,8 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) } pthread_cleanup_push(pollFileCancelCleanup, &pCStr); + bMustPopCleanup = 1; + /* loop below will be exited when strmReadLine() returns EOF */ while(1) { CHKiRet(strmReadLine(pThis->pStrm, &pCStr)); @@ -199,9 +202,11 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) CHKiRet(enqLine(pThis, pCStr)); /* process line */ rsCStrDestruct(&pCStr); /* discard string (must be done by us!) */ } - pthread_cleanup_pop(0); finalize_it: + if(bMustPopCleanup) + pthread_cleanup_pop(0); + if(pCStr != NULL) { rsCStrDestruct(&pCStr); } diff --git a/threads.c b/threads.c index e32ff0d9..03f19ff9 100644 --- a/threads.c +++ b/threads.c @@ -46,6 +46,7 @@ static linkedList_t llThrds; */ static rsRetVal thrdConstruct(thrdInfo_t **ppThis) { + DEFiRet; thrdInfo_t *pThis; assert(ppThis != NULL); @@ -60,7 +61,7 @@ static rsRetVal thrdConstruct(thrdInfo_t **ppThis) pthread_mutex_init (pThis->mutTermOK, NULL); *ppThis = pThis; - return RS_RET_OK; + RETiRet; } @@ -70,6 +71,7 @@ static rsRetVal thrdConstruct(thrdInfo_t **ppThis) */ static rsRetVal thrdDestruct(thrdInfo_t *pThis) { + DEFiRet; assert(pThis != NULL); if(pThis->bIsActive == 1) { @@ -78,7 +80,7 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis) free(pThis->mutTermOK); free(pThis); - return RS_RET_OK; + RETiRet; } @@ -86,6 +88,7 @@ static rsRetVal thrdDestruct(thrdInfo_t *pThis) */ rsRetVal thrdTerminate(thrdInfo_t *pThis) { + DEFiRet; assert(pThis != NULL); pthread_cancel(pThis->thrdID); @@ -96,7 +99,7 @@ rsRetVal thrdTerminate(thrdInfo_t *pThis) if(pThis->pAfterRun != NULL) pThis->pAfterRun(pThis); - return RS_RET_OK; + RETiRet; } @@ -104,8 +107,9 @@ rsRetVal thrdTerminate(thrdInfo_t *pThis) */ rsRetVal thrdTerminateAll(void) { + DEFiRet; llDestroy(&llThrds); - return RS_RET_OK; + RETiRet; } -- cgit From bf3a0f72885e2f171678197271ea6248a8614d98 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 15 Aug 2008 12:16:49 +0200 Subject: updated syslog-ng comparison thanks to William --- doc/rsyslog_ng_comparison.html | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/rsyslog_ng_comparison.html b/doc/rsyslog_ng_comparison.html index 28413337..aac543a7 100644 --- a/doc/rsyslog_ng_comparison.html +++ b/doc/rsyslog_ng_comparison.html @@ -424,11 +424,11 @@ directories (log targets) dynamically control of log output format, including ability to present channel and priority as visible log data yes -not sure... +yes native ability to send mail messages yes (ommail, introduced in 3.17.0) -not sure... +no (only via piped external process) good timestamp format control; at a @@ -578,6 +578,6 @@ feature sheet. I have not yet been able to fully work through it. In the mean time, you may want to read it in parallel. It is available at Balabit's site.

-

This document is current as of 2008-04-08 and definitely +

This document is current as of 2008-08-15 and definitely incomplete (I did not yet manage to complete it!).

- \ No newline at end of file + -- cgit From 3aa86ed554aeb05c386e99d605ddc220250d35d2 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 15 Aug 2008 12:21:26 +0200 Subject: fixed cross-platform compile problem introduced with recent change ...which fixed the imfile segfault issue. --- configure.ac | 2 +- plugins/imfile/imfile.c | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index b5427ab9..cb505367 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[3.18.3-Test4],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.18.3-Test6],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([syslogd.c]) AC_CONFIG_HEADERS([config.h]) diff --git a/plugins/imfile/imfile.c b/plugins/imfile/imfile.c index 6b7f516e..2df1aaf7 100644 --- a/plugins/imfile/imfile.c +++ b/plugins/imfile/imfile.c @@ -183,18 +183,18 @@ static void pollFileCancelCleanup(void *pArg) static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) { cstr_t *pCStr = NULL; - int bMustPopCleanup = 0; DEFiRet; ASSERT(pbHadFileData != NULL); + /* Note: we must do pthread_cleanup_push() immediately, because the POXIS macros + * otherwise do not work if I include the _cleanup_pop() inside an if... -- rgerhards, 2008-08-14 + */ + pthread_cleanup_push(pollFileCancelCleanup, &pCStr); if(pThis->pStrm == NULL) { CHKiRet(openFile(pThis)); /* open file */ } - pthread_cleanup_push(pollFileCancelCleanup, &pCStr); - bMustPopCleanup = 1; - /* loop below will be exited when strmReadLine() returns EOF */ while(1) { CHKiRet(strmReadLine(pThis->pStrm, &pCStr)); @@ -204,8 +204,15 @@ static rsRetVal pollFile(fileInfo_t *pThis, int *pbHadFileData) } finalize_it: - if(bMustPopCleanup) - pthread_cleanup_pop(0); + /*EMPTY - just to keep the compiler happy, do NOT remove*/; + /* Note: the problem above is that pthread:cleanup_pop() is a macro which + * evaluates to something like "} while(0);". So the code would become + * "finalize_it: }", that is a label without a statement. The C standard does + * not permit this. So we add an empty statement "finalize_it: ; }" and + * everybody is happy. Note that without the ;, an error is reported only + * on some platforms/compiler versions. -- rgerhards, 2008-08-15 + */ + pthread_cleanup_pop(0); if(pCStr != NULL) { rsCStrDestruct(&pCStr); -- cgit From 3f2856b4b5010dfcaa720b292dc3a655e7b9f6da Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 15 Aug 2008 12:54:42 +0200 Subject: preparing for 3.18.3 --- ChangeLog | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index ff45137b..943d4bf7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,5 @@ --------------------------------------------------------------------------- -Version 3.18.3 (rgerhards), 2008-08-?? +Version 3.18.3 (rgerhards), 2008-08-18 - bugfix: imfile could cause a segfault upon rsyslogd HUP and termination Thanks to lperr for an excellent bug report that helped detect this problem. diff --git a/configure.ac b/configure.ac index cb505367..314dc10f 100644 --- a/configure.ac +++ b/configure.ac @@ -2,7 +2,7 @@ # Process this file with autoconf to produce a configure script. AC_PREREQ(2.61) -AC_INIT([rsyslog],[3.18.3-Test6],[rsyslog@lists.adiscon.com]) +AC_INIT([rsyslog],[3.18.3],[rsyslog@lists.adiscon.com]) AM_INIT_AUTOMAKE AC_CONFIG_SRCDIR([syslogd.c]) AC_CONFIG_HEADERS([config.h]) -- cgit