diff options
author | Denys Vlasenko | 2021-06-05 15:24:04 +0200 |
---|---|---|
committer | Denys Vlasenko | 2021-06-05 15:24:04 +0200 |
commit | d3e1090308b6d3c55e01a2000a743b73605ddd7f (patch) | |
tree | 57783b69d314c4b8998880ad6c38389989656782 | |
parent | ac444861b0b0212803ef775c366c41c4c020b1f1 (diff) | |
download | busybox-d3e1090308b6d3c55e01a2000a743b73605ddd7f.zip busybox-d3e1090308b6d3c55e01a2000a743b73605ddd7f.tar.gz |
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>
-rw-r--r-- | networking/tcpudp.c | 33 | ||||
-rw-r--r-- | runit/runsv.c | 5 |
2 files changed, 27 insertions, 11 deletions
diff --git a/networking/tcpudp.c b/networking/tcpudp.c index 8c4afab..708e05c 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 ecab8cd..61ea240 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) |