>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     [email protected]:/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@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@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@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:
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"

Reply via email to