Date: Mon, 2 Dec 2013 03:31:49 GMT From: Klaas Teschauer <ktsr42@fastmail.fm> To: freebsd-gnats-submit@FreeBSD.org Subject: bin/184424: newsyslog - run shell command feature does not work (R flag in config file) Message-ID: <201312020331.rB23VnTU050860@oldred.freebsd.org> Resent-Message-ID: <201312020340.rB23e0Lk087164@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>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@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@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:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201312020331.rB23VnTU050860>