From d0ca3acbf971141b8826d0bb3a184eaadb2804c1 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Thu, 3 Apr 2008 08:49:09 +0000 Subject: bugfix: memory leaks in script engine --- ChangeLog | 1 + conf.c | 6 ++++++ obj.h | 1 - queue.c | 2 ++ syslogd.c | 15 ++++++++++++--- var.c | 28 +++++++++++++++++++++------- vm.c | 5 ++--- vmop.c | 6 ++++++ vmstk.c | 2 -- 9 files changed, 50 insertions(+), 16 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3c20f16b..66994c28 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,7 @@ Version 3.15.1 (rgerhards), 2008-04-?? - file relputil.c deleted, is not actually needed - added more meaningful error messages to rsyslogd (when some errors happens during startup) +- bugfix: memory leaks in script engine --------------------------------------------------------------------------- Version 3.15.0 (rgerhards), 2008-04-01 - major new feature: imrelp/omrelp support reliable delivery of syslog diff --git a/conf.c b/conf.c index 14e73f0e..f87ab992 100644 --- a/conf.c +++ b/conf.c @@ -68,6 +68,7 @@ static rsRetVal processConfFile(uchar *pConfFile); DEFobjStaticHelpers DEFobjCurrIf(expr) DEFobjCurrIf(ctok) +DEFobjCurrIf(ctok_token) DEFobjCurrIf(module) DEFobjCurrIf(errmsg) DEFobjCurrIf(net) @@ -762,9 +763,12 @@ dbgprintf("calling expression parser, pp %p ('%s')\n", *pline, *pline); */ CHKiRet(ctok.GetToken(tok, &pToken)); if(pToken->tok != ctok_THEN) { + ctok_token.Destruct(&pToken); ABORT_FINALIZE(RS_RET_SYNTAX_ERROR); } + ctok_token.Destruct(&pToken); /* no longer needed */ + /* we are done, so we now need to restore things */ CHKiRet(ctok.Getpp(tok, pline)); CHKiRet(ctok.Destruct(&tok)); @@ -1183,6 +1187,7 @@ CODESTARTObjClassExit(conf) /* release objects we no longer need */ objRelease(expr, CORE_COMPONENT); objRelease(ctok, CORE_COMPONENT); + objRelease(ctok_token, CORE_COMPONENT); objRelease(module, CORE_COMPONENT); objRelease(errmsg, CORE_COMPONENT); objRelease(net, LM_NET_FILENAME); @@ -1197,6 +1202,7 @@ BEGINAbstractObjClassInit(conf, 1, OBJ_IS_CORE_MODULE) /* class, version - CHANG /* request objects we use */ CHKiRet(objUse(expr, CORE_COMPONENT)); CHKiRet(objUse(ctok, CORE_COMPONENT)); + CHKiRet(objUse(ctok_token, CORE_COMPONENT)); CHKiRet(objUse(module, CORE_COMPONENT)); CHKiRet(objUse(errmsg, CORE_COMPONENT)); CHKiRet(objUse(net, LM_NET_FILENAME)); /* TODO: make this dependcy go away! */ diff --git a/obj.h b/obj.h index a9d676d5..87c6c91d 100644 --- a/obj.h +++ b/obj.h @@ -92,7 +92,6 @@ /* interfaces */ BEGINinterface(obj) /* name must also be changed in ENDinterface macro! */ - //rsRetVal (*UseObj)(char *srcFile, uchar *pObjName, uchar *pMyLib, uchar *pObjFile, interface_t *pIf); rsRetVal (*UseObj)(char *srcFile, uchar *pObjName, uchar *pObjFile, interface_t *pIf); rsRetVal (*ReleaseObj)(char *srcFile, uchar *pObjName, uchar *pObjFile, interface_t *pIf); rsRetVal (*InfoConstruct)(objInfo_t **ppThis, uchar *pszID, int iObjVers, diff --git a/queue.c b/queue.c index ed720c55..7ae5815d 100644 --- a/queue.c +++ b/queue.c @@ -2208,6 +2208,8 @@ rsRetVal queueQueryInterface(void) { return RS_RET_NOT_IMPLEMENTED; } */ BEGINObjClassInit(queue, 1, OBJ_IS_CORE_MODULE) /* request objects we use */ +DEFpropSetMeth(queue, iDeqtWinFromHr, int); +DEFpropSetMeth(queue, iDeqtWinToHr, int); /* now set our own handlers */ OBJSetMethodHandler(objMethod_SETPROPERTY, queueSetProperty); diff --git a/syslogd.c b/syslogd.c index 8c71f57d..b5554e5f 100644 --- a/syslogd.c +++ b/syslogd.c @@ -170,6 +170,7 @@ DEFobjCurrIf(datetime) DEFobjCurrIf(conf) DEFobjCurrIf(expr) DEFobjCurrIf(vm) +DEFobjCurrIf(var) DEFobjCurrIf(module) DEFobjCurrIf(errmsg) DEFobjCurrIf(net) /* TODO: make go away! */ @@ -928,8 +929,8 @@ static rsRetVal shouldProcessThisMessage(selector_t *f, msg_t *pMsg, int *bProce unsigned short pbMustBeFreed; char *pszPropVal; int bRet = 0; - vm_t *pVM; - var_t *pResult; + vm_t *pVM = NULL; + var_t *pResult = NULL; assert(f != NULL); assert(pMsg != NULL); @@ -995,7 +996,7 @@ static rsRetVal shouldProcessThisMessage(selector_t *f, msg_t *pMsg, int *bProce CHKiRet(vm.ExecProg(pVM, f->f_filterData.f_expr->pVmprg)); CHKiRet(vm.PopBoolFromStack(pVM, &pResult)); dbgprintf("result of expression evaluation: %lld\n", pResult->val.num); - CHKiRet(vm.Destruct(&pVM)); + /* VM is destructed on function exit */ bRet = (pResult->val.num) ? 1 : 0; } else { assert(f->f_filter_type == FILTER_PROP); /* assert() just in case... */ @@ -1051,6 +1052,12 @@ static rsRetVal shouldProcessThisMessage(selector_t *f, msg_t *pMsg, int *bProce } finalize_it: + /* destruct in any case, not just on error, but it makes error handling much easier */ + if(pVM != NULL) { + var.Destruct(&pResult); + vm.Destruct(&pVM); + } + *bProcessMsg = bRet; RETiRet; } @@ -2838,6 +2845,8 @@ InitGlobalClasses(void) CHKiRet(objUse(errmsg, CORE_COMPONENT)); pErrObj = "module"; CHKiRet(objUse(module, CORE_COMPONENT)); + pErrObj = "var"; + CHKiRet(objUse(var, CORE_COMPONENT)); /* initialize and use 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 diff --git a/var.c b/var.c index c1d66643..7de00d88 100644 --- a/var.c +++ b/var.c @@ -63,12 +63,11 @@ rsRetVal varConstructFinalize(var_t __attribute__((unused)) *pThis) BEGINobjDestruct(var) /* be sure to specify the object type also in END and CODESTART macros! */ CODESTARTobjDestruct(var) if(pThis->pcsName != NULL) - d_free(pThis->pcsName); + rsCStrDestruct(&pThis->pcsName); if(pThis->varType == VARTYPE_STR) { if(pThis->val.pStr != NULL) - d_free(pThis->val.pStr); + rsCStrDestruct(&pThis->val.pStr); } - ENDobjDestruct(var) @@ -192,10 +191,16 @@ ConvToNumber(var_t *pThis) } else if(pThis->varType == VARTYPE_STR) { iRet = rsCStrConvertToNumber(pThis->val.pStr, &n); if(iRet == RS_RET_NOT_A_NUMBER) { - n = 0; /* TODO: isn't it better to pass the error? */ + n = 0; + iRet = RS_RET_OK; /* we accept this as part of the language definition */ } else if (iRet != RS_RET_OK) { FINALIZE; } + + /* we need to destruct the string first, because string and number are + * inside a union and share the memory area! -- rgerhards, 2008-04-03 + */ + rsCStrDestruct(&pThis->val.pStr); pThis->val.num = n; pThis->varType = VARTYPE_NUMBER; @@ -244,7 +249,18 @@ ConvToBool(var_t *pThis) if(pThis->varType == VARTYPE_NUMBER) { FINALIZE; } else if(pThis->varType == VARTYPE_STR) { - CHKiRet(rsCStrConvertToBool(pThis->val.pStr, &n)); + iRet = rsCStrConvertToBool(pThis->val.pStr, &n); + if(iRet == RS_RET_NOT_A_NUMBER) { + n = 0; + iRet = RS_RET_OK; /* we accept this as part of the language definition */ + } else if (iRet != RS_RET_OK) { + FINALIZE; + } + + /* we need to destruct the string first, because string and number are + * inside a union and share the memory area! -- rgerhards, 2008-04-03 + */ + rsCStrDestruct(&pThis->val.pStr); pThis->val.num = n; pThis->varType = VARTYPE_NUMBER; } @@ -366,8 +382,6 @@ CODESTARTobjQueryInterface(var) * work here (if we can support an older interface version - that, * of course, also affects the "if" above). */ - //xxxpIf->oID = OBJvar; - pIf->Construct = varConstruct; pIf->ConstructFinalize = varConstructFinalize; pIf->Destruct = varDestruct; diff --git a/vm.c b/vm.c index 3cd05622..a26e4331 100644 --- a/vm.c +++ b/vm.c @@ -109,8 +109,6 @@ BEGINop(name) \ number_t bRes; \ CODESTARTop(name) \ CHKiRet(vmstk.Pop2CommOp(pThis->pStk, &operand1, &operand2)); \ -var.DebugPrint(operand1); \ -var.DebugPrint(operand2); \ /* data types are equal (so we look only at operand1), but we must \ * check which type we have to deal with... \ */ \ @@ -122,7 +120,6 @@ var.DebugPrint(operand2); \ } \ \ /* we have a result, so let's push it */ \ -RUNLOG_VAR("%lld", bRes); \ var.SetNumber(operand1, bRes); \ vmstk.Push(pThis->pStk, operand1); /* result */ \ var.Destruct(&operand2); /* no longer needed */ \ @@ -374,6 +371,8 @@ BEGINobjDestruct(vm) /* be sure to specify the object type also in END and CODES CODESTARTobjDestruct(vm) if(pThis->pStk != NULL) vmstk.Destruct(&pThis->pStk); + if(pThis->pMsg != NULL) + msgDestruct(&pThis->pMsg); ENDobjDestruct(vm) diff --git a/vmop.c b/vmop.c index affa83e7..91f84b91 100644 --- a/vmop.c +++ b/vmop.c @@ -59,6 +59,12 @@ rsRetVal vmopConstructFinalize(vmop_t __attribute__((unused)) *pThis) /* destructor for the vmop object */ BEGINobjDestruct(vmop) /* be sure to specify the object type also in END and CODESTART macros! */ CODESTARTobjDestruct(vmop) + if( pThis->opcode == opcode_PUSHSYSVAR + || pThis->opcode == opcode_PUSHMSGVAR + || pThis->opcode == opcode_PUSHCONSTANT) { + if(pThis->operand.pVar != NULL) + var.Destruct(&pThis->operand.pVar); + } ENDobjDestruct(vmop) diff --git a/vmstk.c b/vmstk.c index 9ca815ff..f6614f71 100644 --- a/vmstk.c +++ b/vmstk.c @@ -201,8 +201,6 @@ CODESTARTobjQueryInterface(vmstk) * work here (if we can support an older interface version - that, * of course, also affects the "if" above). */ - //xxxpIf->oID = OBJvmstk; - pIf->Construct = vmstkConstruct; pIf->ConstructFinalize = vmstkConstructFinalize; pIf->Destruct = vmstkDestruct; -- cgit v1.2.3