From 50855878062dbcf8d2861ea169d3ca686c65b23b Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 17 Dec 2010 18:18:57 +0100 Subject: bugfix: imfile potentially duplicates lines This can happen when 0 bytes are read from the input file, and some writer appends data to the file BEFORE we check if a rollover happens. The check for rollover uses the inode and size as a criterion. So far, we checked for equality of sizes, which is not given in this scenario, but that does not indicate a rollover. From the source code comments: Note that when we check the size, we MUST NOT check for equality. The reason is that the file may have been written right after we did try to read (so the file size has increased). That is NOT in indicator of a rollover (this is an actual bug scenario we experienced). So we need to check if the new size is smaller than what we already have seen! --- ChangeLog | 12 ++++++++++++ runtime/stream.c | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/ChangeLog b/ChangeLog index 3c7d3c4c..951a0703 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,17 @@ --------------------------------------------------------------------------- Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? +- bugfix: imfile potentially duplicates lines + This can happen when 0 bytes are read from the input file, and some + writer appends data to the file BEFORE we check if a rollover happens. + The check for rollover uses the inode and size as a criterion. So far, + we checked for equality of sizes, which is not given in this scenario, + but that does not indicate a rollover. From the source code comments: + Note that when we check the size, we MUST NOT check for equality. + The reason is that the file may have been written right after we + did try to read (so the file size has increased). That is NOT in + indicator of a rollover (this is an actual bug scenario we + experienced). So we need to check if the new size is smaller than + what we already have seen! - bugfix: a couple of problems that imfile had on some platforms, namely Ubuntu (not their fault, but occured there) - bugfix: imfile utilizes 32 bit to track offset. Most importantly, diff --git a/runtime/stream.c b/runtime/stream.c index 869324ec..baf69881 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -422,10 +422,15 @@ strmHandleEOFMonitor(strm_t *pThis) ABORT_FINALIZE(RS_RET_IO_ERROR); if(stat((char*) pThis->pszCurrFName, &statName) == -1) ABORT_FINALIZE(RS_RET_IO_ERROR); + DBGPRINTF("stream checking for file change on '%s', inode %u/%u, size %lld/%lld\n", + pThis->pszCurrFName, (unsigned) statOpen.st_ino, + (unsigned) statName.st_ino, + pThis->iCurrOffs, (long long) statName.st_size); if(statOpen.st_ino == statName.st_ino && pThis->iCurrOffs == statName.st_size) { ABORT_FINALIZE(RS_RET_EOF); } else { /* we had a file change! */ + DBGPRINTF("we had a file change on '%s'\n", pThis->pszCurrFName); CHKiRet(strmCloseFile(pThis)); CHKiRet(strmOpenFile(pThis)); } -- cgit v1.2.3 From 25ff60f5238204ddde0eb7b76ca346372f496ba7 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 17 Dec 2010 18:34:39 +0100 Subject: forgot the actual patch with the last commit :( --- runtime/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/stream.c b/runtime/stream.c index baf69881..a2eeb039 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -426,7 +426,7 @@ strmHandleEOFMonitor(strm_t *pThis) pThis->pszCurrFName, (unsigned) statOpen.st_ino, (unsigned) statName.st_ino, pThis->iCurrOffs, (long long) statName.st_size); - if(statOpen.st_ino == statName.st_ino && pThis->iCurrOffs == statName.st_size) { + if(statOpen.st_ino == statName.st_ino && pThis->iCurrOffs >= statName.st_size) { ABORT_FINALIZE(RS_RET_EOF); } else { /* we had a file change! */ -- cgit v1.2.3 From cdc27aea8f9be98b3f886532191c73bfbc42b8a4 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 10 Jan 2011 08:22:50 +0100 Subject: imfile bugfix: potential duplication of log content Under some circumstances an invalid truncation was detected. This code has now been removed, a file change (and thus resent) is only detected if the inode number changes. --- ChangeLog | 3 +++ runtime/stream.c | 22 +++++++++------------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 951a0703..571c8bcb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,9 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? indicator of a rollover (this is an actual bug scenario we experienced). So we need to check if the new size is smaller than what we already have seen! + Also, under some circumstances an invalid truncation was detected. This + code has now been removed, a file change (and thus resent) is only + detected if the inode number changes. - bugfix: a couple of problems that imfile had on some platforms, namely Ubuntu (not their fault, but occured there) - bugfix: imfile utilizes 32 bit to track offset. Most importantly, diff --git a/runtime/stream.c b/runtime/stream.c index a2eeb039..af38bcb7 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -400,6 +400,12 @@ finalize_it: * If we are monitoring a file, someone may have rotated it. In this case, we * also need to close it and reopen it under the same name. * rgerhards, 2008-02-13 + * The previous code also did a check for file truncation, in which case the + * file was considered rewritten. However, this potential border case turned + * out to be a big trouble spot on busy systems. It caused massive message + * duplication (I guess stat() can return a too-low number under some + * circumstances). So starting as of now, we only check the inode number and + * a file change is detected only if the inode changes. -- rgerhards, 2011-01-10 */ static rsRetVal strmHandleEOFMonitor(strm_t *pThis) @@ -409,24 +415,14 @@ strmHandleEOFMonitor(strm_t *pThis) struct stat statName; ISOBJ_TYPE_assert(pThis, strm); - /* find inodes of both current descriptor as well as file now in file - * system. If they are different, the file has been rotated (or - * otherwise rewritten). We also check the size, because the inode - * does not change if the file is truncated (this, BTW, is also a case - * where we actually loose log lines, because we can not do anything - * against truncation...). We do NOT rely on the time of last - * modificaton because that may not be available under all - * circumstances. -- rgerhards, 2008-02-13 - */ if(fstat(pThis->fd, &statOpen) == -1) ABORT_FINALIZE(RS_RET_IO_ERROR); if(stat((char*) pThis->pszCurrFName, &statName) == -1) ABORT_FINALIZE(RS_RET_IO_ERROR); - DBGPRINTF("stream checking for file change on '%s', inode %u/%u, size %lld/%lld\n", + DBGPRINTF("stream checking for file change on '%s', inode %u/%u", pThis->pszCurrFName, (unsigned) statOpen.st_ino, - (unsigned) statName.st_ino, - pThis->iCurrOffs, (long long) statName.st_size); - if(statOpen.st_ino == statName.st_ino && pThis->iCurrOffs >= statName.st_size) { + (unsigned) statName.st_ino); + if(statOpen.st_ino == statName.st_ino) { ABORT_FINALIZE(RS_RET_EOF); } else { /* we had a file change! */ -- cgit v1.2.3 From 6bad782f154b7f838c7371bf99c13f6dc4ec4101 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 10 Feb 2011 17:54:09 +0100 Subject: bugfix: abort if imfile reads file line of more than 64KiB Thanks to Peter Eisentraut for reporting and analysing this problem. bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=221 --- ChangeLog | 3 +++ runtime/stringbuf.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 571c8bcb..3f478e0d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,9 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? - bugfix: imfile utilizes 32 bit to track offset. Most importantly, this problem can not experienced on Fedora 64 bit OS (which has 64 bit long's!) +- bugfix: abort if imfile reads file line of more than 64KiB + Thanks to Peter Eisentraut for reporting and analysing this problem. + bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=221 - some improvements thanks to clang's static code analyzer o overall cleanup (mostly unnecessary writes and otherwise unused stuff) o bugfix: fixed a very remote problem in msg.c which could occur when diff --git a/runtime/stringbuf.c b/runtime/stringbuf.c index 93995b38..8b2fe455 100644 --- a/runtime/stringbuf.c +++ b/runtime/stringbuf.c @@ -156,7 +156,7 @@ rsRetVal rsCStrExtendBuf(cstr_t *pThis, size_t iMinNeeded) { uchar *pNewBuf; - unsigned short iNewSize; + size_t iNewSize; DEFiRet; /* first compute the new size needed */ -- cgit v1.2.3 From c1760db6bbd07086177679b2be4b2d307657ddce Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 9 Mar 2011 13:01:57 +0100 Subject: bugfix: omlibdbi did not use password from rsyslog.con closes: http://bugzilla.adiscon.com/show_bug.cgi?id=203 --- ChangeLog | 2 ++ plugins/omlibdbi/omlibdbi.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 3f478e0d..917ce51f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -23,6 +23,8 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? - bugfix: abort if imfile reads file line of more than 64KiB Thanks to Peter Eisentraut for reporting and analysing this problem. bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=221 +- bugfix: omlibdbi did not use password from rsyslog.con + closes: http://bugzilla.adiscon.com/show_bug.cgi?id=203 - some improvements thanks to clang's static code analyzer o overall cleanup (mostly unnecessary writes and otherwise unused stuff) o bugfix: fixed a very remote problem in msg.c which could occur when diff --git a/plugins/omlibdbi/omlibdbi.c b/plugins/omlibdbi/omlibdbi.c index 6f130f54..67b1edf9 100644 --- a/plugins/omlibdbi/omlibdbi.c +++ b/plugins/omlibdbi/omlibdbi.c @@ -287,7 +287,7 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1) if(dbName != NULL) if((pData->dbName = (uchar*) strdup((char*)dbName)) == NULL) ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); if(pwd != NULL) - if((pData->pwd = (uchar*) strdup((char*)"")) == NULL) ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); + if((pData->pwd = (uchar*) strdup((char*)pwd)) == NULL) ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); CHKiRet(cflineParseTemplateName(&p, *ppOMSR, 0, OMSR_RQD_TPL_OPT_SQL, (uchar*) " StdDBFmt")); -- cgit v1.2.3 From 765317d54ec5658dd0c598170d0fc0824f751265 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 10 Mar 2011 15:13:31 +0100 Subject: improved tcpflood test tool to support many more connections even on platforms that place a low limit on the number of file descriptors per processes. The tool now increases the fd limit as required. --- tests/tcpflood.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/tcpflood.c b/tests/tcpflood.c index d8c3f038..138706aa 100644 --- a/tests/tcpflood.c +++ b/tests/tcpflood.c @@ -62,6 +62,7 @@ #include #include #include +#include #define EXIT_FAILURE 1 #define INVALID_SOCKET -1 @@ -377,6 +378,7 @@ int main(int argc, char *argv[]) int ret = 0; int opt; struct sigaction sigAct; + struct rlimit maxFiles; static char buf[1024]; srand(time(NULL)); /* seed is good enough for our needs */ @@ -439,6 +441,18 @@ int main(int argc, char *argv[]) } } + if(numConnections > 100) { + maxFiles.rlim_cur = numConnections + 50; + maxFiles.rlim_max = numConnections + 50; + if(setrlimit(RLIMIT_NOFILE, &maxFiles) < 0) { + perror("set number of open files"); + fprintf(stderr, + "could net set sufficiently large number of " + "open files for required connection count!\n"); + exit(1); + } + } + if(dataFile != NULL) { if((dataFP = fopen(dataFile, "r")) == NULL) { perror(dataFile); -- cgit v1.2.3 From 4195f9cb08cddaeae37dc88c5e879d84391b4359 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 11 Mar 2011 10:58:07 +0100 Subject: added new test for imfile to testbench --- tests/Makefile.am | 11 ++++++++++- tests/diag.sh | 2 ++ tests/imfile-basic.sh | 14 ++++++++++++++ tests/inputfilegen.c | 23 +++++++++++++++++++++++ tests/random.sh | 4 +--- tests/testsuites/imfile-basic.conf | 12 ++++++++++++ 6 files changed, 62 insertions(+), 4 deletions(-) create mode 100755 tests/imfile-basic.sh create mode 100644 tests/inputfilegen.c create mode 100644 tests/testsuites/imfile-basic.conf diff --git a/tests/Makefile.am b/tests/Makefile.am index 0045f00a..a438a926 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,6 +1,6 @@ if ENABLE_TESTBENCH TESTRUNS = rt_init rscript -check_PROGRAMS = $(TESTRUNS) ourtail nettester tcpflood chkseq randomgen +check_PROGRAMS = $(TESTRUNS) ourtail nettester tcpflood chkseq randomgen inputfilegen TESTS = $(TESTRUNS) cfg.sh \ validation-run.sh \ imtcp-multiport.sh \ @@ -44,6 +44,10 @@ if ENABLE_EXTENDED_TESTS TESTS += random.sh endif +if ENABLE_IMFILE +TESTS += imfile-basic.sh +endif + check_JAVA = DiagTalker.java endif # if ENABLE_TESTBENCH @@ -187,6 +191,8 @@ EXTRA_DIST= 1.rstest 2.rstest 3.rstest err1.rstest \ testsuites/complex1.conf \ random.sh \ testsuites/random.conf \ + imfile-basic.sh \ + testsuites/imfile-basic.conf \ dynfile_invld_async.sh \ dynfile_invld_sync.sh \ dynfile_cachemiss.sh \ @@ -213,6 +219,9 @@ tcpflood_LDADD = $(SOL_LIBS) randomgen_SOURCES = randomgen.c randomgen_LDADD = $(SOL_LIBS) +inputfilegen_SOURCES = inputfilegen.c +inputfilegen_LDADD = $(SOL_LIBS) + nettester_SOURCES = nettester.c getline.c nettester_LDADD = $(SOL_LIBS) diff --git a/tests/diag.sh b/tests/diag.sh index 51ad5f6a..806edf80 100755 --- a/tests/diag.sh +++ b/tests/diag.sh @@ -22,6 +22,7 @@ case $1 in rm -f work rsyslog.out.log rsyslog.out.log.save # common work files rm -f rsyslog.out.*.log work-presort rm -rf test-spool + rm -f rsyslog.input rm -f core.* vgcore.* mkdir test-spool ;; @@ -30,6 +31,7 @@ case $1 in rm -f work rsyslog.out.log rsyslog.out.log.save # common work files rm -f rsyslog.out.*.log rsyslog.random.data work-presort rm -rf test-spool + rm -f rsyslog.input ;; 'startup') # start rsyslogd with default params. $2 is the config file name to use # returns only after successful startup, $3 is the instance (blank or 2!) diff --git a/tests/imfile-basic.sh b/tests/imfile-basic.sh new file mode 100755 index 00000000..ca6a5d3a --- /dev/null +++ b/tests/imfile-basic.sh @@ -0,0 +1,14 @@ +# This is part of the rsyslog testbench, licensed under GPLv3 +echo [imfile-basic.sh] +source $srcdir/diag.sh init +# generate input file first. Note that rsyslog processes it as +# soon as it start up (so the file should exist at that point). +./inputfilegen 50000 > rsyslog.input +ls -l rsyslog.input +source $srcdir/diag.sh startup imfile-basic.conf +# sleep a little to give rsyslog a chance to begin processing +sleep 1 +source $srcdir/diag.sh shutdown-when-empty # shut down rsyslogd when done processing messages +source $srcdir/diag.sh wait-shutdown # we need to wait until rsyslogd is finished! +source $srcdir/diag.sh seq-check 0 49999 +source $srcdir/diag.sh exit diff --git a/tests/inputfilegen.c b/tests/inputfilegen.c new file mode 100644 index 00000000..26fb79af --- /dev/null +++ b/tests/inputfilegen.c @@ -0,0 +1,23 @@ +/* generate an input file suitable for use by the testbench + * Copyright (C) 2011 by Rainer Gerhards and Adiscon GmbH. + * Part of rsyslog, licensed under GPLv3 + */ +#include +#include + +int main(int argc, char* argv[]) +{ + int nmsgs; + int i; + + if(argc != 2) { + fprintf(stderr, "usage: inputfilegen num-messages\n"); + return 1; + } + + nmsgs = atoi(argv[1]); + for(i = 0 ; i < nmsgs ; ++i) { + printf("msgnum:%8.8d:\n", i); + } + return 0; +} diff --git a/tests/random.sh b/tests/random.sh index d1f392d3..969d720c 100755 --- a/tests/random.sh +++ b/tests/random.sh @@ -5,9 +5,6 @@ echo =============================================================================== echo TEST: \[random.sh\]: testing random data source $srcdir/diag.sh init -# uncomment for debugging support: -#export RSYSLOG_DEBUG="debug nostdout noprintmutexaction" -#export RSYSLOG_DEBUGLOG="log" source $srcdir/diag.sh startup random.conf # generate random data ./randomgen -f rsyslog.random.data -s 100000 @@ -17,4 +14,5 @@ source $srcdir/diag.sh shutdown-when-empty # shut down rsyslogd when done proces source $srcdir/diag.sh wait-shutdown # and wait for it to terminate # we do not check anything yet, the point is if rsyslog survived ;) # TODO: check for exit message, but we'll notice an abort anyhow, so not that important +rm -f random.data source $srcdir/diag.sh exit diff --git a/tests/testsuites/imfile-basic.conf b/tests/testsuites/imfile-basic.conf new file mode 100644 index 00000000..9fb9b5ca --- /dev/null +++ b/tests/testsuites/imfile-basic.conf @@ -0,0 +1,12 @@ +$IncludeConfig diag-common.conf + +$ModLoad ../plugins/imfile/.libs/imfile +$InputFileName ./rsyslog.input +$InputFileTag file: +$InputFileStateFile stat-file1 +$InputFileSeverity error +$InputFileFacility local7 +$InputRunFileMonitor + +$template outfmt,"%msg:F,58:2%\n" +:msg, contains, "msgnum:" ./rsyslog.out.log;outfmt -- cgit v1.2.3 From e7deab65dcad38a613c749e44e36c6c795000867 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 14 Apr 2011 12:47:04 +0200 Subject: bugfix: IPv6-address could not be specified in omrelp this was due to improper parsing of ":" closes: http://bugzilla.adiscon.com/show_bug.cgi?id=250 --- ChangeLog | 3 +++ doc/omrelp.html | 1 + plugins/omrelp/omrelp.c | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 917ce51f..e63c5d72 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ --------------------------------------------------------------------------- Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? +- bugfix: IPv6-address could not be specified in omrelp + this was due to improper parsing of ":" + closes: http://bugzilla.adiscon.com/show_bug.cgi?id=250 - bugfix: imfile potentially duplicates lines This can happen when 0 bytes are read from the input file, and some writer appends data to the file BEFORE we check if a rollover happens. diff --git a/doc/omrelp.html b/doc/omrelp.html index b3132d78..22e6845f 100644 --- a/doc/omrelp.html +++ b/doc/omrelp.html @@ -44,6 +44,7 @@ special "RSYSLOG_ForwardFormat" (case sensitive!) template is used.
# port 2514 *.* :omrelp:centralserv:2514;RSYSLOG_ForwardFormat +Note: to use IPv6 addresses, encode them in [::1] format.

[rsyslog.conf overview] [manual index] [rsyslog site]

This documentation is part of the diff --git a/plugins/omrelp/omrelp.c b/plugins/omrelp/omrelp.c index d5ef8b4f..4a21627d 100644 --- a/plugins/omrelp/omrelp.c +++ b/plugins/omrelp/omrelp.c @@ -249,8 +249,18 @@ CODE_STD_STRING_REQUESTparseSelectorAct(1) /* extract the host first (we do a trick - we replace the ';' or ':' with a '\0') * now skip to port and then template name. rgerhards 2005-07-06 */ - for(q = p ; *p && *p != ';' && *p != ':' ; ++p) - /* JUST SKIP */; + if(*p == '[') { /* everything is hostname upto ']' */ + ++p; /* skip '[' */ + for(q = p ; *p && *p != ']' ; ++p) + /* JUST SKIP */; + if(*p == ']') { + *p = '\0'; /* trick to obtain hostname (later)! */ + ++p; /* eat it */ + } + } else { /* traditional view of hostname */ + for(q = p ; *p && *p != ';' && *p != ':' && *p != '#' ; ++p) + /* JUST SKIP */; + } pData->port = NULL; if(*p == ':') { /* process port */ -- cgit v1.2.3 From d13ec67f9ab9c59a88952af6aec372fd082401ca Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 2 May 2011 12:03:42 +0200 Subject: slightly more informative errmsg when TCP connection is aborted --- ChangeLog | 2 ++ runtime/strmsrv.c | 6 ++++-- tcpsrv.c | 8 +++++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/ChangeLog b/ChangeLog index e63c5d72..6adcc29e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,8 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=221 - bugfix: omlibdbi did not use password from rsyslog.con closes: http://bugzilla.adiscon.com/show_bug.cgi?id=203 +- bugfix: a slightly more informative error message when a TCP + connections is aborted - some improvements thanks to clang's static code analyzer o overall cleanup (mostly unnecessary writes and otherwise unused stuff) o bugfix: fixed a very remote problem in msg.c which could occur when diff --git a/runtime/strmsrv.c b/runtime/strmsrv.c index 3dc53a97..8cebf810 100644 --- a/runtime/strmsrv.c +++ b/runtime/strmsrv.c @@ -520,6 +520,7 @@ Run(strmsrv_t *pThis) strms_sess_t *pNewSess; nssel_t *pSel; ssize_t iRcvd; + rsRetVal localRet; ISOBJ_TYPE_assert(pThis, strmsrv); @@ -579,11 +580,12 @@ Run(strmsrv_t *pThis) break; case RS_RET_OK: /* valid data received, process it! */ - if(strms_sess.DataRcvd(pThis->pSessions[iSTRMSess], buf, iRcvd) != RS_RET_OK) { + localRet = strms_sess.DataRcvd(pThis->pSessions[iSTRMSess], buf, iRcvd); + if(localRet != RS_RET_OK) { /* in this case, something went awfully wrong. * We are instructed to terminate the session. */ - errmsg.LogError(0, NO_ERRCODE, "Tearing down STRM Session %d - see " + errmsg.LogError(0, localRet, "Tearing down STRM Session %d - see " "previous messages for reason(s)\n", iSTRMSess); pThis->pOnErrClose(pThis->pSessions[iSTRMSess]); strms_sess.Destruct(&pThis->pSessions[iSTRMSess]); diff --git a/tcpsrv.c b/tcpsrv.c index 0102bee7..5de805b2 100644 --- a/tcpsrv.c +++ b/tcpsrv.c @@ -471,6 +471,7 @@ static rsRetVal Run(tcpsrv_t *pThis) { DEFiRet; + rsRetVal localRet; int nfds; int i; int iTCPSess; @@ -545,12 +546,13 @@ Run(tcpsrv_t *pThis) break; case RS_RET_OK: /* valid data received, process it! */ - if(tcps_sess.DataRcvd(pThis->pSessions[iTCPSess], buf, iRcvd) != RS_RET_OK) { + localRet = tcps_sess.DataRcvd(pThis->pSessions[iTCPSess], buf, iRcvd); + if(localRet != RS_RET_OK) { /* in this case, something went awfully wrong. * We are instructed to terminate the session. */ - errmsg.LogError(0, NO_ERRCODE, "Tearing down TCP Session %d - see " - "previous messages for reason(s)\n", iTCPSess); + errmsg.LogError(0, localRet, "Tearing down TCP Session %d - see " + "previous messages for reason(s)", iTCPSess); pThis->pOnErrClose(pThis->pSessions[iTCPSess]); tcps_sess.Destruct(&pThis->pSessions[iTCPSess]); } -- cgit v1.2.3 From 921bebc8ee203c0569332c636515c4de036dfcf9 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 2 May 2011 12:43:58 +0200 Subject: bugfix: TCP connection invalidly aborted when messages needed to be discarded due to QUEUE_FULL or similar problem --- ChangeLog | 2 ++ tcpsrv.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 6adcc29e..16928921 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,8 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=221 - bugfix: omlibdbi did not use password from rsyslog.con closes: http://bugzilla.adiscon.com/show_bug.cgi?id=203 +- bugfix: TCP connection invalidly aborted when messages needed to be + discarded (due to QUEUE_FULL or similar problem) - bugfix: a slightly more informative error message when a TCP connections is aborted - some improvements thanks to clang's static code analyzer diff --git a/tcpsrv.c b/tcpsrv.c index 5de805b2..0581828e 100644 --- a/tcpsrv.c +++ b/tcpsrv.c @@ -547,7 +547,7 @@ Run(tcpsrv_t *pThis) case RS_RET_OK: /* valid data received, process it! */ localRet = tcps_sess.DataRcvd(pThis->pSessions[iTCPSess], buf, iRcvd); - if(localRet != RS_RET_OK) { + if(localRet != RS_RET_OK && localRet != RS_RET_QUEUE_FULL) { /* in this case, something went awfully wrong. * We are instructed to terminate the session. */ -- cgit v1.2.3 From 678a904620222b9113db8b5f89e988a064b05cd9 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 3 May 2011 09:41:35 +0200 Subject: bugfix: memory and file descriptor leak in stream processing Leaks could occur under some circumstances if the file stream handler errored out during the open call. Among others, this could cause very big memory leaks if there were a problem with unreadable disk queue files. In regard to the memory leak, this closes: http://bugzilla.adiscon.com/show_bug.cgi?id=256 --- ChangeLog | 6 ++++++ runtime/stream.c | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/ChangeLog b/ChangeLog index 16928921..2bd52a85 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,12 @@ Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? - bugfix: IPv6-address could not be specified in omrelp this was due to improper parsing of ":" closes: http://bugzilla.adiscon.com/show_bug.cgi?id=250 +- bugfix: memory and file descriptor leak in stream processing + Leaks could occur under some circumstances if the file stream handler + errored out during the open call. Among others, this could cause very + big memory leaks if there were a problem with unreadable disk queue + files. In regard to the memory leak, this + closes: http://bugzilla.adiscon.com/show_bug.cgi?id=256 - bugfix: imfile potentially duplicates lines This can happen when 0 bytes are read from the input file, and some writer appends data to the file BEFORE we check if a rollover happens. diff --git a/runtime/stream.c b/runtime/stream.c index af38bcb7..a12dda41 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -258,6 +258,7 @@ static rsRetVal strmOpenFile(strm_t *pThis) if(pThis->fd != -1) ABORT_FINALIZE(RS_RET_OK); + pThis->pszCurrFName = NULL; /* used to prevent mem leak in case of error */ if(pThis->pszFName == NULL) ABORT_FINALIZE(RS_RET_FILE_PREFIX_MISSING); @@ -289,6 +290,16 @@ static rsRetVal strmOpenFile(strm_t *pThis) (pThis->tOperationsMode == STREAMMODE_READ) ? "READ" : "WRITE", pThis->fd); finalize_it: + if(iRet != RS_RET_OK) { + if(pThis->pszCurrFName != NULL) { + free(pThis->pszCurrFName); + pThis->pszCurrFName = NULL; /* just to prevent mis-adressing down the road... */ + } + if(pThis->fd != -1) { + close(pThis->fd); + pThis->fd = -1; + } + } RETiRet; } -- cgit v1.2.3 From 816ab51f4b81bccc4b8408dfec311d9168269818 Mon Sep 17 00:00:00 2001 From: Tomas Heinrich Date: Mon, 9 May 2011 12:24:14 +0200 Subject: bugfix: stream driver mode was not correctly set on tcp ouput on big endian systems. Signed-off-by: Rainer Gerhards --- ChangeLog | 3 +++ tools/omfwd.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 2bd52a85..b06f480c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,8 @@ --------------------------------------------------------------------------- Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? +- bugfix: stream driver mode was not correctly set on tcp ouput on big + endian systems. + thanks varmojfekoj for the patch - bugfix: IPv6-address could not be specified in omrelp this was due to improper parsing of ":" closes: http://bugzilla.adiscon.com/show_bug.cgi?id=250 diff --git a/tools/omfwd.c b/tools/omfwd.c index 96b365a0..16d438f6 100644 --- a/tools/omfwd.c +++ b/tools/omfwd.c @@ -101,7 +101,7 @@ typedef struct _instanceData { /* config data */ static uchar *pszTplName = NULL; /* name of the default template to use */ static uchar *pszStrmDrvr = NULL; /* name of the stream driver to use */ -static short iStrmDrvrMode = 0; /* mode for stream driver, driver-dependent (0 mostly means plain tcp) */ +static int iStrmDrvrMode = 0; /* mode for stream driver, driver-dependent (0 mostly means plain tcp) */ static short bResendLastOnRecon = 0; /* should the last message be re-sent on a successful reconnect? */ static uchar *pszStrmDrvrAuthMode = NULL; /* authentication mode to use */ static int iUDPRebindInterval = 0; /* support for automatic re-binding (load balancers!). 0 - no rebind */ -- cgit v1.2.3 From 831ce25230f6c8cd7c362fda3616e1233a61cf00 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 9 May 2011 12:29:07 +0200 Subject: bugfix: invalid storage type for config variables --- ChangeLog | 1 + tools/omfwd.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index b06f480c..8e586002 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ --------------------------------------------------------------------------- Version 4.6.6 [v4-stable] (rgerhards), 2010-11-?? +- bugfix: invalid storage type for config variables - bugfix: stream driver mode was not correctly set on tcp ouput on big endian systems. thanks varmojfekoj for the patch diff --git a/tools/omfwd.c b/tools/omfwd.c index 16d438f6..aadeec6a 100644 --- a/tools/omfwd.c +++ b/tools/omfwd.c @@ -102,7 +102,7 @@ typedef struct _instanceData { static uchar *pszTplName = NULL; /* name of the default template to use */ static uchar *pszStrmDrvr = NULL; /* name of the stream driver to use */ static int iStrmDrvrMode = 0; /* mode for stream driver, driver-dependent (0 mostly means plain tcp) */ -static short bResendLastOnRecon = 0; /* should the last message be re-sent on a successful reconnect? */ +static int bResendLastOnRecon = 0; /* should the last message be re-sent on a successful reconnect? */ static uchar *pszStrmDrvrAuthMode = NULL; /* authentication mode to use */ static int iUDPRebindInterval = 0; /* support for automatic re-binding (load balancers!). 0 - no rebind */ static int iTCPRebindInterval = 0; /* support for automatic re-binding (load balancers!). 0 - no rebind */ -- cgit v1.2.3