bin/184424: newsyslog - run shell command feature does not work (R flag in config file)

Klaas Teschauer ktsr42 at fastmail.fm
Mon Dec 2 03:40:01 UTC 2013


>Number:         184424
>Category:       bin
>Synopsis:       newsyslog - run shell command feature does not work (R flag in config file)
>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:   Mon Dec 02 03:40:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Klaas Teschauer
>Release:        9.1-RELEASE
>Organization:
>Environment:
FreeBSD hub 9.1-RELEASE FreeBSD 9.1-RELEASE #0 r243825: Tue Dec  4 09:23:10 UTC 2012     root at farrell.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC  amd64
>Description:
According to newsyslog.conf(5), newsyslog offers a feature to execute an external command to signal a daemon after a log has been rotated.

Quoting man newsyslog.conf, section "flags":

             R       if this flag is set the newsyslog(8) will run shell com-
                     mand defined in path_to_pid_cmd_file after rotation
                     instead of trying to send signal to a process id stored
                     in the file.

In FreeBSD 9.1, this feature does not work.
>How-To-Repeat:
Set up a newsyslog.conf file with an entry that uses the 'R' flag:

[klaas at hub ~]$ cat newsyslog.netatalk.test 
# test config for newsyslog(8)

# logfilename          [owner:group]    mode count size when  flags [/pid_file] [sig_num]
/var/log/netatalk.log  root:wheel       644     6  50   @T00  R    /usr/home/klaas/restart.netatalk

Perform a test run to confirm that the script is not invoked (due to the bug):

[klaas at hub ~]$ newsyslog -f newsyslog.netatalk.test -r -n -v -F
Processing newsyslog.netatalk.test
/var/log/netatalk.log <6>: size (Kb): 5 [50] --> trimming log....
        rm -f /var/log/netatalk.log.6
        rm -f /var/log/netatalk.log.6.gz
        rm -f /var/log/netatalk.log.6.bz2
        rm -f /var/log/netatalk.log.6.xz
        rm -f /var/log/netatalk.log.5
        rm -f /var/log/netatalk.log.5.gz
        rm -f /var/log/netatalk.log.5.bz2
        rm -f /var/log/netatalk.log.5.xz
        mv /var/log/netatalk.log.2.gz /var/log/netatalk.log.3.gz
        chmod 644 /var/log/netatalk.log.3.gz
        chown 0:0 /var/log/netatalk.log.3.gz
        mv /var/log/netatalk.log.1.gz /var/log/netatalk.log.2.gz
        chmod 644 /var/log/netatalk.log.2.gz
        chown 0:0 /var/log/netatalk.log.2.gz
        mv /var/log/netatalk.log.0 /var/log/netatalk.log.1
        chmod 644 /var/log/netatalk.log.1
        chown 0:0 /var/log/netatalk.log.1
        ln /var/log/netatalk.log /var/log/netatalk.log.0
        chmod 644 /var/log/netatalk.log.0
        chown 0:0 /var/log/netatalk.log.0
Start new log...
        mktemp /var/log/netatalk.log.zXXXXXX
        chown 0:0 /var/log/netatalk.log.zXXXXXX
        chmod 644 /var/log/netatalk.log.zXXXXXX
        mv /var/log/netatalk.log.zXXXXXX /var/log/netatalk.log
Signal all daemon process(es)...
        sleep 10


>Fix:
A patch is attached to this report. Here is the output from newsyslog with it:

[klaas at hub ~]$ ./hacks/newsyslog/newsyslog -f newsyslog.netatalk.test -r -n -v -F
Processing newsyslog.netatalk.test
/var/log/netatalk.log <6>: size (Kb): 5 [50] --> trimming log....
        rm -f /var/log/netatalk.log.6
        rm -f /var/log/netatalk.log.6.gz
        rm -f /var/log/netatalk.log.6.bz2
        rm -f /var/log/netatalk.log.6.xz
        rm -f /var/log/netatalk.log.5
        rm -f /var/log/netatalk.log.5.gz
        rm -f /var/log/netatalk.log.5.bz2
        rm -f /var/log/netatalk.log.5.xz
        mv /var/log/netatalk.log.2.gz /var/log/netatalk.log.3.gz
        chmod 644 /var/log/netatalk.log.3.gz
        chown 0:0 /var/log/netatalk.log.3.gz
        mv /var/log/netatalk.log.1.gz /var/log/netatalk.log.2.gz
        chmod 644 /var/log/netatalk.log.2.gz
        chown 0:0 /var/log/netatalk.log.2.gz
        mv /var/log/netatalk.log.0 /var/log/netatalk.log.1
        chmod 644 /var/log/netatalk.log.1
        chown 0:0 /var/log/netatalk.log.1
        ln /var/log/netatalk.log /var/log/netatalk.log.0
        chmod 644 /var/log/netatalk.log.0
        chown 0:0 /var/log/netatalk.log.0
Start new log...
        mktemp /var/log/netatalk.log.zXXXXXX
        chown 0:0 /var/log/netatalk.log.zXXXXXX
        chmod 644 /var/log/netatalk.log.zXXXXXX
        mv /var/log/netatalk.log.zXXXXXX /var/log/netatalk.log
Signal all daemon process(es)...
do_sigwork: /usr/home/klaas/restart.netatalk, run_cmd 1, pidok 0, pid 0
noaction: 1
Run command: /usr/home/klaas/restart.netatalk 1
        sleep 10

Warning: This patch has not been tested in any way. I only confirmed that things seem to happen as expected when running newsyslog with -n and -v.

Other notes:

1) I am not sure why the original implementation of do_sigwork() passes the signal number to the external program. It seems to he redundant because I would expect the dedicated script to know what to do. 

2) I am no security expert, but in my mind the system() call from stdlib is a security risk. system() passes the string to the sh command interpreter and thus is open to manipulation of the search path. I would recommend that newsyslog uses fork/exec for calling external programs, including kill(1).

Patch attached with submission follows:

--- /usr/src/usr.sbin/newsyslog/newsyslog.c	2012-12-03 16:22:39.000000000 -0500
+++ newsyslog.c	2013-12-01 00:48:48.607206990 -0500
@@ -1855,7 +1855,7 @@
 	int kres, secs;
 	char *tmp;
 
-	if (!(swork->sw_pidok) || swork->sw_pid == 0)
+	if (!(swork->run_cmd) && (!(swork->sw_pidok) || swork->sw_pid == 0) )
 		return;			/* no work to do... */
 
 	/*
@@ -1888,14 +1888,6 @@
 			secs = 1;
 	}
 
-	if (noaction) {
-		printf("\tkill -%d %d \t\t# %s\n", swork->sw_signum,
-		    (int)swork->sw_pid, swork->sw_fname);
-		if (secs > 0)
-			printf("\tsleep %d\n", secs);
-		return;
-	}
-
 	if (swork->run_cmd) {
 		asprintf(&tmp, "%s %d", swork->sw_fname, swork->sw_signum);
 		if (tmp == NULL) {
@@ -1905,15 +1897,25 @@
 		}
 		if (verbose)
 			printf("Run command: %s\n", tmp);
-		kres = system(tmp);
-		if (kres) {
-			warnx("%s: returned non-zero exit code: %d",
-			    tmp, kres);
+		if( !noaction ) {
+			kres = system(tmp);
+			if (kres) {
+				warnx("%s: returned non-zero exit code: %d",
+				    tmp, kres);
+			}
 		}
 		free(tmp);
 		return;
 	}
 
+	if (noaction) {
+		printf("\tkill -%d %d \t\t# %s\n", swork->sw_signum,
+		    (int)swork->sw_pid, swork->sw_fname);
+		if (secs > 0)
+			printf("\tsleep %d\n", secs);
+		return;
+	}
+
 	kres = kill(swork->sw_pid, swork->sw_signum);
 	if (kres != 0) {
 		/*


>Release-Note:
>Audit-Trail:
>Unformatted:


More information about the freebsd-bugs mailing list