Date: Fri, 14 Dec 2012 17:15:32 GMT From: Mark Johnston <markjdb@gmail.com> To: freebsd-gnats-submit@FreeBSD.org Subject: bin/174438: [patch][newsyslog] interval-based rotation happens too frequently Message-ID: <201212141715.qBEHFWVm045481@red.freebsd.org> Resent-Message-ID: <201212141720.qBEHK0dU065132@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 174438
>Category: bin
>Synopsis: [patch][newsyslog] interval-based rotation happens too frequently
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Fri Dec 14 17:20:00 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator: Mark Johnston
>Release: 10-CURRENT
>Organization:
>Environment:
FreeBSD oddish 10.0-CURRENT FreeBSD 10.0-CURRENT #41 r+e6f8ae9-dirty: Fri Dec 14 09:23:53 EST 2012 mark@oddish:/usr/obj/usr/home/mark/src/freebsd/sys/GENERIC amd64
>Description:
newsyslog can be configured to archive log files after a certain period of time has elapsed. This is done by just specifying the length of the interval (in hours) in the "when" column in newsyslog.conf.
When newsyslog runs, it compares the mtime of the most recent archive log (e.g. sendmail.st.0) with the current time, and performs a rotation if the difference is longer than the configured interval. However, when a rotation is performed, the last-modified timestamps on the archives aren't updated, so the next rotation may happen sooner than it's supposed to.
The solution is to update the mtime of the newest archived log file after a rotation. Since that mtime is used to determine when the next rotation should occur, this ensures that the correct amount of time elapses between rotations.
This bug will only affect binary log files that use interval-based rotations. sendmail.st is the only log file affected in the default config.
>How-To-Repeat:
The timestamps on the archive copies of sendmail.st indicate the problem. That log is only supposed to be rotated once a week (every 168 hours):
$ ls -la /var/log/sendmail.st*
-rw-r----- 1 root wheel 0 Dec 11 02:00 /var/log/sendmail.st
-rw-r----- 1 root wheel 0 Dec 11 01:00 /var/log/sendmail.st.0
-rw-r----- 1 root wheel 0 Dec 4 00:00 /var/log/sendmail.st.1
-rw-r----- 1 root wheel 0 Dec 3 23:00 /var/log/sendmail.st.2
-rw-r----- 1 root wheel 0 Nov 27 00:00 /var/log/sendmail.st.3
-rw-r----- 1 root wheel 0 Nov 26 23:00 /var/log/sendmail.st.4
-rw-r----- 1 root wheel 0 Nov 20 00:00 /var/log/sendmail.st.5
-rw-r----- 1 root wheel 0 Nov 19 23:00 /var/log/sendmail.st.6
-rw-r----- 1 root wheel 0 Nov 13 00:00 /var/log/sendmail.st.7
-rw-r----- 1 root wheel 0 Nov 12 23:00 /var/log/sendmail.st.8
-rw-r----- 1 root wheel 0 Nov 6 00:00 /var/log/sendmail.st.9
>Fix:
Apply the attached patch. This patch also fixes a typo in an error message and adds a check for the return value of rename(2). Specifically, if rename(2) fails, we should exit, since otherwise some archived log files will probably be overwritten.
Patch attached with submission follows:
diff --git a/usr.sbin/newsyslog/newsyslog.c b/usr.sbin/newsyslog/newsyslog.c
index 875f911..5f946ae 100644
--- a/usr.sbin/newsyslog/newsyslog.c
+++ b/usr.sbin/newsyslog/newsyslog.c
@@ -79,6 +79,7 @@ __FBSDID("$FreeBSD$");
#include <libgen.h>
#include <stdlib.h>
#include <string.h>
+#include <sysexits.h>
#include <time.h>
#include <unistd.h>
@@ -1582,7 +1583,7 @@ delete_oldest_timelog(const struct conf_entry *ent, const char *archive_dir)
oldlogs[i].fname);
else if (unlinkat(dir_fd, oldlogs[i].fname, 0) != 0) {
snprintf(errbuf, sizeof(errbuf),
- "Could not delet old logfile '%s'",
+ "Could not delete old logfile '%s'",
oldlogs[i].fname);
perror(errbuf);
}
@@ -1796,8 +1797,9 @@ do_rotate(const struct conf_entry *ent)
if (noaction)
printf("\tmv %s %s\n", zfile1, zfile2);
else {
- /* XXX - Ought to be checking for failure! */
- (void)rename(zfile1, zfile2);
+ if (rename(zfile1, zfile2))
+ err(EX_OSERR, "failed renaming %s to %s",
+ zfile1, zfile2);
}
change_attrs(zfile2, ent);
}
@@ -1820,6 +1822,12 @@ do_rotate(const struct conf_entry *ent)
log_trim(ent->log, ent);
}
savelog(ent->log, file1);
+ /*
+ * Interval-based rotations are done using the mtime of
+ * the most recently archived log, so make sure it gets
+ * updated during a rotation.
+ */
+ utimes(file1, NULL);
}
change_attrs(file1, ent);
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201212141715.qBEHFWVm045481>