syslogd block signals

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

syslogd block signals

Alexander Bluhm
Hi,

While I was testing syslogd with SIGHUP, it sometimes died.  This
seems to happen if the signal hits the process durig its initilisation
phase.  I think we should block the signals until the handler in
both processes are installed.

ok?

bluhm

Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.64
diff -u -p -r1.64 privsep.c
--- usr.sbin/syslogd/privsep.c 16 Oct 2016 22:12:50 -0000 1.64
+++ usr.sbin/syslogd/privsep.c 26 Dec 2016 23:12:18 -0000
@@ -167,6 +167,7 @@ priv_exec(char *conf, int numeric, int c
  struct stat cf_info, cf_stat;
  struct addrinfo hints, *res0;
  struct sigaction sa;
+ sigset_t sigmask;
 
  if (pledge("stdio rpath wpath cpath dns getpw sendfd id proc exec",
     NULL) == -1)
@@ -200,6 +201,10 @@ priv_exec(char *conf, int numeric, int c
 
  setproctitle("[priv]");
  logdebug("[priv]: fork+exec done\n");
+
+ sigemptyset(&sigmask);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask priv");
 
  if (stat(conf, &cf_info) < 0)
  err(1, "stat config file failed");
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.224
diff -u -p -r1.224 syslogd.c
--- usr.sbin/syslogd/syslogd.c 23 Dec 2016 23:01:48 -0000 1.224
+++ usr.sbin/syslogd/syslogd.c 26 Dec 2016 23:12:20 -0000
@@ -354,6 +354,7 @@ main(int argc, char *argv[])
  struct event *ev_klog, *ev_sendsys, *ev_udp, *ev_udp6,
  *ev_bind, *ev_listen, *ev_tls, *ev_unix,
  *ev_hup, *ev_int, *ev_quit, *ev_term, *ev_mark;
+ sigset_t sigmask;
  const char *errstr;
  char *p;
  int ch, i;
@@ -361,6 +362,16 @@ main(int argc, char *argv[])
  int fd_ctlsock, fd_klog, fd_sendsys, fd_bind, fd_listen;
  int *fd_unix;
 
+ /* block signals during initialization */
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGTERM);
+ sigaddset(&sigmask, SIGHUP);
+ sigaddset(&sigmask, SIGINT);
+ sigaddset(&sigmask, SIGQUIT);
+ sigaddset(&sigmask, SIGCHLD);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask block");
+
  if ((path_unix = malloc(sizeof(*path_unix))) == NULL)
  err(1, "malloc %s", _PATH_LOG);
  path_unix[0] = _PATH_LOG;
@@ -841,6 +852,10 @@ main(int argc, char *argv[])
 
  logmsg(LOG_SYSLOG|LOG_INFO, "syslogd: start", LocalHostName, ADDDATE);
  logdebug("syslogd: started\n");
+
+ sigemptyset(&sigmask);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask unblock");
 
  event_dispatch();
  /* NOTREACHED */

Reply | Threaded
Open this post in threaded view
|

Re: syslogd block signals

Alexander Bluhm
On Mon, Dec 26, 2016 at 04:54:54PM -0700, Theo de Raadt wrote:
> I think programs should only block the absolutely critical things, and this
> is overreach.

Yes, blocking SIGINT and SIGQUIT is not clever.  I thought there
were races with SIGCHLD and SIGTERM where only one process would
survive.  But races don't exist as parent and child communicate
over the socketpair and terminate if the other one dies.

I have seen problems with SIGHUP during debugging, instead of
restarting syslogd died.  I think this could also happen during
normal operation.

- edit syslog.conf
- send SIGHUP
- syslogd execs itself
- newsyslog rotates logfile and sends SIGHUP
- syslogd dies as signal handlers are not installed yet

So I suggest to block SIGHUP during startup.

bluhm

Index: usr.sbin/syslogd/privsep.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
retrieving revision 1.65
diff -u -p -r1.65 privsep.c
--- usr.sbin/syslogd/privsep.c 27 Dec 2016 19:16:24 -0000 1.65
+++ usr.sbin/syslogd/privsep.c 30 Dec 2016 18:24:00 -0000
@@ -175,6 +175,7 @@ priv_exec(char *conf, int numeric, int c
  struct stat cf_info, cf_stat;
  struct addrinfo hints, *res0;
  struct sigaction sa;
+ sigset_t sigmask;
 
  if (pledge("stdio rpath wpath cpath dns getpw sendfd id proc exec",
     NULL) == -1)
@@ -209,6 +210,10 @@ priv_exec(char *conf, int numeric, int c
  setproctitle("[priv]");
  logdebug("[priv]: fork+exec done\n");
 
+ sigemptyset(&sigmask);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask priv");
+
  if (stat(conf, &cf_info) < 0)
  err(1, "stat config file failed");
 
@@ -409,6 +414,10 @@ priv_exec(char *conf, int numeric, int c
  int status;
 
  waitpid(child_pid, &status, 0);
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGHUP);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask exec");
  execvp(argv[0], argv);
  err(1, "exec restart '%s' failed", argv[0]);
  }
Index: usr.sbin/syslogd/syslogd.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
retrieving revision 1.225
diff -u -p -r1.225 syslogd.c
--- usr.sbin/syslogd/syslogd.c 27 Dec 2016 19:16:24 -0000 1.225
+++ usr.sbin/syslogd/syslogd.c 30 Dec 2016 17:55:37 -0000
@@ -354,6 +354,7 @@ main(int argc, char *argv[])
  struct event *ev_klog, *ev_sendsys, *ev_udp, *ev_udp6,
  *ev_bind, *ev_listen, *ev_tls, *ev_unix,
  *ev_hup, *ev_int, *ev_quit, *ev_term, *ev_mark;
+ sigset_t sigmask;
  const char *errstr;
  char *p;
  int ch, i;
@@ -361,6 +362,12 @@ main(int argc, char *argv[])
  int fd_ctlsock, fd_klog, fd_sendsys, fd_bind, fd_listen;
  int *fd_unix;
 
+ /* block signal until handler is set up */
+ sigemptyset(&sigmask);
+ sigaddset(&sigmask, SIGHUP);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask block");
+
  if ((path_unix = malloc(sizeof(*path_unix))) == NULL)
  err(1, "malloc %s", _PATH_LOG);
  path_unix[0] = _PATH_LOG;
@@ -839,6 +846,10 @@ main(int argc, char *argv[])
 
  logmsg(LOG_SYSLOG|LOG_INFO, "syslogd: start", LocalHostName, ADDDATE);
  logdebug("syslogd: started\n");
+
+ sigemptyset(&sigmask);
+ if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
+ err(1, "sigprocmask unblock");
 
  event_dispatch();
  /* NOTREACHED */


Reply | Threaded
Open this post in threaded view
|

Re: syslogd block signals

Theo de Raadt-2
Yes, that's much better.  It solves the problem, without trying to
solve other problems which don't exist.

> On Mon, Dec 26, 2016 at 04:54:54PM -0700, Theo de Raadt wrote:
> > I think programs should only block the absolutely critical things, and this
> > is overreach.
>
> Yes, blocking SIGINT and SIGQUIT is not clever.  I thought there
> were races with SIGCHLD and SIGTERM where only one process would
> survive.  But races don't exist as parent and child communicate
> over the socketpair and terminate if the other one dies.
>
> I have seen problems with SIGHUP during debugging, instead of
> restarting syslogd died.  I think this could also happen during
> normal operation.
>
> - edit syslog.conf
> - send SIGHUP
> - syslogd execs itself
> - newsyslog rotates logfile and sends SIGHUP
> - syslogd dies as signal handlers are not installed yet
>
> So I suggest to block SIGHUP during startup.
>
> bluhm
>
> Index: usr.sbin/syslogd/privsep.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/privsep.c,v
> retrieving revision 1.65
> diff -u -p -r1.65 privsep.c
> --- usr.sbin/syslogd/privsep.c 27 Dec 2016 19:16:24 -0000 1.65
> +++ usr.sbin/syslogd/privsep.c 30 Dec 2016 18:24:00 -0000
> @@ -175,6 +175,7 @@ priv_exec(char *conf, int numeric, int c
>   struct stat cf_info, cf_stat;
>   struct addrinfo hints, *res0;
>   struct sigaction sa;
> + sigset_t sigmask;
>  
>   if (pledge("stdio rpath wpath cpath dns getpw sendfd id proc exec",
>      NULL) == -1)
> @@ -209,6 +210,10 @@ priv_exec(char *conf, int numeric, int c
>   setproctitle("[priv]");
>   logdebug("[priv]: fork+exec done\n");
>  
> + sigemptyset(&sigmask);
> + if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
> + err(1, "sigprocmask priv");
> +
>   if (stat(conf, &cf_info) < 0)
>   err(1, "stat config file failed");
>  
> @@ -409,6 +414,10 @@ priv_exec(char *conf, int numeric, int c
>   int status;
>  
>   waitpid(child_pid, &status, 0);
> + sigemptyset(&sigmask);
> + sigaddset(&sigmask, SIGHUP);
> + if (sigprocmask(SIG_SETMASK, &sigmask, NULL) == -1)
> + err(1, "sigprocmask exec");
>   execvp(argv[0], argv);
>   err(1, "exec restart '%s' failed", argv[0]);
>   }
> Index: usr.sbin/syslogd/syslogd.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.225
> diff -u -p -r1.225 syslogd.c
> --- usr.sbin/syslogd/syslogd.c 27 Dec 2016 19:16:24 -0000 1.225
> +++ usr.sbin/syslogd/syslogd.c 30 Dec 2016 17:55:37 -0000
> @@ -354,6 +354,7 @@ main(int argc, char *argv[])
>   struct event *ev_klog, *ev_sendsys, *ev_udp, *ev_udp6,
>   *ev_bind, *ev_listen, *ev_tls, *ev_unix,
>   *ev_hup, *ev_int, *ev_quit, *ev_term, *ev_mark;
> + sigset_t sigmask;
>   const char *errstr;

Reply | Threaded
Open this post in threaded view
|

Re: syslogd block signals

Jeremie Courreges-Anglas-2
In reply to this post by Alexander Bluhm
Alexander Bluhm <[hidden email]> writes:

> On Mon, Dec 26, 2016 at 04:54:54PM -0700, Theo de Raadt wrote:
>> I think programs should only block the absolutely critical things, and this
>> is overreach.
>
> Yes, blocking SIGINT and SIGQUIT is not clever.  I thought there
> were races with SIGCHLD and SIGTERM where only one process would
> survive.  But races don't exist as parent and child communicate
> over the socketpair and terminate if the other one dies.
>
> I have seen problems with SIGHUP during debugging, instead of
> restarting syslogd died.  I think this could also happen during
> normal operation.
>
> - edit syslog.conf
> - send SIGHUP
> - syslogd execs itself
> - newsyslog rotates logfile and sends SIGHUP
> - syslogd dies as signal handlers are not installed yet
>
> So I suggest to block SIGHUP during startup.

ok jca@

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE