Re: Bug#651529: rsyslog: FTBFS on hurd-i386
Hi!
On Tue, 2011-12-13 at 09:50:21 +0100, Svante Signell wrote:
> On Tue, 2011-12-13 at 06:41 +0100, Guillem Jover wrote:
> > On Fri, 2011-12-09 at 16:40:41 +0100, Svante Signell wrote:
> > > Source: rsyslog
> > > Version: 5.8.6-1
> > > Severity: important
> > > Tags: patch
> > > User: debian-hurd@lists.debian.org
> > > Usertags: hurd
> --- rsyslog-5.8.6/runtime/modules.c 2011-10-21 11:53:02.000000000 +0200
> +++ rsyslog-5.8.6.modified/runtime/modules.c 2011-12-13 09:25:16.000000000 +0100
> @@ -767,9 +767,9 @@
> DEFiRet;
>
> size_t iPathLen, iModNameLen;
> - uchar szPath[PATH_MAX];
> + uchar *szPath = NULL;
> uchar *pModNameCmp;
> - int bHasExtension;
> + int bHasExtension, pHasSlash;
> void *pModHdlr, *pModInit;
> modInfo_t *pModInfo;
> uchar *pModDirCurr, *pModDirNext;
> @@ -805,12 +805,16 @@
> do {
> /* now build our load module name */
> if(*pModName == '/' || *pModName == '.') {
> - *szPath = '\0'; /* we do not need to append the path - its already in the module name */
> + *szPath = '\0'; /* we do not need to append the path - its already in the module name */
Still not allocating, and now assigning which will segfault right away.
Allocation should happen as the first thing on the loop, before any
modification to szPath.
> iPathLen = 0;
> } else {
> - *szPath = '\0';
> -
> iPathLen = strlen((char *)pModDirCurr);
> + if(pModDirCurr[iPathLen - 1] == '/')
> + pHasSlash = TRUE;
> + else {
> + iPathLen += iPathLen;
Why doubling the memory required? Just 1 would be enough I'd say.
> + pHasSlash = FALSE;
> + }
> pModDirNext = (uchar *)strchr((char *)pModDirCurr, ':');
> if(pModDirNext)
> iPathLen = (size_t)(pModDirNext - pModDirCurr);
> @@ -821,45 +825,38 @@
> continue;
> }
> break;
> - } else if(iPathLen > sizeof(szPath) - 1) {
> - errmsg.LogError(0, NO_ERRCODE, "could not load module '%s', module path too long\n", pModName);
> + }
> + if(szPath != NULL)
> + free(szPath);
> + iPathLen = iPathLen + iModNameLen + 3;
This will make the subsequent handling of pHasSlash bogus.
> + if((szPath = malloc(iPathLen+1)) == NULL) {
> + errmsg.LogError(0, NO_ERRCODE, "could not allocate memory '%s'\n", pModName);
> ABORT_FINALIZE(RS_RET_MODULE_LOAD_ERR_PATHLEN);
> }
>
> - strncat((char *) szPath, (char *)pModDirCurr, iPathLen);
> - iPathLen = strlen((char*) szPath);
> + strcpy((char *) szPath, (char *)pModDirCurr);
>
> if(pModDirNext)
> pModDirCurr = pModDirNext + 1;
>
> - if((szPath[iPathLen - 1] != '/')) {
> - if((iPathLen <= sizeof(szPath) - 2)) {
> - szPath[iPathLen++] = '/';
> - szPath[iPathLen] = '\0';
> - } else {
> - errmsg.LogError(0, RS_RET_MODULE_LOAD_ERR_PATHLEN, "could not load module '%s', path too long\n", pModName);
> - ABORT_FINALIZE(RS_RET_MODULE_LOAD_ERR_PATHLEN);
> - }
> + if(!pHasSlash) {
> + szPath[iPathLen - 2] = '/';
> + szPath[iPathLen - 1] = '\0';
iPathLen does not point anymore to the actual end of the written string,
but to the end of the allocated memory. In addition -2 would not be
right either, as if '/' is not found in - 1 (pHasSlash), then we have
to append it, which means just iPathLen. This is the code as can be
seen originally.
regards,
guillem
Reply to: