bin/174438: [patch][newsyslog] interval-based rotation happens too frequently

[Mark Johnston]

>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 at 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: