From 0114b531b38b16db98b04b680017d6c758987fd9 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Fri, 15 Mar 2013 14:33:23 +0100 Subject: debug system improvement & bug fix for init system status (see below) - rsyslogd startup information is now properly conveyed back to init when privileges are beging dropped Actually, we have moved termination of the parent in front of the priv drop. So it shall work now in all cases. See code comments in commit for more details. - If forking, the parent now waits for a maximum of 60 seconds for termination by the child - improved debugging support in forked (auto-backgrounding) mode The rsyslog debug log file is now continued to be written across the fork. --- ChangeLog | 10 ++++++++++ runtime/debug.c | 9 +++++++++ runtime/debug.h | 1 + tools/syslogd.c | 48 ++++++++++++++++++++++++++++-------------------- 4 files changed, 48 insertions(+), 20 deletions(-) diff --git a/ChangeLog b/ChangeLog index c41a89c0..b143ae81 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,16 @@ Version 7.2.7 [v7-stable] 2013-03-?? config directives. - doc bugfix: rsyslog.conf man page had invalid file format info closes: http://bugzilla.adiscon.com/show_bug.cgi?id=418 +- rsyslogd startup information is now properly conveyed back to init + when privileges are beging dropped + Actually, we have moved termination of the parent in front of the + priv drop. So it shall work now in all cases. See code comments in + commit for more details. +- If forking, the parent now waits for a maximum of 60 seconds for + termination by the child +- improved debugging support in forked (auto-backgrounding) mode + The rsyslog debug log file is now continued to be written across the + fork. ---------------------------------------------------------------------------- Version 7.2.6 [v7-stable] 2013-03-05 - slightly improved config parser error messages when invalid escapes happen diff --git a/runtime/debug.c b/runtime/debug.c index 1d306dbd..26da31b4 100644 --- a/runtime/debug.c +++ b/runtime/debug.c @@ -1296,6 +1296,15 @@ dbgmalloc(size_t size) } +/* report fd used for debug log. This is needed in case of + * auto-backgrounding, where the debug log shall not be closed. + */ +int +dbgGetDbglogFd(void) +{ + return altdbg; +} + /* read in the runtime options * rgerhards, 2008-02-28 */ diff --git a/runtime/debug.h b/runtime/debug.h index 5bd26bd8..e6971f82 100644 --- a/runtime/debug.h +++ b/runtime/debug.h @@ -104,6 +104,7 @@ void dbgSetExecLocation(int iStackPtr, int line); void dbgSetThrdName(uchar *pszName); void dbgPrintAllDebugInfo(void); void *dbgmalloc(size_t size); +int dbgGetDbglogFd(void); /* macros */ #ifdef DEBUGLESS diff --git a/tools/syslogd.c b/tools/syslogd.c index 62c18e72..0d7aac6c 100644 --- a/tools/syslogd.c +++ b/tools/syslogd.c @@ -224,7 +224,7 @@ struct queuefilenames_s { int MarkInterval = 20 * 60; /* interval between marks in seconds - read-only after startup */ int send_to_all = 0; /* send message to all IPv4/IPv6 addresses */ -static int NoFork = 0; /* don't fork - don't run in daemon mode - read-only after startup */ +static int doFork = 1; /* fork - run in daemon mode - read-only after startup */ int bHaveMainQueue = 0;/* set to 1 if the main queue - in queueing mode - is available * If the main queue is either not yet ready or not running in * queueing mode (mode DIRECT!), then this is set to 0. @@ -473,7 +473,7 @@ logmsgInternal(int iErr, int pri, uchar *msg, int flags) * permits us to process unmodified config files which otherwise contain a * supressor statement. */ - if(((Debug == DEBUG_FULL || NoFork) && ourConf->globals.bErrMsgToStderr) || iConfigVerify) { + if(((Debug == DEBUG_FULL || !doFork) && ourConf->globals.bErrMsgToStderr) || iConfigVerify) { if(LOG_PRI(pri) == LOG_ERR) fprintf(stderr, "rsyslogd: %s\n", msg); } @@ -652,7 +652,6 @@ finalize_it: rsRetVal multiSubmitMsg(multi_submit_t *pMultiSub) { - int i; qqueue_t *pQueue; ruleset_t *pRuleset; DEFiRet; @@ -1693,8 +1692,7 @@ doGlblProcessInit(void) thrdInit(); - if( !(Debug == DEBUG_FULL || NoFork) ) - { + if(doFork) { DBGPRINTF("Checking pidfile '%s'.\n", PidFile); if (!check_pid(PidFile)) { @@ -1706,16 +1704,23 @@ doGlblProcessInit(void) /* stop writing debug messages to stdout (if debugging is on) */ stddbg = -1; + dbgprintf("ready for forking\n"); if (fork()) { /* Parent process */ - sleep(300); - /* Not reached unless something major went wrong. 5 - * minutes should be a fair amount of time to wait. - * Please note that this procedure is important since - * the father must not exit before syslogd isn't - * initialized or the klogd won't be able to flush its - * logs. -Joey + dbgprintf("parent process going to sleep for 60 secs\n"); + sleep(60); + /* Not reached unless something major went wrong. 1 + * minute should be a fair amount of time to wait. + * The parent should not exit before rsyslogd is + * properly initilized (at least almost) or the init + * system may get a wrong impression of our readyness. + * Note that we exit before being completely initialized, + * but at this point it is very, very unlikely that something + * bad can happen. We do this here, because otherwise we would + * need to have much more code to handle priv drop (which we + * don't consider worth for the init system, especially as it + * is going away on the majority of distros). */ exit(1); /* "good" exit - after forking, not diasabling anything */ } @@ -1724,6 +1729,7 @@ doGlblProcessInit(void) close(0); /* we keep stdout and stderr open in case we have to emit something */ i = 3; + dbgprintf("in child, finalizing initialization\n"); /* if (sd_booted()) */ { const char *e; @@ -1757,7 +1763,8 @@ doGlblProcessInit(void) i = SD_LISTEN_FDS_START + sd_fds; } for ( ; i < num_fds; i++) - (void) close(i); + if(i != dbgGetDbglogFd()) + close(i); untty(); } else { @@ -1968,7 +1975,7 @@ int realMain(int argc, char **argv) fprintf(stderr, "rsyslogd: error -m is no longer supported - use immark instead"); break; case 'n': /* don't fork */ - NoFork = 1; + doFork = 0; break; case 'N': /* enable config verify mode */ iConfigVerify = atoi(arg); @@ -2066,17 +2073,18 @@ int realMain(int argc, char **argv) if(!iConfigVerify) CHKiRet(doGlblProcessInit()); + /* Send a signal to the parent so it can terminate. */ + if(myPid != ppid) { + dbgprintf("signaling parent to terminate\n"); + kill(ppid, SIGTERM); + } + CHKiRet(init()); if(Debug && debugging_on) { dbgprintf("Debugging enabled, SIGUSR1 to turn off debugging.\n"); } - /* Send a signal to the parent so it can terminate. */ - if(myPid != ppid) - kill(ppid, SIGTERM); - - /* END OF INTIALIZATION */ DBGPRINTF("initialization completed, transitioning to regular run mode\n"); @@ -2087,7 +2095,7 @@ int realMain(int argc, char **argv) * is still in its infancy (and not really done), we currently accept this issue. * rgerhards, 2009-06-29 */ - if(!(Debug == DEBUG_FULL || NoFork)) { + if(!doFork) { close(1); close(2); ourConf->globals.bErrMsgToStderr = 0; -- cgit v1.2.3 From 07ddb712ba562480a5492e1c8733b28e3a766d1a Mon Sep 17 00:00:00 2001 From: Milan Bartos Date: Thu, 14 Mar 2013 11:52:21 +0100 Subject: Fix trailing '-' in kernel messages. modified: plugins/imkmsg/kmsg.c --- plugins/imkmsg/kmsg.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/imkmsg/kmsg.c b/plugins/imkmsg/kmsg.c index b771d68a..c531f941 100644 --- a/plugins/imkmsg/kmsg.c +++ b/plugins/imkmsg/kmsg.c @@ -89,6 +89,10 @@ submitSyslog(uchar *buf) for (; isdigit(*buf); buf++) { timestamp += (timestamp * 10) + (*buf - '0'); } + + while (*buf != ';') { + buf++; /* skip everything till the first ; */ + } buf++; /* skip ; */ /* get message */ -- cgit v1.2.3 From 68fe67afc88a105575e6be256523a67c40ccff39 Mon Sep 17 00:00:00 2001 From: Milan Bartos Date: Mon, 18 Mar 2013 09:06:33 +0100 Subject: Remove unnecessary strlen() call modified: plugins/imkmsg/kmsg.c --- plugins/imkmsg/kmsg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/imkmsg/kmsg.c b/plugins/imkmsg/kmsg.c index c531f941..bc91f989 100644 --- a/plugins/imkmsg/kmsg.c +++ b/plugins/imkmsg/kmsg.c @@ -107,7 +107,7 @@ submitSyslog(uchar *buf) if (*buf != '\0') /* message has appended properties, skip \n */ buf++; - while (strlen((char *)buf)) { + while (*buf) { /* get name of the property */ buf++; /* skip ' ' */ offs = 0; -- cgit v1.2.3 From 3c0c1aa425c8f1bbaa61cf93724cae8e0bd72fee Mon Sep 17 00:00:00 2001 From: Milan Bartos Date: Mon, 18 Mar 2013 09:15:32 +0100 Subject: Fix message read buffer size modified: plugins/imkmsg/kmsg.c --- plugins/imkmsg/kmsg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/imkmsg/kmsg.c b/plugins/imkmsg/kmsg.c index bc91f989..0117b6ba 100644 --- a/plugins/imkmsg/kmsg.c +++ b/plugins/imkmsg/kmsg.c @@ -178,14 +178,14 @@ static void readkmsg(void) { int i; - uchar pRcv[8096+1]; + uchar pRcv[8192+1]; char errmsg[2048]; for (;;) { dbgprintf("imkmsg waiting for kernel log line\n"); /* every read() from the opened device node receives one record of the printk buffer */ - i = read(fklog, pRcv, 8096); + i = read(fklog, pRcv, 8192); if (i > 0) { /* successful read of message of nonzero length */ -- cgit v1.2.3 From bc740ffc8009095dd6cec3e3629fcd2283906fcd Mon Sep 17 00:00:00 2001 From: Milan Bartos Date: Mon, 18 Mar 2013 11:12:46 +0100 Subject: Do not exit when messages get overwritten in the circular buffer modified: plugins/imkmsg/kmsg.c --- plugins/imkmsg/kmsg.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/imkmsg/kmsg.c b/plugins/imkmsg/kmsg.c index 0117b6ba..f1815f25 100644 --- a/plugins/imkmsg/kmsg.c +++ b/plugins/imkmsg/kmsg.c @@ -190,6 +190,10 @@ readkmsg(void) if (i > 0) { /* successful read of message of nonzero length */ pRcv[i] = '\0'; + } else if (i == -EPIPE) { + imkmsgLogIntMsg(LOG_WARNING, + "imkmsg: some messages in circular buffer got overwritten"); + continue; } else { /* something went wrong - error or zero length message */ if (i < 0 && errno != EINTR && errno != EAGAIN) { -- cgit v1.2.3 From ff261830c65f7d094584dba1f9bce669bd6748d4 Mon Sep 17 00:00:00 2001 From: Rainer Gerhards Date: Mon, 18 Mar 2013 12:16:28 +0100 Subject: doc: mention last patches in ChangeLog --- ChangeLog | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index b143ae81..9acbbf27 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,11 +1,5 @@ ---------------------------------------------------------------------------- Version 7.2.7 [v7-stable] 2013-03-?? -- bugfix: imuxsock was missing SysSock.ParseTrusted module parameter - To use that functionality, legacy rsyslog.conf syntax had to be used. - Also, the doc was missing information on the "ParseTrusted" set of - config directives. -- doc bugfix: rsyslog.conf man page had invalid file format info - closes: http://bugzilla.adiscon.com/show_bug.cgi?id=418 - rsyslogd startup information is now properly conveyed back to init when privileges are beging dropped Actually, we have moved termination of the parent in front of the @@ -16,6 +10,14 @@ Version 7.2.7 [v7-stable] 2013-03-?? - improved debugging support in forked (auto-backgrounding) mode The rsyslog debug log file is now continued to be written across the fork. +- bugfix: several issues in imkmsg + see bug tracker: http://bugzilla.adiscon.com/show_bug.cgi?id=421#c8 +- bugfix: imuxsock was missing SysSock.ParseTrusted module parameter + To use that functionality, legacy rsyslog.conf syntax had to be used. + Also, the doc was missing information on the "ParseTrusted" set of + config directives. +- doc bugfix: rsyslog.conf man page had invalid file format info + closes: http://bugzilla.adiscon.com/show_bug.cgi?id=418 ---------------------------------------------------------------------------- Version 7.2.6 [v7-stable] 2013-03-05 - slightly improved config parser error messages when invalid escapes happen -- cgit v1.2.3