From: dlg <dlg@openbsd.org>
Date: Thu, 9 Nov 2023 18:36:19 +0000 (+0000)
Subject: avoid restartable syscalls with siginterrupt() against all our handlers.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=79e44c4617ae39ac54daa85ea20638065d3afbd9;p=openbsd

avoid restartable syscalls with siginterrupt() against all our handlers.

pflogd uses blocking bpf reads, but installs a bunch of signal
handlers to handle cleanly closing and (re)opening the log file.
signal() by default sets the handlers up so they're restartable.
this has the effect that when pflogd receives a signal while waiting
in bpfread, the signal handler runs and sets a flag saying the file
should be rotated or closed or whatever, but then the kernel restarts
the read.

when pflogd used a bpf read timeout, pflogd would run it's "event"
loop every time the read timeout expired. this meant even though
the bpf read was restarted, by having the read timeout expire every
500ms the flag the signal handlers set would be processed in a
relatively short period of time.

after moving bpf to a wait timeout, pflogd basically has to wait
for a packet to be captured before the bpf read will return. if
you're trying to kill pflogd on an idle firewall then you're basically
stuck.

making the signal handlers not restartable allows bpfread to fail
with EINTR so pflogd can go around it's even loop and exit as
expected.

reported by Mikhail on bugs@
ok claudio@

pflogd needs a rewrite though.
---

diff --git a/sbin/pflogd/pflogd.c b/sbin/pflogd/pflogd.c
index 6a7a3cbe915..5b0740c84d5 100644
--- a/sbin/pflogd/pflogd.c
+++ b/sbin/pflogd/pflogd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pflogd.c,v 1.63 2023/05/09 00:01:59 dlg Exp $	*/
+/*	$OpenBSD: pflogd.c,v 1.64 2023/11/09 18:36:19 dlg Exp $	*/
 
 /*
  * Copyright (c) 2001 Theo de Raadt
@@ -160,18 +160,21 @@ usage(void)
 void
 sig_close(int sig)
 {
+	pcap_breakloop(hpcap);
 	gotsig_close = 1;
 }
 
 void
 sig_hup(int sig)
 {
+	pcap_breakloop(hpcap);
 	gotsig_hup = 1;
 }
 
 void
 sig_alrm(int sig)
 {
+	pcap_breakloop(hpcap);
 	gotsig_alrm = 1;
 }
 
@@ -685,10 +688,15 @@ main(int argc, char **argv)
 	setproctitle("[initializing]");
 	/* Process is now unprivileged and inside a chroot */
 	signal(SIGTERM, sig_close);
+	siginterrupt(SIGTERM, 1);
 	signal(SIGINT, sig_close);
+	siginterrupt(SIGTERM, 1);
 	signal(SIGQUIT, sig_close);
+	siginterrupt(SIGTERM, 1);
 	signal(SIGALRM, sig_alrm);
+	siginterrupt(SIGTERM, 1);
 	signal(SIGHUP, sig_hup);
+	siginterrupt(SIGTERM, 1);
 	alarm(delay);
 
 	if (priv_init_pcap(snaplen))