Re: don't try to signal with newsyslog -r
On Feb 10 14:53:33, mill...@openbsd.org wrote: > On Mon, 10 Feb 2020 17:12:53 +0100, Jan Stary wrote: > > > The -r option of newsyslog(8) removes the requirement > > that newsyslog runs as root. Would it also make sense > > to not try to send the SIGHUP to syslogd in that case? > > This seems wrong to me. You are disabling more than just sending > a signal to SIGHUP, this will cause newsyslog to ignore *all* pid > files. Yes; it is wrong. A better way to avoid trying to send the signal seems to be to simply specify an empty command in the config file. Jan
Re: don't try to signal with newsyslog -r
Hi Ingo,
On Feb 10 22:40:20, schwa...@usta.de wrote:
> > The -r option of newsyslog(8) removes the requirement
> > that newsyslog runs as root. Would it also make sense
> > to not try to send the SIGHUP to syslogd in that case?
>
> While i'm not sure that i want to take care of this patch,
> given that i'm not quite sure what the point of the -r option
> even is in the first place,
for example, I use it to rotate ~/battery/battery.log on my laptop
upon boot, obtaining a concise history of the battery's life.
Without -r, newsyslog would refuse to run. It is my impression
that this is the inteded use: people rotating their own logs,
not just system logs. I don't know why newsyslog refuses to run
without root, creating a "need" for -r in the first place,
> i'd like to point out that you
> are also removing the warning. Is that intentional?
Yes.
> Naively, getting a warning when files are rotated but the
> daemon isn't notified seems useful to me, even if you kind
> of requested it with -r.
>
> In that case, wouldn't it make more sense to say something
> like (untested, and no code auditing done)
>
> else if (needroot == 0 || kill(pid, signal))
>
> ?
>
> That would also avoid the hopeless attempt to send a signal,
> but would still print the warning.
Sorry for being unclear.
The log I am rotating is not written by syslog; it is written
by my user's cronjob (basicaly saving the voltage and capacity
from hw.sensors.acpibat0 every cron minutes). Given that, and
the fact that I am not running newsyslog as root, I don't need
or expect to signal syslog; the warning then only informs me
about the hopeless attempt to send the signal.
Jan
> > Index: newsyslog.8
> > ===
> > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> > retrieving revision 1.54
> > diff -u -p -r1.54 newsyslog.8
> > --- newsyslog.8 20 Jul 2017 18:39:16 - 1.54
> > +++ newsyslog.8 10 Feb 2020 16:08:51 -
> > @@ -124,7 +124,7 @@ Removes the restriction that
> > must be running as root.
> > Note that in this mode
> > .Nm
> > -will not be able to send a
> > +will not try to send a
> > .Dv SIGHUP
> > signal to
> > .Xr syslogd 8 .
> > Index: newsyslog.c
> > ===
> > RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> > retrieving revision 1.112
> > diff -u -p -r1.112 newsyslog.c
> > --- newsyslog.c 28 Jun 2019 13:35:02 - 1.112
> > +++ newsyslog.c 10 Feb 2020 16:08:51 -
> > @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal)
> > warnx("%s pid file: %s", err, pidfile);
> > else if (noaction)
> > (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> > - else if (kill(pid, signal))
> > + else if (needroot && kill(pid, signal))
> > warnx("warning - could not send SIG%s to PID from pid file %s",
> > sys_signame[signal], pidfile);
> > }
>
>
Re: don't try to signal with newsyslog -r
On Mon, 10 Feb 2020 17:12:53 +0100, Jan Stary wrote: > The -r option of newsyslog(8) removes the requirement > that newsyslog runs as root. Would it also make sense > to not try to send the SIGHUP to syslogd in that case? This seems wrong to me. You are disabling more than just sending a signal to SIGHUP, this will cause newsyslog to ignore *all* pid files. A better approach would be to avoid initializing working->pidfile to PIDFILE if the euid is non-zero. That way, user-specified pid files are still honored. - todd
Re: don't try to signal with newsyslog -r
Hi,
Jan Stary wrote on Mon, Feb 10, 2020 at 05:12:53PM +0100:
> The -r option of newsyslog(8) removes the requirement
> that newsyslog runs as root. Would it also make sense
> to not try to send the SIGHUP to syslogd in that case?
While i'm not sure that i want to take care of this patch,
given that i'm not quite sure what the point of the -r option
even is in the first place, i'd like to point out that you
are also removing the warning. Is that intentional?
Naively, getting a warning when files are rotated but the
daemon isn't notified seems useful to me, even if you kind
of requested it with -r.
In that case, wouldn't it make more sense to say something
like (untested, and no code auditing done)
else if (needroot == 0 || kill(pid, signal))
?
That would also avoid the hopeless attempt to send a signal,
but would still print the warning.
Yours,
Ingo
> Index: newsyslog.8
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> retrieving revision 1.54
> diff -u -p -r1.54 newsyslog.8
> --- newsyslog.8 20 Jul 2017 18:39:16 - 1.54
> +++ newsyslog.8 10 Feb 2020 16:08:51 -
> @@ -124,7 +124,7 @@ Removes the restriction that
> must be running as root.
> Note that in this mode
> .Nm
> -will not be able to send a
> +will not try to send a
> .Dv SIGHUP
> signal to
> .Xr syslogd 8 .
> Index: newsyslog.c
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.112
> diff -u -p -r1.112 newsyslog.c
> --- newsyslog.c 28 Jun 2019 13:35:02 - 1.112
> +++ newsyslog.c 10 Feb 2020 16:08:51 -
> @@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal)
> warnx("%s pid file: %s", err, pidfile);
> else if (noaction)
> (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> - else if (kill(pid, signal))
> + else if (needroot && kill(pid, signal))
> warnx("warning - could not send SIG%s to PID from pid file %s",
> sys_signame[signal], pidfile);
> }
don't try to signal with newsyslog -r
The -r option of newsyslog(8) removes the requirement
that newsyslog runs as root. Would it also make sense
to not try to send the SIGHUP to syslogd in that case?
Jan
Index: newsyslog.8
===
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
retrieving revision 1.54
diff -u -p -r1.54 newsyslog.8
--- newsyslog.8 20 Jul 2017 18:39:16 - 1.54
+++ newsyslog.8 10 Feb 2020 16:08:51 -
@@ -124,7 +124,7 @@ Removes the restriction that
must be running as root.
Note that in this mode
.Nm
-will not be able to send a
+will not try to send a
.Dv SIGHUP
signal to
.Xr syslogd 8 .
Index: newsyslog.c
===
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
retrieving revision 1.112
diff -u -p -r1.112 newsyslog.c
--- newsyslog.c 28 Jun 2019 13:35:02 - 1.112
+++ newsyslog.c 10 Feb 2020 16:08:51 -
@@ -394,7 +394,7 @@ send_signal(char *pidfile, int signal)
warnx("%s pid file: %s", err, pidfile);
else if (noaction)
(void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
- else if (kill(pid, signal))
+ else if (needroot && kill(pid, signal))
warnx("warning - could not send SIG%s to PID from pid file %s",
sys_signame[signal], pidfile);
}
Re: newsyslog -r
ping
On Nov 12 22:21:39, h...@stare.cz wrote:
> The -r option of newsyslog(8) removes the requirement
> that newsyslog runs as root. Would it also make sense
> to not try to send the SIGHUP to syslogd in that case?
>
> Jan
>
>
> Index: newsyslog.8
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> retrieving revision 1.52
> diff -u -p -u -p -r1.52 newsyslog.8
> --- newsyslog.8 16 Sep 2014 16:27:23 - 1.52
> +++ newsyslog.8 12 Nov 2015 21:20:52 -
> @@ -123,7 +123,7 @@ Removes the restriction that
> must be running as root.
> Note that in this mode
> .Nm
> -will not be able to send a
> +will not try to send a
> .Dv SIGHUP
> signal to
> .Xr syslogd 8 .
> Index: newsyslog.c
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.95
> diff -u -p -u -p -r1.95 newsyslog.c
> --- newsyslog.c 20 Aug 2015 22:32:41 - 1.95
> +++ newsyslog.c 12 Nov 2015 21:20:52 -
> @@ -406,7 +406,7 @@ send_signal(char *pidfile, int signal)
> warnx("%s pid file: %s", err, pidfile);
> else if (noaction)
> (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> - else if (kill(pid, signal))
> + else if (needroot && kill(pid, signal))
> warnx("warning - could not send SIG%s to PID from pid file %s",
> sys_signame[signal], pidfile);
> }
Re: newsyslog -r
ping
On Nov 12 22:21:39, h...@stare.cz wrote:
> The -r option of newsyslog(8) removes the requirement
> that newsyslog runs as root. Would it also make sense
> to not try to send the SIGHUP to syslogd in that case?
>
> Jan
>
>
> Index: newsyslog.8
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
> retrieving revision 1.52
> diff -u -p -u -p -r1.52 newsyslog.8
> --- newsyslog.8 16 Sep 2014 16:27:23 - 1.52
> +++ newsyslog.8 12 Nov 2015 21:20:52 -
> @@ -123,7 +123,7 @@ Removes the restriction that
> must be running as root.
> Note that in this mode
> .Nm
> -will not be able to send a
> +will not try to send a
> .Dv SIGHUP
> signal to
> .Xr syslogd 8 .
> Index: newsyslog.c
> ===
> RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
> retrieving revision 1.95
> diff -u -p -u -p -r1.95 newsyslog.c
> --- newsyslog.c 20 Aug 2015 22:32:41 - 1.95
> +++ newsyslog.c 12 Nov 2015 21:20:52 -
> @@ -406,7 +406,7 @@ send_signal(char *pidfile, int signal)
> warnx("%s pid file: %s", err, pidfile);
> else if (noaction)
> (void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
> - else if (kill(pid, signal))
> + else if (needroot && kill(pid, signal))
> warnx("warning - could not send SIG%s to PID from pid file %s",
> sys_signame[signal], pidfile);
> }
newsyslog -r
The -r option of newsyslog(8) removes the requirement
that newsyslog runs as root. Would it also make sense
to not try to send the SIGHUP to syslogd in that case?
Jan
Index: newsyslog.8
===
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.8,v
retrieving revision 1.52
diff -u -p -u -p -r1.52 newsyslog.8
--- newsyslog.8 16 Sep 2014 16:27:23 - 1.52
+++ newsyslog.8 12 Nov 2015 21:20:52 -
@@ -123,7 +123,7 @@ Removes the restriction that
must be running as root.
Note that in this mode
.Nm
-will not be able to send a
+will not try to send a
.Dv SIGHUP
signal to
.Xr syslogd 8 .
Index: newsyslog.c
===
RCS file: /cvs/src/usr.bin/newsyslog/newsyslog.c,v
retrieving revision 1.95
diff -u -p -u -p -r1.95 newsyslog.c
--- newsyslog.c 20 Aug 2015 22:32:41 - 1.95
+++ newsyslog.c 12 Nov 2015 21:20:52 -
@@ -406,7 +406,7 @@ send_signal(char *pidfile, int signal)
warnx("%s pid file: %s", err, pidfile);
else if (noaction)
(void)printf("kill -%s %ld\n", sys_signame[signal], (long)pid);
- else if (kill(pid, signal))
+ else if (needroot && kill(pid, signal))
warnx("warning - could not send SIG%s to PID from pid file %s",
sys_signame[signal], pidfile);
}
