[git commit] tcp/udpsvd: robustify SIGCHLD handling

[Denys Vlasenko]

commit: https://git.busybox.net/busybox/commit/?id=d3e1090308b6d3c55e01a2000a743b73605ddd7f
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

function                                             old     new   delta
if_verbose_print_connection_status                     -      40     +40
tcpudpsvd_main                                      1798    1794      -4
connection_status                                     31       -     -31
------------------------------------------------------------------------------
(add/remove: 1/1 grow/shrink: 0/1 up/down: 40/-35)              Total: 5 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 networking/tcpudp.c | 33 +++++++++++++++++++++++----------
 runit/runsv.c       |  5 ++++-
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/networking/tcpudp.c b/networking/tcpudp.c
index 8c4afabf6..708e05c2e 100644
--- a/networking/tcpudp.c
+++ b/networking/tcpudp.c
@@ -216,17 +216,25 @@ enum {
 	OPT_K = (1 << 16),
 };
 
-static void connection_status(void)
+static void if_verbose_print_connection_status(void)
 {
-	/* "only 1 client max" desn't need this */
-	if (cmax > 1)
-		bb_error_msg("status %u/%u", cnum, cmax);
+	if (verbose) {
+		/* "only 1 client max" desn't need this */
+		if (cmax > 1)
+			bb_error_msg("status %u/%u", cnum, cmax);
+	}
 }
 
+/* SIGCHLD handler is reentrancy-safe because SIGCHLD is unmasked
+ * only over accept() or recvfrom() calls, not over memory allocations
+ * or printouts. Do need to save/restore errno in order not to mangle
+ * these syscalls' error code, if any.
+ */
 static void sig_child_handler(int sig UNUSED_PARAM)
 {
 	int wstat;
 	pid_t pid;
+	int sv_errno = errno;
 
 	while ((pid = wait_any_nohang(&wstat)) > 0) {
 		if (max_per_host)
@@ -236,8 +244,8 @@ static void sig_child_handler(int sig UNUSED_PARAM)
 		if (verbose)
 			print_waitstat(pid, wstat);
 	}
-	if (verbose)
-		connection_status();
+	if_verbose_print_connection_status();
+	errno = sv_errno;
 }
 
 int tcpudpsvd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
@@ -458,7 +466,7 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
 		xconnect(0, &remote.u.sa, sa_len);
 	/* hole? at this point we have no wildcard udp socket...
 	 * can this cause clients to get "port unreachable" icmp?
-	 * Yup, time window is very small, but it exists (is it?) */
+	 * Yup, time window is very small, but it exists (does it?) */
 		/* ..."open new socket", continued */
 		xbind(sock, &lsa->u.sa, sa_len);
 		socket_want_pktinfo(sock);
@@ -491,8 +499,7 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
 	if (pid != 0) {
 		/* Parent */
 		cnum++;
-		if (verbose)
-			connection_status();
+		if_verbose_print_connection_status();
 		if (hccp)
 			hccp->pid = pid;
 		/* clean up changes done by vforked child */
@@ -586,8 +593,14 @@ int tcpudpsvd_main(int argc UNUSED_PARAM, char **argv)
 
 	xdup2(0, 1);
 
+	/* Restore signal handling for the to-be-execed process */
 	signal(SIGPIPE, SIG_DFL); /* this one was SIG_IGNed */
-	/* Non-ignored signals revert to SIG_DFL on exec anyway */
+	/* Non-ignored signals revert to SIG_DFL on exec anyway
+	 * But we can get signals BEFORE execvp(), this is unlikely
+	 * but it would invoke sig_child_handler(), which would
+	 * check waitpid(WNOHANG), then print "status N/M" if verbose.
+	 * I guess we can live with that possibility.
+	 */
 	/*signal(SIGCHLD, SIG_DFL);*/
 	sig_unblock(SIGCHLD);
 
diff --git a/runit/runsv.c b/runit/runsv.c
index ecab8cdf5..61ea240ff 100644
--- a/runit/runsv.c
+++ b/runit/runsv.c
@@ -380,7 +380,10 @@ static void startservice(struct svdir *s)
 				xdup2(logpipe.wr, 1);
 			}
 		}
-		/* Non-ignored signals revert to SIG_DFL on exec anyway */
+		/* Non-ignored signals revert to SIG_DFL on exec anyway.
+		 * But we can get signals BEFORE execl(), this is unlikely
+		 * but wouldn't be good...
+		 */
 		/*bb_signals(0
 			+ (1 << SIGCHLD)
 			+ (1 << SIGTERM)