From 8c3ab2e26f1bae46ff34fc1d0a10a69c4db78127 Mon Sep 17 00:00:00 2001 From: varmojfekoj Date: Thu, 4 Feb 2010 16:02:57 +0100 Subject: bugfix: several smaller bugs resolved after flexelint review Signed-off-by: Rainer Gerhards --- runtime/conf.c | 8 +++++--- runtime/modules.c | 2 +- runtime/vm.c | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) (limited to 'runtime') diff --git a/runtime/conf.c b/runtime/conf.c index a1d74b93..34752726 100644 --- a/runtime/conf.c +++ b/runtime/conf.c @@ -500,7 +500,7 @@ finalize_it: rsRetVal cflineParseTemplateName(uchar** pp, omodStringRequest_t *pOMSR, int iEntry, int iTplOpts, uchar *dfltTplName) { uchar *p; - uchar *tplName; + uchar *tplName = NULL; cstr_t *pStrB; DEFiRet; @@ -537,10 +537,12 @@ rsRetVal cflineParseTemplateName(uchar** pp, omodStringRequest_t *pOMSR, int iEn CHKiRet(cstrConvSzStrAndDestruct(pStrB, &tplName, 0)); } - iRet = OMSRsetEntry(pOMSR, iEntry, tplName, iTplOpts); - if(iRet != RS_RET_OK) goto finalize_it; + CHKiRet(OMSRsetEntry(pOMSR, iEntry, tplName, iTplOpts)); finalize_it: + if(iRet != RS_RET_OK) + free(tplName); + *pp = p; RETiRet; diff --git a/runtime/modules.c b/runtime/modules.c index 32ae659f..90802c96 100644 --- a/runtime/modules.c +++ b/runtime/modules.c @@ -383,7 +383,7 @@ doModInit(rsRetVal (*modInit)(int, int*, rsRetVal(**)(), rsRetVal(*)(), modInfo_ * can never change in the lifetime of an module. -- rgerhards, 2007-12-14 */ CHKiRet((*pNew->modQueryEtryPt)((uchar*)"getType", &modGetType)); - CHKiRet((iRet = (*modGetType)(&pNew->eType)) != RS_RET_OK); + CHKiRet((*modGetType)(&pNew->eType)); dbgprintf("module of type %d being loaded.\n", pNew->eType); /* OK, we know we can successfully work with the module. So we now fill the diff --git a/runtime/vm.c b/runtime/vm.c index 8cbf9e12..da2f122b 100644 --- a/runtime/vm.c +++ b/runtime/vm.c @@ -87,6 +87,9 @@ rsfrAddFunction(uchar *szName, prsf_t rsf) funcRegRoot = pEntry; finalize_it: + if(iRet != RS_RET_OK && iRet != RS_RET_DUP_FUNC_NAME) + free(pEntry); + RETiRet; } -- cgit From e1584b71f316b9ef2db58c8dbd2218f0b38962e7 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 8 Feb 2010 16:53:30 +0100 Subject: several bugfixes for the property replacer - bugfix: property replacer returned invalid parameters under some (unusual) conditions. In extreme cases, this could lead to garbled logs and/or a system failure. - bugfix: invalid length returned (often) when using regular expressions inside the property replacer - bugfix: submatch regex in property replacer did not honor "return 0 on no match" config case --- runtime/msg.c | 102 +++++++++++++++++++++++++++++++++------------------------- 1 file changed, 58 insertions(+), 44 deletions(-) (limited to 'runtime') diff --git a/runtime/msg.c b/runtime/msg.c index b45775b6..8e3ad314 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -2139,6 +2139,10 @@ static uchar *getNOW(eNOWType eNow) * be used in selector line processing. * rgerhards 2005-09-15 */ +/* a quick helper to save some writing: */ +#define RET_OUT_OF_MEMORY { *pbMustBeFreed = 0;\ + *pPropLen = sizeof("**OUT OF MEMORY**") - 1; \ + return(UCHAR_CONSTANT("**OUT OF MEMORY**"));} uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, propid_t propID, size_t *pPropLen, unsigned short *pbMustBeFreed) @@ -2200,8 +2204,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, case PROP_PRI_TEXT: pBuf = malloc(20 * sizeof(uchar)); if(pBuf == NULL) { - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else { *pbMustBeFreed = 1; pRes = (uchar*)textpri((char*)pBuf, 20, getPRIi(pMsg)); @@ -2245,49 +2248,49 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, break; case PROP_SYS_NOW: if((pRes = getNOW(NOW_NOW)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_YEAR: if((pRes = getNOW(NOW_YEAR)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_MONTH: if((pRes = getNOW(NOW_MONTH)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_DAY: if((pRes = getNOW(NOW_DAY)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_HOUR: if((pRes = getNOW(NOW_HOUR)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_HHOUR: if((pRes = getNOW(NOW_HHOUR)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_QHOUR: if((pRes = getNOW(NOW_QHOUR)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; case PROP_SYS_MINUTE: if((pRes = getNOW(NOW_MINUTE)) == NULL) { - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } else *pbMustBeFreed = 1; /* all of these functions allocate dyn. memory */ break; @@ -2299,6 +2302,8 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, * error message unreadable. rgerhards, 2007-07-10 */ dbgprintf("invalid property id: '%d'\n", propID); + *pbMustBeFreed = 0; + *pPropLen = sizeof("**INVALID PROPERTY NAME**") - 1; return UCHAR_CONSTANT("**INVALID PROPERTY NAME**"); } @@ -2357,8 +2362,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pBuf == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } /* now copy */ memcpy(pBuf, pFld, iLen); @@ -2375,6 +2379,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(*pbMustBeFreed == 1) free(pRes); *pbMustBeFreed = 0; + *pPropLen = sizeof("**FIELD NOT FOUND**") - 1; return UCHAR_CONSTANT("**FIELD NOT FOUND**"); } } else if(pTpe->data.field.iFromPos != 0 || pTpe->data.field.iToPos != 0) { @@ -2403,8 +2408,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pBuf == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } pSb = pRes; if(iFrom) { @@ -2434,9 +2438,15 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, } else { /* Check for regular expressions */ if (pTpe->data.field.has_regex != 0) { - if (pTpe->data.field.has_regex == 2) + if (pTpe->data.field.has_regex == 2) { /* Could not compile regex before! */ + if (*pbMustBeFreed == 1) { + free(pRes); + *pbMustBeFreed = 0; + } + *pPropLen = sizeof("**NO MATCH** **BAD REGULAR EXPRESSION**") - 1; return UCHAR_CONSTANT("**NO MATCH** **BAD REGULAR EXPRESSION**"); + } dbgprintf("string to match for regex is: %s\n", pRes); @@ -2476,12 +2486,16 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, free(pRes); *pbMustBeFreed = 0; } - if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_DFLTSTR) - return UCHAR_CONSTANT("**NO MATCH**"); - else if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_ZERO) - return UCHAR_CONSTANT("0"); - else - return UCHAR_CONSTANT(""); + if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_DFLTSTR) { + bufLen = sizeof("**NO MATCH**") - 1; + pRes = UCHAR_CONSTANT("**NO MATCH**"); + } else if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_ZERO) { + bufLen = 1; + pRes = UCHAR_CONSTANT("0"); + } else { + bufLen = 0; + pRes = UCHAR_CONSTANT(""); + } } } else { /* Match- but did it match the one we wanted? */ @@ -2492,10 +2506,16 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, free(pRes); *pbMustBeFreed = 0; } - if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_DFLTSTR) - return UCHAR_CONSTANT("**NO MATCH**"); - else - return UCHAR_CONSTANT(""); + if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_DFLTSTR) { + bufLen = sizeof("**NO MATCH**") - 1; + pRes = UCHAR_CONSTANT("**NO MATCH**"); + } else if(pTpe->data.field.nomatchAction == TPL_REGEX_NOMATCH_USE_ZERO) { + bufLen = 1; + pRes = UCHAR_CONSTANT("0"); + } else { + bufLen = 0; + pRes = UCHAR_CONSTANT(""); + } } } /* OK, we have a usable match - we now need to malloc pB */ @@ -2509,13 +2529,12 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if (pB == NULL) { if (*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } /* Lets copy the matched substring to the buffer */ memcpy(pB, pRes + iOffs + pmatch[pTpe->data.field.iSubMatchToUse].rm_so, iLenBuf); - bufLen = iLenBuf - 1; + bufLen = iLenBuf; pB[iLenBuf] = '\0';/* terminate string, did not happen before */ if (*pbMustBeFreed == 1) @@ -2533,6 +2552,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, free(pRes); *pbMustBeFreed = 0; } + *pPropLen = sizeof("***REGEXP NOT AVAILABLE***") - 1; return UCHAR_CONSTANT("***REGEXP NOT AVAILABLE***"); } } @@ -2565,8 +2585,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pB == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } pSrc = pRes; while(*pSrc) { @@ -2612,8 +2631,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pDst == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } for(pSrc = pRes; *pSrc; pSrc++) { if(!iscntrl((int) *pSrc)) @@ -2648,8 +2666,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pDst == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } for(pSrc = pRes; *pSrc; pSrc++) { if(iscntrl((int) *pSrc)) @@ -2688,8 +2705,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pB == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } while(*pRes) { if(iscntrl((int) *pRes)) { @@ -2734,8 +2750,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pDst == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } for(pSrc = pRes; *pSrc; pSrc++) { if(*pSrc != '/') @@ -2770,8 +2785,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pDst == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } for(pSrc = pRes; *pSrc; pSrc++) { if(*pSrc == '/') @@ -2825,8 +2839,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, /* ok, original copy, need a private one */ pB = malloc((iLn + 1) * sizeof(uchar)); if(pB == NULL) { - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } memcpy(pB, pRes, iLn - 1); pRes = pB; @@ -2845,6 +2858,7 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pTpe->data.field.options.bCSV) { /* we need to obtain a private copy, as we need to at least add the double quotes */ int iBufLen; + int i; uchar *pBStart; uchar *pDst; uchar *pSrc; @@ -2856,10 +2870,10 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, if(pDst == NULL) { if(*pbMustBeFreed == 1) free(pRes); - *pbMustBeFreed = 0; - return UCHAR_CONSTANT("**OUT OF MEMORY**"); + RET_OUT_OF_MEMORY; } pSrc = pRes; + i = 0; *pDst++ = '"'; /* starting quote */ while(*pSrc) { if(*pSrc == '"') -- cgit From c577e9c64cec0eebf6b7c3bd964354ab90c045ae Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 22 Feb 2010 09:31:10 +0100 Subject: bugfix: message without MSG part could case a segfault [backported from v5 commit 98d1ed504ec001728955a5bcd7916f64cd85f39f] This actually was a "recent" regression, but I did not realize that it was introduced by the performance optimization in v4-devel. Shame on me for having two devel versions at the same time... --- runtime/msg.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'runtime') diff --git a/runtime/msg.c b/runtime/msg.c index 8e3ad314..70207075 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1171,7 +1171,7 @@ uchar *getMSG(msg_t *pM) if(pM == NULL) ret = UCHAR_CONSTANT(""); else { - if(pM->offMSG == -1) + if(pM->iLenMSG == 0) ret = UCHAR_CONSTANT(""); else ret = pM->pszRawMsg + pM->offMSG; @@ -1947,12 +1947,20 @@ void MsgSetHOSTNAME(msg_t *pThis, uchar* pszHOSTNAME, int lenHOSTNAME) /* set the offset of the MSG part into the raw msg buffer + * Note that the offset may be higher than the length of the raw message + * (exactly by one). This can happen if we have a message that does not + * contain any MSG part. */ void MsgSetMSGoffs(msg_t *pMsg, short offs) { ISOBJ_TYPE_assert(pMsg, msg); - pMsg->iLenMSG = pMsg->iLenRawMsg - offs; pMsg->offMSG = offs; + if(offs > pMsg->iLenRawMsg) { + assert(offs - 1 == pMsg->iLenRawMsg); + pMsg->iLenMSG = 0; + } else { + pMsg->iLenMSG = pMsg->iLenRawMsg - offs; + } } @@ -1986,7 +1994,8 @@ rsRetVal MsgReplaceMSG(msg_t *pThis, uchar* pszMSG, int lenMSG) pThis->pszRawMsg = bufNew; } - memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); + if(lenMSG > 0) + memcpy(pThis->pszRawMsg + pThis->offMSG, pszMSG, lenMSG); pThis->pszRawMsg[lenNew] = '\0'; /* this also works with truncation! */ pThis->iLenRawMsg = lenNew; pThis->iLenMSG = lenMSG; -- cgit From af5fb078d48b364b20adf7e56e9869664e7424f9 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 22 Feb 2010 14:25:56 +0100 Subject: message parser fixes and testbench enhancements - improved testbench to contain samples for totally malformed messages which miss parts of the message content - bugfix: some malformed messages could lead to a missing LF inside files or some other missing parts of the template content. - bugfix: if a message ended immediately with a hostname, the hostname was mistakenly interpreted as TAG, and localhost be used as hostname --- runtime/datetime.c | 17 ++++++++++++----- runtime/rsyslog.h | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) (limited to 'runtime') diff --git a/runtime/datetime.c b/runtime/datetime.c index 6160bd7c..7b0d8d11 100644 --- a/runtime/datetime.c +++ b/runtime/datetime.c @@ -291,11 +291,11 @@ ParseTIMESTAMP3339(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) } /* OK, we actually have a 3339 timestamp, so let's indicated this */ - if(lenStr > 0 && *pszTS == ' ') { + if(lenStr > 0) { + if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */ + ABORT_FINALIZE(RS_RET_INVLD_TIME); + ++pszTS; /* just skip past it */ --lenStr; - ++pszTS; - } else { - ABORT_FINALIZE(RS_RET_INVLD_TIME); } /* we had success, so update parse pointer and caller-provided timestamp */ @@ -510,6 +510,7 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) if(lenStr == 0 || *pszTS++ != ' ') ABORT_FINALIZE(RS_RET_INVLD_TIME); + --lenStr; /* we accept a slightly malformed timestamp when receiving. This is * we accept one-digit days @@ -565,7 +566,13 @@ ParseTIMESTAMP3164(struct syslogTime *pTime, uchar** ppszTS, int *pLenStr) * invalid format, it occurs frequently enough (e.g. with Cisco devices) * to permit it as a valid case. -- rgerhards, 2008-09-12 */ - if(lenStr == 0 || *pszTS++ == ':') { + if(lenStr > 0 && *pszTS == ':') { + ++pszTS; /* just skip past it */ + --lenStr; + } + if(lenStr > 0) { + if(*pszTS != ' ') /* if it is not a space, it can not be a "good" time - 2010-02-22 rgerhards */ + ABORT_FINALIZE(RS_RET_INVLD_TIME); ++pszTS; /* just skip past it */ --lenStr; } diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 27bea6bc..0f489a7f 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -35,7 +35,7 @@ * value to the fixed size of the message object. */ #define CONF_TAG_MAXSIZE 512 /* a value that is deemed far too large for any valid TAG */ -#define CONF_TAG_HOSTNAME 512 /* a value that is deemed far too large for any valid HOSTNAME */ +#define CONF_HOSTNAME_MAXSIZE 512 /* a value that is deemed far too large for any valid HOSTNAME */ #define CONF_RAWMSG_BUFSIZE 101 #define CONF_TAG_BUFSIZE 32 #define CONF_HOSTNAME_BUFSIZE 32 -- cgit From 2c39f76037328459c5cff14762e0839b1e77d570 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 1 Mar 2010 07:33:09 +0100 Subject: make $ActonFileDefaultTemplate available to ompipe This was not honored by the new ompipe module, because it is a local file directive (it was applied to pipes as a side-effect of using the same module for pipes and files...). I now made this a global, so that semantics are the same as previously. Not really nice, but probably the best thing to do in the current situation (everything else would involve much more overhead --- leave that for the new config system). --- runtime/srutils.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'runtime') diff --git a/runtime/srutils.c b/runtime/srutils.c index c403b312..4bd552da 100644 --- a/runtime/srutils.c +++ b/runtime/srutils.c @@ -500,12 +500,14 @@ int decodeSyslogName(uchar *name, syslogName_t *codetab) for (p = buf; *p; p++) if (isupper((int) *p)) *p = tolower((int) *p); +dbgprintf("obtained syslogName '%s'\n", buf); for (c = codetab; c->c_name; c++) if (!strcmp((char*) buf, (char*) c->c_name)) { dbgprintf(" ==> %d\n", c->c_val); return (c->c_val); } +dbgprintf("syslogName '%s' NOT found!\n", buf); return (-1); } -- cgit From f9ec561e6ee96e2c864bb69773ad6fc7303aef58 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 1 Mar 2010 08:41:00 +0100 Subject: cleanup: removed debug messages that accidently made it into the commit --- runtime/rule.c | 1 + runtime/srutils.c | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/rule.c b/runtime/rule.c index 182d616a..4c2c9edb 100644 --- a/runtime/rule.c +++ b/runtime/rule.c @@ -164,6 +164,7 @@ shouldProcessThisMessage(rule_t *pRule, msg_t *pMsg, int *bProcessMsg) if(pRule->f_filter_type == FILTER_PRI) { /* skip messages that are incorrect priority */ +dbgprintf("testing filter, f_pmask %d\n", pRule->f_filterData.f_pmask[pMsg->iFacility]); if ( (pRule->f_filterData.f_pmask[pMsg->iFacility] == TABLE_NOPRI) || \ ((pRule->f_filterData.f_pmask[pMsg->iFacility] & (1<iSeverity)) == 0) ) bRet = 0; diff --git a/runtime/srutils.c b/runtime/srutils.c index 4bd552da..c403b312 100644 --- a/runtime/srutils.c +++ b/runtime/srutils.c @@ -500,14 +500,12 @@ int decodeSyslogName(uchar *name, syslogName_t *codetab) for (p = buf; *p; p++) if (isupper((int) *p)) *p = tolower((int) *p); -dbgprintf("obtained syslogName '%s'\n", buf); for (c = codetab; c->c_name; c++) if (!strcmp((char*) buf, (char*) c->c_name)) { dbgprintf(" ==> %d\n", c->c_val); return (c->c_val); } -dbgprintf("syslogName '%s' NOT found!\n", buf); return (-1); } -- cgit From 01e6df368cdec89920d075fe096971481cb8ce49 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 1 Mar 2010 12:44:57 +0100 Subject: bugfix: comment char ('#') in literal terminated script parsing ...and thus could not be used. but tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=119 --- runtime/ctok.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) (limited to 'runtime') diff --git a/runtime/ctok.c b/runtime/ctok.c index ceab15bd..71a10a20 100644 --- a/runtime/ctok.c +++ b/runtime/ctok.c @@ -87,11 +87,12 @@ ctokUngetCharFromStream(ctok_t *pThis, uchar __attribute__((unused)) c) } -/* get the next character from the input "stream" (currently just a in-memory - * string...) -- rgerhards, 2008-02-19 +/* get the next character from the input "stream". Note that this version + * does NOT look for comment characters as end-of-stream, so it is suitable + * when building constant strings! -- rgerhards, 2010-03-01 */ -static rsRetVal -ctokGetCharFromStream(ctok_t *pThis, uchar *pc) +static inline rsRetVal +ctokGetCharFromStreamNoComment(ctok_t *pThis, uchar *pc) { DEFiRet; @@ -99,7 +100,7 @@ ctokGetCharFromStream(ctok_t *pThis, uchar *pc) ASSERT(pc != NULL); /* end of string or begin of comment terminates the "stream" */ - if(*pThis->pp == '\0' || *pThis->pp == '#') { + if(*pThis->pp == '\0') { ABORT_FINALIZE(RS_RET_EOS); } else { *pc = *pThis->pp; @@ -111,6 +112,28 @@ finalize_it: } +/* get the next character from the input "stream" (currently just a in-memory + * string...) -- rgerhards, 2008-02-19 + */ +static rsRetVal +ctokGetCharFromStream(ctok_t *pThis, uchar *pc) +{ + DEFiRet; + + ISOBJ_TYPE_assert(pThis, ctok); + ASSERT(pc != NULL); + + CHKiRet(ctokGetCharFromStreamNoComment(pThis, pc)); + /* begin of comment terminates the "stream"! */ + if(*pc == '#') { + ABORT_FINALIZE(RS_RET_EOS); + } + +finalize_it: + RETiRet; +} + + /* skip whitespace in the input "stream". * rgerhards, 2008-02-19 */ @@ -297,7 +320,7 @@ ctokGetSimpStr(ctok_t *pThis, ctok_token_t *pToken) pToken->tok = ctok_SIMPSTR; CHKiRet(rsCStrConstruct(&pstrVal)); - CHKiRet(ctokGetCharFromStream(pThis, &c)); + CHKiRet(ctokGetCharFromStreamNoComment(pThis, &c)); /* while we are in escape mode (had a backslash), no sequence * terminates the loop. If outside, it is terminated by a single quote. */ @@ -312,7 +335,7 @@ ctokGetSimpStr(ctok_t *pThis, ctok_token_t *pToken) CHKiRet(rsCStrAppendChar(pstrVal, c)); } } - CHKiRet(ctokGetCharFromStream(pThis, &c)); + CHKiRet(ctokGetCharFromStreamNoComment(pThis, &c)); } CHKiRet(rsCStrFinish(pStrB)); -- cgit From cd8c6abcc8cea54924e55dffe820364ad98a40df Mon Sep 17 00:00:00 2001 From: Yann Droneaud Date: Thu, 4 Mar 2010 08:00:39 +0100 Subject: Includes "config.h" before any other header. For consistency, ./configure generated "config.h" must be the first header include through out the project. Signed-off-by: Rainer Gerhards --- runtime/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/debug.c b/runtime/debug.c index 9547eee6..20474a9a 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -1,4 +1,3 @@ -#include /* debug.c * * This file proides debug and run time error analysis support. Some of the @@ -550,6 +549,7 @@ if(pLog == NULL) { return; /* if we don't know it yet, we can not clean up... */ } #endif +#include /* we found the last lock entry. We now need to see from which FuncDB we need to * remove it. This is recorded inside the mutex log entry. -- cgit From 71ffb32ab8a1847f746a298ad20f7dd8b40570a0 Mon Sep 17 00:00:00 2001 From: Yann Droneaud Date: Wed, 3 Mar 2010 18:59:22 +0100 Subject: Fix Large File Support (LFS) support (bug #182) - _FILE_OFFSET_BITS must be defined before including any other system headers otherwise it does nothing. - Don't define it in rsyslog.h, let it be defined in config.h, and let ./configure script enable LFS since Autoconf provides a portable macro to enable LFS support : AC_SYS_LARGEFILE --- runtime/rsyslog.h | 9 --------- 1 file changed, 9 deletions(-) (limited to 'runtime') diff --git a/runtime/rsyslog.h b/runtime/rsyslog.h index 0f489a7f..8979893a 100644 --- a/runtime/rsyslog.h +++ b/runtime/rsyslog.h @@ -46,15 +46,6 @@ * # End Config Settings # * * ############################################################# */ -#ifndef NOLARGEFILE -# undef _LARGEFILE_SOURCE -# undef _LARGEFILE64_SOURCE -# undef _FILE_OFFSET_BITS -# define _LARGEFILE_SOURCE -# define _LARGEFILE64_SOURCE -# define _FILE_OFFSET_BITS 64 -#endif - /* portability: not all platforms have these defines, so we * define them here if they are missing. -- rgerhards, 2008-03-04 */ -- cgit From 396e211e5cfd195b7135947d425b089a0447fa88 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 5 Mar 2010 08:27:26 +0100 Subject: enabled compilation by using "racy" replacements for atomic instructions ... but this is not considered a real solution. For some of the uses, it may acutally be sufficient, but the implications need to be analyzed in detail. --- runtime/atomic.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'runtime') diff --git a/runtime/atomic.h b/runtime/atomic.h index d5aaf56b..62ceb8b3 100644 --- a/runtime/atomic.h +++ b/runtime/atomic.h @@ -66,6 +66,9 @@ # define ATOMIC_DEC_AND_FETCH(data) (--(data)) # define ATOMIC_FETCH_32BIT(data) (data) # define ATOMIC_STORE_1_TO_32BIT(data) (data) = 1 +# define ATOMIC_STORE_1_TO_INT(data) (data) = 1 +# define ATOMIC_STORE_0_TO_INT(data) (data) = 0 +# define ATOMIC_CAS_VAL(data, oldVal, newVal) (data) = (newVal) #endif #endif /* #ifndef INCLUDED_ATOMIC_H */ -- cgit From d97ad63e218112d7cd3a390854b2918407804976 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 8 Mar 2010 17:56:11 +0100 Subject: added new property replacer option "date-rfc3164-buggyday" primarily to ease migration from syslog-ng. See property replacer doc for details. [backport from 5.5.3 because urgently needed by some] --- runtime/datetime.c | 8 ++++++-- runtime/datetime.h | 6 ++++-- runtime/msg.c | 10 +++++++--- 3 files changed, 17 insertions(+), 7 deletions(-) (limited to 'runtime') diff --git a/runtime/datetime.c b/runtime/datetime.c index 7b0d8d11..eff72f91 100644 --- a/runtime/datetime.c +++ b/runtime/datetime.c @@ -793,8 +793,12 @@ int formatTimestamp3339(struct syslogTime *ts, char* pBuf) * buffer that will receive the resulting string. The function * returns the size of the timestamp written in bytes (without * the string termnator). If 0 is returend, an error occured. + * rgerhards, 2010-03-05: Added support to for buggy 3164 dates, + * where a zero-digit is written instead of a space for the first + * day character if day < 10. syslog-ng seems to do that, and some + * parsing scripts (in migration cases) rely on that. */ -int formatTimestamp3164(struct syslogTime *ts, char* pBuf) +int formatTimestamp3164(struct syslogTime *ts, char* pBuf, int bBuggyDay) { static char* monthNames[12] = { "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec" }; @@ -807,7 +811,7 @@ int formatTimestamp3164(struct syslogTime *ts, char* pBuf) pBuf[2] = monthNames[(ts->month - 1) % 12][2]; pBuf[3] = ' '; iDay = (ts->day / 10) % 10; /* we need to write a space if the first digit is 0 */ - pBuf[4] = iDay ? iDay + '0' : ' '; + pBuf[4] = (bBuggyDay || iDay > 0) ? iDay + '0' : ' '; pBuf[5] = ts->day % 10 + '0'; pBuf[6] = ' '; pBuf[7] = (ts->hour / 10) % 10 + '0'; diff --git a/runtime/datetime.h b/runtime/datetime.h index 8140eb71..9dcce3c5 100644 --- a/runtime/datetime.h +++ b/runtime/datetime.h @@ -39,15 +39,17 @@ BEGINinterface(datetime) /* name must also be changed in ENDinterface macro! */ int (*formatTimestampToMySQL)(struct syslogTime *ts, char* pDst); int (*formatTimestampToPgSQL)(struct syslogTime *ts, char *pDst); int (*formatTimestamp3339)(struct syslogTime *ts, char* pBuf); - int (*formatTimestamp3164)(struct syslogTime *ts, char* pBuf); + int (*formatTimestamp3164)(struct syslogTime *ts, char* pBuf, int); int (*formatTimestampSecFrac)(struct syslogTime *ts, char* pBuf); ENDinterface(datetime) -#define datetimeCURR_IF_VERSION 2 /* increment whenever you change the interface structure! */ +#define datetimeCURR_IF_VERSION 4 /* increment whenever you change the interface structure! */ /* interface changes: * 1 - initial version * 2 - not compatible to 1 - bugfix required ParseTIMESTAMP3164 to accept char ** as * last parameter. Did not try to remain compatible as this is not something any * third-party module should call. -- rgerhards, 2008.-09-12 + * 3 - taken by v5 branch! + * 4 - formatTimestamp3164 takes a third int parameter */ /* prototypes */ diff --git a/runtime/msg.c b/runtime/msg.c index 70207075..3a2331f4 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -1213,10 +1213,12 @@ static inline char *getTimeReported(msg_t *pM, enum tplFormatTypes eFmt) switch(eFmt) { case tplFmtDefault: case tplFmtRFC3164Date: + case tplFmtRFC3164BuggyDate: MsgLock(pM); if(pM->pszTIMESTAMP3164 == NULL) { pM->pszTIMESTAMP3164 = pM->pszTimestamp3164; - datetime.formatTimestamp3164(&pM->tTIMESTAMP, pM->pszTIMESTAMP3164); + datetime.formatTimestamp3164(&pM->tTIMESTAMP, pM->pszTIMESTAMP3164, + (eFmt == tplFmtRFC3164BuggyDate)); } MsgUnlock(pM); return(pM->pszTIMESTAMP3164); @@ -1279,7 +1281,7 @@ static inline char *getTimeGenerated(msg_t *pM, enum tplFormatTypes eFmt) MsgUnlock(pM); return ""; } - datetime.formatTimestamp3164(&pM->tRcvdAt, pM->pszRcvdAt3164); + datetime.formatTimestamp3164(&pM->tRcvdAt, pM->pszRcvdAt3164, 0); } MsgUnlock(pM); return(pM->pszRcvdAt3164); @@ -1306,13 +1308,15 @@ static inline char *getTimeGenerated(msg_t *pM, enum tplFormatTypes eFmt) MsgUnlock(pM); return(pM->pszRcvdAt_PgSQL); case tplFmtRFC3164Date: + case tplFmtRFC3164BuggyDate: MsgLock(pM); if(pM->pszRcvdAt3164 == NULL) { if((pM->pszRcvdAt3164 = malloc(16)) == NULL) { MsgUnlock(pM); return ""; } - datetime.formatTimestamp3164(&pM->tRcvdAt, pM->pszRcvdAt3164); + datetime.formatTimestamp3164(&pM->tRcvdAt, pM->pszRcvdAt3164, + (eFmt == tplFmtRFC3164BuggyDate)); } MsgUnlock(pM); return(pM->pszRcvdAt3164); -- cgit From 3d80d6ba301e4d26b646c84d621ebe880ebc513f Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 9 Mar 2010 18:07:31 +0100 Subject: bugfix: potential problem (loop, abort) when file write error occured When a write error occured in stream.c, variable iWritten had the error code but this was handled as if it were the actual number of bytes written. That was used in pointer arithmetic later on, and thus could lead to all sorts of problems. However, this could only happen if the error was EINTR or the file in question was a tty. All other cases were handled properly. Now, iWritten is reset to zero in such cases, resulting in proper retries. --- runtime/stream.c | 1 + 1 file changed, 1 insertion(+) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 36f44003..82718099 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -790,6 +790,7 @@ doWriteCall(strm_t *pThis, uchar *pBuf, size_t *pLenBuf) if(iWritten < 0) { char errStr[1024]; int err = errno; + iWritten = 0; /* we have written NO bytes! */ rs_strerror_r(err, errStr, sizeof(errStr)); DBGPRINTF("log file (%d) write error %d: %s\n", pThis->fd, err, errStr); if(err == EINTR) { -- cgit From 5996414f04118dec4f1dcc12c88ee8c68f6e89ad Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 10 Mar 2010 07:36:38 +0100 Subject: bugfixes and testbench improvement - improved testbench - bugfix: potential data loss during file stream shutdown - bugfix: potential problems during file stream shutdown The shutdown/close sequence was not clean, what potentially (but unlikely) could lead to some issues. We have not been able to describe any fatal cases, but there was some bug potential. Sequence has now been straighted out. --- runtime/stream.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 82718099..5396bae0 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -306,20 +306,22 @@ strmWaitAsyncWriterDone(strm_t *pThis) /* close a strm file * Note that the bDeleteOnClose flag is honored. If it is set, the file will be * deleted after close. This is in support for the qRead thread. + * Note: it is valid to call this function when the physical file is closed. If so, + * strmCloseFile() will still check if there is any unwritten data inside buffers + * (this may be the case) and, if so, will open the file, write the data, and then + * close it again (this is done via strmFlush and friends). */ static rsRetVal strmCloseFile(strm_t *pThis) { DEFiRet; ASSERT(pThis != NULL); - ASSERT(pThis->fd != -1); dbgoprint((obj_t*) pThis, "file %d closing\n", pThis->fd); if(!pThis->bInClose && pThis->tOperationsMode != STREAMMODE_READ) { pThis->bInClose = 1; + strmFlush(pThis); if(pThis->bAsyncWrite) { - strmFlush(pThis); - } else { strmWaitAsyncWriterDone(pThis); } pThis->bInClose = 0; @@ -685,8 +687,10 @@ CODESTARTobjDestruct(strm) /* Note: mutex will be unlocked in stopWriter! */ d_pthread_mutex_lock(&pThis->mut); - if(pThis->tOperationsMode != STREAMMODE_READ) - strmFlush(pThis); + /* strmClose() will handle read-only files as well as need to open + * files that have unwritten buffers. -- rgerhards, 2010-03-09 + */ + strmCloseFile(pThis); if(pThis->bAsyncWrite) { stopWriter(pThis); @@ -705,9 +709,6 @@ CODESTARTobjDestruct(strm) * IMPORTANT: we MUST free this only AFTER the ansyncWriter has been stopped, else * we get random errors... */ - if(pThis->fd != -1) - strmCloseFile(pThis); - free(pThis->pszDir); free(pThis->pZipBuf); free(pThis->pszCurrFName); -- cgit From 8833612e7516694e89148eb3dc1c88f5ea954d16 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 10 Mar 2010 08:21:41 +0100 Subject: some cosmetic changes note that a buffer size calculation was done wrong, but this was cosmetic because our buffers currently all use byte size, so even though the formula was wrong, the result was correct. --- runtime/stream.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 5396bae0..bb92eeb5 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -619,11 +619,11 @@ static rsRetVal strmConstructFinalize(strm_t *pThis) * to make sure we can write out everything with a SINGLE api call! * We add another 128 bytes to take care of the gzip header and "all eventualities". */ - CHKmalloc(pThis->pZipBuf = (Bytef*) malloc(sizeof(uchar) * pThis->sIOBufSize + 128)); + CHKmalloc(pThis->pZipBuf = (Bytef*) malloc(sizeof(uchar) * (pThis->sIOBufSize + 128))); } } - /* if we are aset to sync, we must obtain a file handle to the directory for fsync() purposes */ + /* if we are set to sync, we must obtain a file handle to the directory for fsync() purposes */ if(pThis->bSync && !pThis->bIsTTY) { pThis->fdDir = open((char*)pThis->pszDir, O_RDONLY | O_CLOEXEC | O_NOCTTY); if(pThis->fdDir == -1) { @@ -913,7 +913,7 @@ asyncWriterThread(void *pPtr) if(prctl(PR_SET_NAME, "rs:asyn strmwr", 0, 0, 0) != 0) { DBGPRINTF("prctl failed, not setting thread name for '%s'\n", "stream writer"); } -#endif +# endif while(1) { /* loop broken inside */ d_pthread_mutex_lock(&pThis->mut); @@ -1060,10 +1060,6 @@ finalize_it: * add a config switch so that the user can decide the risk he is ready * to take, but so far this is not yet implemented (not even requested ;)). * rgerhards, 2009-06-04 - * For the time being, we take a very conservative approach and do not run this - * method multithreaded. This is done in an effort to solve a segfault condition - * that seems to be related to the zip code. -- rgerhards, 2009-09-22 - * TODO: make multithreaded again! */ static rsRetVal doZipWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf) @@ -1235,6 +1231,11 @@ finalize_it: * caller-provided buffer is larger than our one. So instead of optimizing a case * which normally does not exist, we expect some degradation in its case but make us * perform better in the regular cases. -- rgerhards, 2009-07-07 + * Note: the pThis->iBufPtr == pThis->sIOBufSize logic below looks a bit like an + * on-off error. In fact, it is not, because iBufPtr always points to the next + * *free* byte in the buffer. So if it is sIOBufSize - 1, there actually is one + * free byte left. This came up during a code walkthrough and was considered + * worth nothing. -- rgerhards, 2010-03-10 */ static rsRetVal strmWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf) @@ -1359,8 +1360,7 @@ strmSetDir(strm_t *pThis, uchar *pszDir, size_t iLenDir) if(iLenDir < 1) ABORT_FINALIZE(RS_RET_FILE_PREFIX_MISSING); - if((pThis->pszDir = malloc(sizeof(uchar) * iLenDir + 1)) == NULL) - ABORT_FINALIZE(RS_RET_OUT_OF_MEMORY); + CHKmalloc(pThis->pszDir = malloc(sizeof(uchar) * iLenDir + 1)); memcpy(pThis->pszDir, pszDir, iLenDir + 1); /* always think about the \0! */ pThis->lenDir = iLenDir; -- cgit From 6c43f93022caa3feaa7b4fa3d88ca31746fd94cf Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 10 Mar 2010 09:52:49 +0100 Subject: fixed regression introduced with previous commit disk queue mode did no longer work correctly. A side-effect of this commit here is slightly cleaned-up (and more elegant) code for circular files. --- runtime/stream.c | 15 +++++++++++---- runtime/stream.h | 1 - 2 files changed, 11 insertions(+), 5 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 5396bae0..c6c8273e 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -318,13 +318,11 @@ static rsRetVal strmCloseFile(strm_t *pThis) ASSERT(pThis != NULL); dbgoprint((obj_t*) pThis, "file %d closing\n", pThis->fd); - if(!pThis->bInClose && pThis->tOperationsMode != STREAMMODE_READ) { - pThis->bInClose = 1; + if(pThis->tOperationsMode != STREAMMODE_READ) { strmFlush(pThis); if(pThis->bAsyncWrite) { strmWaitAsyncWriterDone(pThis); } - pThis->bInClose = 0; } close(pThis->fd); @@ -882,13 +880,22 @@ strmSchedWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf) ASSERT(pThis != NULL); + /* we need to reset the buffer pointer BEFORE calling the actual write + * function. Otherwise, in circular mode, the write function will + * potentially close the file, then close will flush and as the + * buffer pointer is nonzero, will re-call into this code here. In + * the end result, we than have a problem (and things are screwed + * up). So we reset the buffer pointer first, and all this can + * not happen. It is safe to do so, because that pointer is NOT + * used inside the write functions. -- rgerhads, 2010-03-10 + */ + pThis->iBufPtr = 0; /* we are at the begin of a new buffer */ if(pThis->bAsyncWrite) { CHKiRet(doAsyncWriteInternal(pThis, lenBuf)); } else { CHKiRet(doWriteInternal(pThis, pBuf, lenBuf)); } - pThis->iBufPtr = 0; /* we are at the begin of a new buffer */ finalize_it: RETiRet; diff --git a/runtime/stream.h b/runtime/stream.h index 89175b0f..369d5a0f 100644 --- a/runtime/stream.h +++ b/runtime/stream.h @@ -119,7 +119,6 @@ typedef struct strm_s { size_t iBufPtr; /* pointer into current buffer */ int iUngetC; /* char set via UngetChar() call or -1 if none set */ bool bInRecord; /* if 1, indicates that we are currently writing a not-yet complete record */ - bool bInClose; /* used to break "deadly close loops", tells us we are already inside a close */ int iZipLevel; /* zip level (0..9). If 0, zip is completely disabled */ Bytef *pZipBuf; /* support for async flush procesing */ -- cgit From 83c15bb0a004ee348228217861c0eab7c5573952 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 11 Mar 2010 12:36:21 +0100 Subject: added more tests to testbench and improved testing tools --- runtime/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/debug.c b/runtime/debug.c index 20474a9a..4504aaad 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -952,7 +952,7 @@ void dbgprintf(char *fmt, ...) { va_list ap; - char pszWriteBuf[1024]; + char pszWriteBuf[20480]; size_t lenWriteBuf; if(!(Debug && debugging_on)) -- cgit From a1127abbae67ac3a9c154b1914b15f1e16deca56 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 15 Mar 2010 09:29:54 +0100 Subject: bugfix(minor): handling of extremely large strings in dbgprintf() fixed Previously, it could lead to garbagge output and, in extreme cases, also to segfaults. Note: this was a problem only when debug output was actually enabled, so it caused no problem in production use. --- runtime/debug.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'runtime') diff --git a/runtime/debug.c b/runtime/debug.c index 4504aaad..bc581a5d 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -961,6 +961,15 @@ dbgprintf(char *fmt, ...) va_start(ap, fmt); lenWriteBuf = vsnprintf(pszWriteBuf, sizeof(pszWriteBuf), fmt, ap); va_end(ap); + if(lenWriteBuf >= sizeof(pszWriteBuf)) { + /* prevent buffer overrruns and garbagge display */ + pszWriteBuf[sizeof(pszWriteBuf) - 5] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 4] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 3] = '.'; + pszWriteBuf[sizeof(pszWriteBuf) - 2] = '\n'; + pszWriteBuf[sizeof(pszWriteBuf) - 1] = '\0'; + lenWriteBuf = sizeof(pszWriteBuf); + } dbgprint(NULL, pszWriteBuf, lenWriteBuf); } -- cgit From 16cb5ae53caba2a3cc9091026037c55d23cf3199 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 16 Mar 2010 17:06:21 +0100 Subject: enhanced dbgoprint() buffer size --- runtime/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/debug.c b/runtime/debug.c index bc581a5d..9b7c2952 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -922,7 +922,7 @@ void dbgoprint(obj_t *pObj, char *fmt, ...) { va_list ap; - char pszWriteBuf[1024]; + char pszWriteBuf[32*1024]; size_t lenWriteBuf; if(!(Debug && debugging_on)) -- cgit From a577b09e58789c39b2f50e0bd89125fbad76672a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 16 Mar 2010 19:01:22 +0100 Subject: reduced runtime requirements of inactive debug code a slight bit --- runtime/stream.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index d76f12e7..3373b795 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -220,7 +220,7 @@ doPhysOpen(strm_t *pThis) char errStr[1024]; int err = errno; rs_strerror_r(err, errStr, sizeof(errStr)); - dbgoprint((obj_t*) pThis, "open error %d, file '%s': %s\n", errno, pThis->pszCurrFName, errStr); + DBGOPRINT((obj_t*) pThis, "open error %d, file '%s': %s\n", errno, pThis->pszCurrFName, errStr); if(err == ENOENT) ABORT_FINALIZE(RS_RET_FILE_NOT_FOUND); else @@ -278,7 +278,7 @@ static rsRetVal strmOpenFile(strm_t *pThis) pThis->iCurrOffs = offset; } - dbgoprint((obj_t*) pThis, "opened file '%s' for %s as %d\n", pThis->pszCurrFName, + DBGOPRINT((obj_t*) pThis, "opened file '%s' for %s as %d\n", pThis->pszCurrFName, (pThis->tOperationsMode == STREAMMODE_READ) ? "READ" : "WRITE", pThis->fd); finalize_it: @@ -316,7 +316,7 @@ static rsRetVal strmCloseFile(strm_t *pThis) DEFiRet; ASSERT(pThis != NULL); - dbgoprint((obj_t*) pThis, "file %d closing\n", pThis->fd); + DBGOPRINT((obj_t*) pThis, "file %d closing\n", pThis->fd); if(pThis->tOperationsMode != STREAMMODE_READ) { strmFlush(pThis); @@ -441,7 +441,7 @@ strmHandleEOF(strm_t *pThis) case STREAMTYPE_FILE_CIRCULAR: /* we have multiple files and need to switch to the next one */ /* TODO: think about emulating EOF in this case (not yet needed) */ - dbgoprint((obj_t*) pThis, "file %d EOF\n", pThis->fd); + DBGOPRINT((obj_t*) pThis, "file %d EOF\n", pThis->fd); CHKiRet(strmNextFile(pThis)); break; case STREAMTYPE_FILE_MONITOR: @@ -473,7 +473,7 @@ strmReadBuf(strm_t *pThis) */ CHKiRet(strmOpenFile(pThis)); iLenRead = read(pThis->fd, pThis->pIOBuf, pThis->sIOBufSize); - dbgoprint((obj_t*) pThis, "file %d read %ld bytes\n", pThis->fd, iLenRead); + DBGOPRINT((obj_t*) pThis, "file %d read %ld bytes\n", pThis->fd, iLenRead); if(iLenRead == 0) { CHKiRet(strmHandleEOF(pThis)); } else if(iLenRead < 0) @@ -505,7 +505,7 @@ static rsRetVal strmReadChar(strm_t *pThis, uchar *pC) ASSERT(pThis != NULL); ASSERT(pC != NULL); - /* DEV debug only: dbgoprint((obj_t*) pThis, "strmRead index %d, max %d\n", pThis->iBufPtr, pThis->iBufPtrMax); */ + /* DEV debug only: DBGOPRINT((obj_t*) pThis, "strmRead index %d, max %d\n", pThis->iBufPtr, pThis->iBufPtrMax); */ if(pThis->iUngetC != -1) { /* do we have an "unread" char that we need to provide? */ *pC = pThis->iUngetC; ++pThis->iCurrOffs; /* one more octet read */ @@ -731,7 +731,7 @@ static rsRetVal strmCheckNextOutputFile(strm_t *pThis) strmWaitAsyncWriterDone(pThis); if(pThis->iCurrOffs >= pThis->iMaxFileSize) { - dbgoprint((obj_t*) pThis, "max file size %ld reached for %d, now %ld - starting new file\n", + DBGOPRINT((obj_t*) pThis, "max file size %ld reached for %d, now %ld - starting new file\n", (long) pThis->iMaxFileSize, pThis->fd, (long) pThis->iCurrOffs); CHKiRet(strmNextFile(pThis)); } @@ -811,7 +811,7 @@ doWriteCall(strm_t *pThis, uchar *pBuf, size_t *pLenBuf) pWriteBuf += iWritten; } while(lenBuf > 0); /* Warning: do..while()! */ - dbgoprint((obj_t*) pThis, "file %d write wrote %d bytes\n", pThis->fd, (int) iWritten); + DBGOPRINT((obj_t*) pThis, "file %d write wrote %d bytes\n", pThis->fd, (int) iWritten); finalize_it: *pLenBuf = iTotalWritten; @@ -1130,7 +1130,7 @@ strmFlush(strm_t *pThis) DEFiRet; ASSERT(pThis != NULL); - dbgoprint((obj_t*) pThis, "file %d flush, buflen %ld\n", pThis->fd, (long) pThis->iBufPtr); + DBGOPRINT((obj_t*) pThis, "file %d flush, buflen %ld\n", pThis->fd, (long) pThis->iBufPtr); if(pThis->tOperationsMode != STREAMMODE_READ && pThis->iBufPtr > 0) { iRet = strmSchedWrite(pThis, pThis->pIOBuf, pThis->iBufPtr); @@ -1155,7 +1155,7 @@ static rsRetVal strmSeek(strm_t *pThis, off_t offs) else strmFlush(pThis); int i; - dbgoprint((obj_t*) pThis, "file %d seek, pos %ld\n", pThis->fd, (long) offs); + DBGOPRINT((obj_t*) pThis, "file %d seek, pos %ld\n", pThis->fd, (long) offs); i = lseek(pThis->fd, offs, SEEK_SET); // TODO: check error! pThis->iCurrOffs = offs; /* we are now at *this* offset */ pThis->iBufPtr = 0; /* buffer invalidated */ -- cgit From d7faed130cb089343ea3d9875561582e6f1d469f Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 16 Mar 2010 19:06:01 +0100 Subject: bugfix(cosmetic): tried to close non-open fd, resulting in close(-1) --- runtime/stream.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 3373b795..87daedaf 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -325,8 +325,13 @@ static rsRetVal strmCloseFile(strm_t *pThis) } } - close(pThis->fd); - pThis->fd = -1; + /* the file may already be closed (or never have opened), so guard + * against this. -- rgerhards, 2010-03-19 + */ + if(pThis->fd != -1) { + close(pThis->fd); + pThis->fd = -1; + } if(pThis->fdDir != -1) { /* close associated directory handle, if it is open */ -- cgit From 1fb45d3e993e44e1595fc54f1ad3b786c66fbb4c Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 18 Mar 2010 12:34:26 +0100 Subject: some cleanup, some additional comments and a bit more debug output --- runtime/stream.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 87daedaf..9a0a8615 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -316,7 +316,8 @@ static rsRetVal strmCloseFile(strm_t *pThis) DEFiRet; ASSERT(pThis != NULL); - DBGOPRINT((obj_t*) pThis, "file %d closing\n", pThis->fd); + DBGOPRINT((obj_t*) pThis, "file %d(%s) closing\n", pThis->fd, + (pThis->pszFName == NULL) ? "N/A" : (char*)pThis->pszFName); if(pThis->tOperationsMode != STREAMMODE_READ) { strmFlush(pThis); @@ -929,6 +930,7 @@ asyncWriterThread(void *pPtr) while(1) { /* loop broken inside */ d_pthread_mutex_lock(&pThis->mut); +dbgprintf("XXX: asyncWriterThread iterating %s\n", pThis->pszFName); while(pThis->iCnt == 0) { if(pThis->bStopWriter) { pthread_cond_broadcast(&pThis->isEmpty); @@ -944,6 +946,7 @@ asyncWriterThread(void *pPtr) bTimedOut = 0; timeoutComp(&t, pThis->iFlushInterval * 2000); /* *1000 millisconds */ if(pThis->bDoTimedWait) { +dbgprintf("asyncWriter thread going to timeout sleep\n"); if(pthread_cond_timedwait(&pThis->notEmpty, &pThis->mut, &t) != 0) { int err = errno; if(err == ETIMEDOUT) { @@ -957,13 +960,16 @@ asyncWriterThread(void *pPtr) } } } else { +dbgprintf("asyncWriter thread going to eternal sleep\n"); d_pthread_cond_wait(&pThis->notEmpty, &pThis->mut); } +dbgprintf("asyncWriter woke up\n"); } bTimedOut = 0; /* we may have timed out, but there *is* work to do... */ iDeq = pThis->iDeq++ % STREAM_ASYNC_NUMBUFS; +dbgprintf("asyncWriter writes data\n"); doWriteInternal(pThis, pThis->asyncBuf[iDeq].pBuf, pThis->asyncBuf[iDeq].lenBuf); // TODO: error check????? 2009-07-06 @@ -1135,7 +1141,10 @@ strmFlush(strm_t *pThis) DEFiRet; ASSERT(pThis != NULL); - DBGOPRINT((obj_t*) pThis, "file %d flush, buflen %ld\n", pThis->fd, (long) pThis->iBufPtr); + DBGOPRINT((obj_t*) pThis, "file %d(%s) flush, buflen %ld\n", pThis->fd, pThis->pszFName, (long) pThis->iBufPtr); + DBGOPRINT((obj_t*) pThis, "file %d(%s) flush, buflen %ld%s\n", pThis->fd, + (pThis->pszFName == NULL) ? "N/A" : (char*)pThis->pszFName, + (long) pThis->iBufPtr, (pThis->iBufPtr == 0) ? " (no need to flush)" : ""); if(pThis->tOperationsMode != STREAMMODE_READ && pThis->iBufPtr > 0) { iRet = strmSchedWrite(pThis, pThis->pIOBuf, pThis->iBufPtr); -- cgit From e910078e41e2960f9afc55013cbd287532be478e Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 19 Mar 2010 07:19:28 +0100 Subject: bugfix: improper synchronization when "$OMFileFlushOnTXEnd on" was used Internal data structures were not properly protected due to missing mutex calls. --- runtime/stream.c | 46 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 11 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 9a0a8615..93f7fd58 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -67,7 +67,7 @@ DEFobjStaticHelpers DEFobjCurrIf(zlibw) /* forward definitions */ -static rsRetVal strmFlush(strm_t *pThis); +static rsRetVal strmFlushInternal(strm_t *pThis); static rsRetVal strmWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf); static rsRetVal strmCloseFile(strm_t *pThis); static void *asyncWriterThread(void *pPtr); @@ -309,7 +309,7 @@ strmWaitAsyncWriterDone(strm_t *pThis) * Note: it is valid to call this function when the physical file is closed. If so, * strmCloseFile() will still check if there is any unwritten data inside buffers * (this may be the case) and, if so, will open the file, write the data, and then - * close it again (this is done via strmFlush and friends). + * close it again (this is done via strmFlushInternal and friends). */ static rsRetVal strmCloseFile(strm_t *pThis) { @@ -320,7 +320,7 @@ static rsRetVal strmCloseFile(strm_t *pThis) (pThis->pszFName == NULL) ? "N/A" : (char*)pThis->pszFName); if(pThis->tOperationsMode != STREAMMODE_READ) { - strmFlush(pThis); + strmFlushInternal(pThis); if(pThis->bAsyncWrite) { strmWaitAsyncWriterDone(pThis); } @@ -939,7 +939,7 @@ dbgprintf("XXX: asyncWriterThread iterating %s\n", pThis->pszFName); } if(bTimedOut && pThis->iBufPtr > 0) { /* if we timed out, we need to flush pending data */ - strmFlush(pThis); + strmFlushInternal(pThis); bTimedOut = 0; continue; /* now we should have data */ } @@ -1136,12 +1136,11 @@ finalize_it: * rgerhards, 2008-01-10 */ static rsRetVal -strmFlush(strm_t *pThis) +strmFlushInternal(strm_t *pThis) { DEFiRet; ASSERT(pThis != NULL); - DBGOPRINT((obj_t*) pThis, "file %d(%s) flush, buflen %ld\n", pThis->fd, pThis->pszFName, (long) pThis->iBufPtr); DBGOPRINT((obj_t*) pThis, "file %d(%s) flush, buflen %ld%s\n", pThis->fd, (pThis->pszFName == NULL) ? "N/A" : (char*)pThis->pszFName, (long) pThis->iBufPtr, (pThis->iBufPtr == 0) ? " (no need to flush)" : ""); @@ -1154,6 +1153,31 @@ strmFlush(strm_t *pThis) } +/* flush stream output buffer to persistent storage. This can be called at any time + * and is automatically called when the output buffer is full. This function is for + * use by EXTERNAL callers. Do NOT use it internally. It locks the async writer + * mutex if ther is need to do so. + * rgerhards, 2010-03-18 + */ +static rsRetVal +strmFlush(strm_t *pThis) +{ + DEFiRet; + + ASSERT(pThis != NULL); + + if(pThis->bAsyncWrite) + d_pthread_mutex_lock(&pThis->mut); + CHKiRet(strmFlushInternal(pThis)); + +finalize_it: + if(pThis->bAsyncWrite) + d_pthread_mutex_unlock(&pThis->mut); + + RETiRet; +} + + /* seek a stream to a specific location. Pending writes are flushed, read data * is invalidated. * rgerhards, 2008-01-12 @@ -1167,7 +1191,7 @@ static rsRetVal strmSeek(strm_t *pThis, off_t offs) if(pThis->fd == -1) strmOpenFile(pThis); else - strmFlush(pThis); + strmFlushInternal(pThis); int i; DBGOPRINT((obj_t*) pThis, "file %d seek, pos %ld\n", pThis->fd, (long) offs); i = lseek(pThis->fd, offs, SEEK_SET); // TODO: check error! @@ -1208,7 +1232,7 @@ static rsRetVal strmWriteChar(strm_t *pThis, uchar c) /* if the buffer is full, we need to flush before we can write */ if(pThis->iBufPtr == pThis->sIOBufSize) { - CHKiRet(strmFlush(pThis)); + CHKiRet(strmFlushInternal(pThis)); } /* we now always have space for one character, so we simply copy it */ *(pThis->pIOBuf + pThis->iBufPtr) = c; @@ -1278,7 +1302,7 @@ strmWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf) iOffset = 0; do { if(pThis->iBufPtr == pThis->sIOBufSize) { - CHKiRet(strmFlush(pThis)); /* get a new buffer for rest of data */ + CHKiRet(strmFlushInternal(pThis)); /* get a new buffer for rest of data */ } iWrite = pThis->sIOBufSize - pThis->iBufPtr; /* this fits in current buf */ if(iWrite > lenBuf) @@ -1293,7 +1317,7 @@ strmWrite(strm_t *pThis, uchar *pBuf, size_t lenBuf) * write it. This seems more natural than waiting (hours?) for the next message... */ if(pThis->iBufPtr == pThis->sIOBufSize) { - CHKiRet(strmFlush(pThis)); /* get a new buffer for rest of data */ + CHKiRet(strmFlushInternal(pThis)); /* get a new buffer for rest of data */ } finalize_it: @@ -1452,7 +1476,7 @@ static rsRetVal strmSerialize(strm_t *pThis, strm_t *pStrm) ISOBJ_TYPE_assert(pThis, strm); ISOBJ_TYPE_assert(pStrm, strm); - strmFlush(pThis); + strmFlushInternal(pThis); CHKiRet(obj.BeginSerialize(pStrm, (obj_t*) pThis)); objSerializeSCALAR(pStrm, iCurrFNum, INT); -- cgit From 3e0578605f7df8427aa5b70c2b4396504113fafc Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 19 Mar 2010 07:37:56 +0100 Subject: bugfix: potential hang condition during filestream close predicate was not properly checked when waiting for the background file writer --- runtime/stream.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 93f7fd58..88606db7 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -163,7 +163,7 @@ doSizeLimitProcessing(strm_t *pThis) ASSERT(pThis->fd != -1); if(pThis->iCurrOffs >= pThis->iSizeLimit) { - /* strmClosefile() destroys the current file name, so we + /* strmCloseFile() destroys the current file name, so we * need to preserve it. */ CHKmalloc(pszCurrFName = ustrdup(pThis->pszCurrFName)); @@ -296,8 +296,10 @@ strmWaitAsyncWriterDone(strm_t *pThis) BEGINfunc if(pThis->bAsyncWrite) { /* awake writer thread and make it write out everything */ - pthread_cond_signal(&pThis->notEmpty); - d_pthread_cond_wait(&pThis->isEmpty, &pThis->mut); + while(pThis->iCnt > 0) { + pthread_cond_signal(&pThis->notEmpty); + d_pthread_cond_wait(&pThis->isEmpty, &pThis->mut); + } } ENDfunc } @@ -944,7 +946,7 @@ dbgprintf("XXX: asyncWriterThread iterating %s\n", pThis->pszFName); continue; /* now we should have data */ } bTimedOut = 0; - timeoutComp(&t, pThis->iFlushInterval * 2000); /* *1000 millisconds */ + timeoutComp(&t, pThis->iFlushInterval * 2000); /* *1000 millisconds */ // TODO: check the 2000?!? if(pThis->bDoTimedWait) { dbgprintf("asyncWriter thread going to timeout sleep\n"); if(pthread_cond_timedwait(&pThis->notEmpty, &pThis->mut, &t) != 0) { -- cgit From 9cdcba0bdc8b8d2df8d06784f3c4f0066c90fd40 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 19 Mar 2010 07:41:04 +0100 Subject: bugfix: invalid buffer write in (file) stream class currently being accessed buffer could be overwritten with new data. While this probably did not cause access violations, it could case loss and/or duplication of some data (definitely a race with no deterministic outcome) --- runtime/stream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index 88606db7..e2f6623e 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -863,7 +863,8 @@ doAsyncWriteInternal(strm_t *pThis, size_t lenBuf) DEFiRet; ISOBJ_TYPE_assert(pThis, strm); - while(pThis->iCnt >= STREAM_ASYNC_NUMBUFS) + /* the -1 below is important, because we need one buffer for the main thread! */ + while(pThis->iCnt >= STREAM_ASYNC_NUMBUFS - 1) d_pthread_cond_wait(&pThis->notFull, &pThis->mut); pThis->asyncBuf[pThis->iEnq % STREAM_ASYNC_NUMBUFS].lenBuf = lenBuf; -- cgit From 89216d6a96ea5f6d1fa9893d56fa877a2131d390 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 19 Mar 2010 09:42:46 +0100 Subject: fixed regression from previos (yet unrelease) $omfileFlushOnTXEnd fix The previous fix fixed an issue with on/off bying used in the exact wrong semantic. It corrected the situation, but failed to fix one spot where the wrong semantics were used. This is done with this commit. Note that this is NOT a bug seen in any released version. --- runtime/stream.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index e2f6623e..bfeecee5 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -641,6 +641,9 @@ static rsRetVal strmConstructFinalize(strm_t *pThis) } } + DBGPRINTF("file stream %s params: flush interval %d, async write %d\n", + (pThis->pszFName == NULL) ? "N/A" : (char*)pThis->pszFName, + pThis->iFlushInterval, pThis->bAsyncWrite); /* if we have a flush interval, we need to do async writes in any case */ if(pThis->iFlushInterval != 0) { pThis->bAsyncWrite = 1; -- cgit From 148910c285161097b44cd5df165c3bd19e21ae33 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 22 Mar 2010 15:47:07 +0100 Subject: bugfix(minor): BSD_SO_COMPAT query function had some global vars not properly initialized. However, in practice the loader initializes them with zero, the desired value, so there were no actual issue in almost all cases. --- runtime/net.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/net.c b/runtime/net.c index e91c8a7f..fe6eef5b 100644 --- a/runtime/net.c +++ b/runtime/net.c @@ -1010,8 +1010,8 @@ static int should_use_so_bsdcompat(void) { #ifndef OS_BSD - static int init_done; - static int so_bsdcompat_is_obsolete; + static int init_done = 0; + static int so_bsdcompat_is_obsolete = 0; if (!init_done) { struct utsname myutsname; -- cgit From 5d58774813d4ecd4fc9f8230f8d5446457eb2ed5 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 23 Mar 2010 14:21:33 +0100 Subject: streamline dynafile cache entry deletion a bit The old code looks a bit "strange", though not necessarily incorrect. The new code looks correct and is probably less irritating during bug hunting. --- runtime/stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'runtime') diff --git a/runtime/stream.c b/runtime/stream.c index bfeecee5..e8805a40 100644 --- a/runtime/stream.c +++ b/runtime/stream.c @@ -722,7 +722,7 @@ CODESTARTobjDestruct(strm) free(pThis->pZipBuf); free(pThis->pszCurrFName); free(pThis->pszFName); - + pThis->bStopWriter = 2; /* RG: use as flag for destruction */ ENDobjDestruct(strm) -- cgit From 072fc663a83b2050d961d1c7ec7aa16f12842c9a Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 23 Mar 2010 15:04:24 +0100 Subject: added replacements for atomic instructions on systems that do not support them. [backport of Stefen Sledz' patch for v5] --- runtime/Makefile.am | 1 + runtime/atomic.h | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++ runtime/rsyslog.c | 14 +++++++ 3 files changed, 132 insertions(+) (limited to 'runtime') diff --git a/runtime/Makefile.am b/runtime/Makefile.am index 14abe722..e5b3ccdf 100644 --- a/runtime/Makefile.am +++ b/runtime/Makefile.am @@ -9,6 +9,7 @@ librsyslog_la_SOURCES = \ rsyslog.h \ unicode-helper.h \ atomic.h \ + atomic-posix-sem.c \ syslogd-types.h \ module-template.h \ obj-types.h \ diff --git a/runtime/atomic.h b/runtime/atomic.h index 62ceb8b3..ea3be37a 100644 --- a/runtime/atomic.h +++ b/runtime/atomic.h @@ -53,6 +53,122 @@ # define ATOMIC_CAS(data, oldVal, newVal) __sync_bool_compare_and_swap(&(data), (oldVal), (newVal)); # define ATOMIC_CAS_VAL(data, oldVal, newVal) __sync_val_compare_and_swap(&(data), (oldVal), (newVal)); #else +#ifdef HAVE_SEMAPHORE_H + /* we use POSIX semaphores instead */ + +#include "rsyslog.h" +#include + +extern sem_t atomicSem; +rsRetVal atomicSemInit(void); +void atomicSemExit(void); + +#if HAVE_TYPEOF +#define my_typeof(x) typeof(x) +#else /* sorry, can't determine types, using 'int' */ +#define my_typeof(x) int +#endif + +# define ATOMIC_SUB(data, val) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data -= val; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_ADD(data, val) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data += val; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_INC_AND_FETCH(data) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data += 1; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_INC(data) ((void) ATOMIC_INC_AND_FETCH(data)) + +# define ATOMIC_DEC_AND_FETCH(data) \ +({ \ + sem_wait(&atomicSem); \ + data -= 1; \ + sem_post(&atomicSem); \ + data; \ +}) + +# define ATOMIC_DEC(data) ((void) ATOMIC_DEC_AND_FETCH(data)) + +# define ATOMIC_FETCH_32BIT(data) ((unsigned) ATOMIC_ADD((data), 0xffffffff)) + +# define ATOMIC_STORE_1_TO_32BIT(data) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data = 1; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_STORE_0_TO_INT(data) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data = 0; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_STORE_1_TO_INT(data) \ +({ \ + my_typeof(data) tmp; \ + sem_wait(&atomicSem); \ + tmp = data; \ + data = 1; \ + sem_post(&atomicSem); \ + tmp; \ +}) + +# define ATOMIC_CAS(data, oldVal, newVal) \ +({ \ + int ret; \ + sem_wait(&atomicSem); \ + if(data != oldVal) ret = 0; \ + else \ + { \ + data = newVal; \ + ret = 1; \ + } \ + sem_post(&atomicSem); \ + ret; \ +}) + +# define ATOMIC_CAS_VAL(data, oldVal, newVal) \ +({ \ + sem_wait(&atomicSem); \ + if(data == oldVal) \ + { \ + data = newVal; \ + } \ + sem_post(&atomicSem); \ + data; \ +}) + +#else /* not HAVE_SEMAPHORE_H */ /* note that we gained parctical proof that theoretical problems DO occur * if we do not properly address them. See this blog post for details: * http://blog.gerhards.net/2009/01/rsyslog-data-race-analysis.html @@ -70,5 +186,6 @@ # define ATOMIC_STORE_0_TO_INT(data) (data) = 0 # define ATOMIC_CAS_VAL(data, oldVal, newVal) (data) = (newVal) #endif +#endif #endif /* #ifndef INCLUDED_ATOMIC_H */ diff --git a/runtime/rsyslog.c b/runtime/rsyslog.c index 443d0f41..5750ca76 100644 --- a/runtime/rsyslog.c +++ b/runtime/rsyslog.c @@ -80,6 +80,7 @@ #include "prop.h" #include "rule.h" #include "ruleset.h" +#include "atomic.h" /* forward definitions */ static rsRetVal dfltErrLogger(int, uchar *errMsg); @@ -139,6 +140,12 @@ rsrtInit(char **ppErrObj, obj_if_t *pObjIF) CHKiRet(objClassInit(NULL)); /* *THIS* *MUST* always be the first class initilizer being called! */ CHKiRet(objGetObjInterface(pObjIF)); /* this provides the root pointer for all other queries */ +#ifndef HAVE_ATOMIC_BUILTINS +#ifdef HAVE_SEMAPHORE_H + CHKiRet(atomicSemInit()); +#endif /* HAVE_SEMAPHORE_H */ +#endif /* !defined(HAVE_ATOMIC_BUILTINS) */ + /* initialize core classes. We must be very careful with the order of events. Some * classes use others and if we do not initialize them in the right order, we may end * up with an invalid call. The most important thing that can happen is that an error @@ -215,6 +222,13 @@ rsrtExit(void) glblClassExit(); rulesetClassExit(); ruleClassExit(); + +#ifndef HAVE_ATOMIC_BUILTINS +#ifdef HAVE_SEMAPHORE_H + atomicSemExit(); +#endif /* HAVE_SEMAPHORE_H */ +#endif /* !defined(HAVE_ATOMIC_BUILTINS) */ + objClassExit(); /* *THIS* *MUST/SHOULD?* always be the first class initilizer being called (except debug)! */ } -- cgit From 2f8b5cafd0dcc5f09b7b64f5015a2aebd1c7b31c Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Tue, 23 Mar 2010 15:05:34 +0100 Subject: forgot to add file with last commit --- runtime/atomic-posix-sem.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 runtime/atomic-posix-sem.c (limited to 'runtime') diff --git a/runtime/atomic-posix-sem.c b/runtime/atomic-posix-sem.c new file mode 100644 index 00000000..979fae02 --- /dev/null +++ b/runtime/atomic-posix-sem.c @@ -0,0 +1,70 @@ +/* atomic_posix_sem.c: This file supplies an emulation for atomic operations using + * POSIX semaphores. + * + * Copyright 2010 DResearch Digital Media Systems GmbH + * + * This file is part of the rsyslog runtime library. + * + * The rsyslog runtime library is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * The rsyslog runtime library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with the rsyslog runtime library. If not, see . + * + * A copy of the GPL can be found in the file "COPYING" in this distribution. + * A copy of the LGPL can be found in the file "COPYING.LESSER" in this distribution. + */ + +#include "config.h" +#ifndef HAVE_ATOMIC_BUILTINS +#ifdef HAVE_SEMAPHORE_H +#include +#include + +#include "atomic.h" +#include "rsyslog.h" +#include "srUtils.h" + +sem_t atomicSem; + +rsRetVal +atomicSemInit(void) +{ + DEFiRet; + + dbgprintf("init posix semaphore for atomics emulation\n"); + if(sem_init(&atomicSem, 0, 1) == -1) + { + char errStr[1024]; + rs_strerror_r(errno, errStr, sizeof(errStr)); + dbgprintf("init posix semaphore for atomics emulation failed: %s\n", errStr); + iRet = RS_RET_SYS_ERR; /* the right error code ??? */ + } + + RETiRet; +} + +void +atomicSemExit(void) +{ + dbgprintf("destroy posix semaphore for atomics emulation\n"); + if(sem_destroy(&atomicSem) == -1) + { + char errStr[1024]; + rs_strerror_r(errno, errStr, sizeof(errStr)); + dbgprintf("destroy posix semaphore for atomics emulation failed: %s\n", errStr); + } +} + +#endif /* HAVE_SEMAPHORE_H */ +#endif /* !defined(HAVE_ATOMIC_BUILTINS) */ + +/* vim:set ai: + */ -- cgit From 9e5b31fc44136dbcc1e443cfe7714e9daf97d844 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 25 Mar 2010 07:50:55 +0100 Subject: bugfix: race condition during directory creation If multiple files try to create a directory at (almost) the same time, some of them may fail. This is a data race and also exists with other processes that may create the same directory. We do now check for this condition and gracefully handle it. --- runtime/srutils.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'runtime') diff --git a/runtime/srutils.c b/runtime/srutils.c index c403b312..1452c9b7 100644 --- a/runtime/srutils.c +++ b/runtime/srutils.c @@ -166,10 +166,22 @@ uchar *srUtilStrDup(uchar *pOld, size_t len) /* creates a path recursively - * Return 0 on success, -1 otherwise. On failure, errno - * hold the last OS error. - * Param "mode" holds the mode that all non-existing directories - * are to be created with. + * Return 0 on success, -1 otherwise. On failure, errno * hold the last OS error. + * Param "mode" holds the mode that all non-existing directories are to be + * created with. + * Note that we have a potential race inside that code, a race that even exists + * outside of the rsyslog process (if multiple instances run, or other programs + * generate directories): If the directory does not exist, a context switch happens, + * at that moment another process creates it, then our creation on the context + * switch back fails. This actually happened in practice, and depending on the + * configuration it is even likely to happen. We can not solve this situation + * with a mutex, as that works only within out process space. So the solution + * is that we take the optimistic approach, try the creation, and if it fails + * with "already exists" we go back and do one retry of the check/create + * sequence. That should then succeed. If the directory is still not found but + * the creation fails in the similar way, we return an error on that second + * try because otherwise we would potentially run into an endless loop. + * loop. -- rgerhards, 2010-03-25 */ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, uid_t uid, gid_t gid, int bFailOnChownFail) @@ -177,6 +189,8 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, uchar *p; uchar *pszWork; size_t len; + int err; + int iTry = 0; int bErr = 0; assert(szFile != NULL); @@ -190,8 +204,9 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, if(*p == '/') { /* temporarily terminate string, create dir and go on */ *p = '\0'; +again: if(access((char*)pszWork, F_OK)) { - if(mkdir((char*)pszWork, mode) == 0) { + if((err = mkdir((char*)pszWork, mode)) == 0) { if(uid != (uid_t) -1 || gid != (gid_t) -1) { /* we need to set owner/group */ if(chown((char*)pszWork, uid, gid) != 0) @@ -201,8 +216,13 @@ int makeFileParentDirs(uchar *szFile, size_t lenFile, mode_t mode, * to do so. */ } - } else + } else { + if(err == EEXIST && iTry == 0) { + iTry = 1; + goto again; + } bErr = 1; + } if(bErr) { int eSave = errno; free(pszWork); -- cgit From a3e48b697fa664110567fcd0027d24ea5a239041 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 25 Mar 2010 08:03:37 +0100 Subject: bugfix(temporary): message-induced off-by-one error (potential segfault) Some types of malformed messages could trigger an off-by-one error (for example, \0 or \n as the last character, and generally control character escaption is questionable). This is due to not strictly following a the \0 or string counted string paradigm (during the last optimization on the cstring class). As a temporary fix, we have introduced a proper recalculation of the size. However, a final patch is expected in the future. See bug tracker for further details and when the final patch will be available: http://bugzilla.adiscon.com/show_bug.cgi?id=184 Note that the current patch is considered sufficient to solve the situation, but it requires a bit more runtime than desirable. --- runtime/msg.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'runtime') diff --git a/runtime/msg.c b/runtime/msg.c index 3a2331f4..2ce7843a 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -2319,6 +2319,12 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, *pPropLen = sizeof("**INVALID PROPERTY NAME**") - 1; return UCHAR_CONSTANT("**INVALID PROPERTY NAME**"); } + /* the following line fixes the symptom, but not the root cause -- at least MSG sometimes + * returns a size of one too less. To prevent all troubles, we recalculate the sizes based + * on what we actually got. TODO: remove once root cause is found. + * rgerhards, 2010-03-23 + */ + bufLen = ustrlen(pRes); /* If we did not receive a template pointer, we are already done... */ -- cgit From 2cd132eebb84dbcffcf0c20b9354c14f797c29cd Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 7 Apr 2010 12:42:41 +0200 Subject: enhanced nettester tool so that it re-uses it's callers environment this enables us to work with the "usual" environment tweaks (for debugging and other purposes), without the need for any special handling in nettester itself --- runtime/msg.c | 9 ++------- runtime/parser.c | 9 +++++++-- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'runtime') diff --git a/runtime/msg.c b/runtime/msg.c index 2ce7843a..91057f97 100644 --- a/runtime/msg.c +++ b/runtime/msg.c @@ -2010,6 +2010,8 @@ finalize_it: /* set raw message in message object. Size of message is provided. + * The function makes sure that the stored rawmsg is properly + * terminated by '\0'. * rgerhards, 2009-06-16 */ void MsgSetRawMsg(msg_t *pThis, char* pszRawMsg, size_t lenMsg) @@ -2319,13 +2321,6 @@ uchar *MsgGetProp(msg_t *pMsg, struct templateEntry *pTpe, *pPropLen = sizeof("**INVALID PROPERTY NAME**") - 1; return UCHAR_CONSTANT("**INVALID PROPERTY NAME**"); } - /* the following line fixes the symptom, but not the root cause -- at least MSG sometimes - * returns a size of one too less. To prevent all troubles, we recalculate the sizes based - * on what we actually got. TODO: remove once root cause is found. - * rgerhards, 2010-03-23 - */ - bufLen = ustrlen(pRes); - /* If we did not receive a template pointer, we are already done... */ if(pTpe == NULL) { diff --git a/runtime/parser.c b/runtime/parser.c index 466066e7..36e88ebd 100644 --- a/runtime/parser.c +++ b/runtime/parser.c @@ -176,7 +176,10 @@ sanitizeMessage(msg_t *pMsg) pszMsg = pMsg->pszRawMsg; lenMsg = pMsg->iLenRawMsg; - /* remove NUL character at end of message (see comment in function header) */ + /* remove NUL character at end of message (see comment in function header) + * Note that we do not need to add a NUL character in this case, because it + * is already present ;) + */ if(pszMsg[lenMsg-1] == '\0') { DBGPRINTF("dropped NUL at very end of message\n"); bUpdatedLen = TRUE; @@ -190,8 +193,9 @@ sanitizeMessage(msg_t *pMsg) */ if(bDropTrailingLF && pszMsg[lenMsg-1] == '\n') { DBGPRINTF("dropped LF at very end of message (DropTrailingLF is set)\n"); - bUpdatedLen = TRUE; lenMsg--; + pszMsg[lenMsg] = '\0'; + bUpdatedLen = TRUE; } /* it is much quicker to sweep over the message and see if it actually @@ -245,6 +249,7 @@ sanitizeMessage(msg_t *pMsg) } ++iSrc; } + pDst[iDst] = '\0'; MsgSetRawMsg(pMsg, (char*)pDst, iDst); /* save sanitized string */ -- cgit From 7db6ffbce3e2a709e77ff05782f703ec112ed84b Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Wed, 7 Apr 2010 13:57:46 +0200 Subject: bugfix: the T/P/E config size specifiers did not work properly under call 32-bit platforms --- runtime/cfsysline.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'runtime') diff --git a/runtime/cfsysline.c b/runtime/cfsysline.c index 184c0d87..5df8e64c 100644 --- a/runtime/cfsysline.c +++ b/runtime/cfsysline.c @@ -217,9 +217,11 @@ static rsRetVal doGetSize(uchar **pp, rsRetVal (*pSetHdlr)(void*, uid_t), void * case 'K': i *= 1000; ++(*pp); break; case 'M': i *= 1000000; ++(*pp); break; case 'G': i *= 1000000000; ++(*pp); break; - case 'T': i *= 1000000000000; ++(*pp); break; /* tera */ - case 'P': i *= 1000000000000000; ++(*pp); break; /* peta */ - case 'E': i *= 1000000000000000000; ++(*pp); break; /* exa */ + /* we need to use the multiplication below because otherwise + * the compiler gets an error during constant parsing */ + case 'T': i *= (int64) 1000 * 1000000000; ++(*pp); break; /* tera */ + case 'P': i *= (int64) 1000000 * 1000000000; ++(*pp); break; /* peta */ + case 'E': i *= (int64) 1000000000 * 1000000000; ++(*pp); break; /* exa */ } /* done */ -- cgit