tcp/udpsvd: robustify SIGCHLD handling

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@googlemail.com>

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)