From fa535f3e485456a7fd85db060532ea6539670af0 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 2 Mar 2015 17:37:31 +0100 Subject: runsvdir: (almost) close a signal race We could lose a signal while processing previous one function old new delta runsvdir_main 1088 1077 -11 Signed-off-by: Denys Vlasenko --- runit/runsvdir.c | 53 ++++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 25 deletions(-) (limited to 'runit') diff --git a/runit/runsvdir.c b/runit/runsvdir.c index af7e75ba7..809c48a51 100644 --- a/runit/runsvdir.c +++ b/runit/runsvdir.c @@ -59,7 +59,6 @@ struct globals { int svnum; #if ENABLE_FEATURE_RUNSVDIR_LOG char *rplog; - int rploglen; struct fd_pair logpipe; struct pollfd pfd[1]; unsigned stamplog; @@ -70,7 +69,6 @@ struct globals { #define svdir (G.svdir ) #define svnum (G.svnum ) #define rplog (G.rplog ) -#define rploglen (G.rploglen ) #define logpipe (G.logpipe ) #define pfd (G.pfd ) #define stamplog (G.stamplog ) @@ -219,15 +217,11 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) struct stat s; dev_t last_dev = last_dev; /* for gcc */ ino_t last_ino = last_ino; /* for gcc */ - time_t last_mtime = 0; - int wstat; + time_t last_mtime; int curdir; - pid_t pid; - unsigned deadline; - unsigned now; unsigned stampcheck; int i; - int need_rescan = 1; + int need_rescan; char *opt_s_argv[3]; INIT_G(); @@ -257,8 +251,7 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) /* setup log */ if (*argv) { rplog = *argv; - rploglen = strlen(rplog); - if (rploglen < 7) { + if (strlen(rplog) < 7) { warnx("log must have at least seven characters"); } else if (piped_pair(logpipe)) { warnx("can't create pipe for log"); @@ -287,11 +280,16 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) close_on_exec_on(curdir); stampcheck = monotonic_sec(); + need_rescan = 1; + last_mtime = 0; for (;;) { + unsigned now; + unsigned sig; + /* collect children */ for (;;) { - pid = wait_any_nohang(&wstat); + pid_t pid = wait_any_nohang(NULL); if (pid <= 0) break; for (i = 0; i < svnum; i++) { @@ -345,15 +343,17 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) } pfd[0].revents = 0; #endif - deadline = (need_rescan ? 1 : 5); - sig_block(SIGCHLD); + { + unsigned deadline = (need_rescan ? 1 : 5); + sig_block(SIGCHLD); #if ENABLE_FEATURE_RUNSVDIR_LOG - if (rplog) - poll(pfd, 1, deadline*1000); - else + if (rplog) + poll(pfd, 1, deadline*1000); + else #endif - sleep(deadline); - sig_unblock(SIGCHLD); + sleep(deadline); + sig_unblock(SIGCHLD); + } #if ENABLE_FEATURE_RUNSVDIR_LOG if (pfd[0].revents & POLLIN) { @@ -361,21 +361,25 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) while (read(logpipe.rd, &ch, 1) > 0) { if (ch < ' ') ch = ' '; - for (i = 6; i < rploglen; i++) + for (i = 6; rplog[i] != '\0'; i++) rplog[i-1] = rplog[i]; - rplog[rploglen-1] = ch; + rplog[i-1] = ch; } } #endif - if (!bb_got_signal) + sig = bb_got_signal; + if (!sig) continue; + bb_got_signal = 0; /* -s SCRIPT: useful if we are init. * In this case typically script never returns, * it halts/powers off/reboots the system. */ if (opt_s_argv[0]) { + pid_t pid; + /* Single parameter: signal# */ - opt_s_argv[1] = utoa(bb_got_signal); + opt_s_argv[1] = utoa(sig); pid = spawn(opt_s_argv); if (pid > 0) { /* Remembering to wait for _any_ children, @@ -385,7 +389,7 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) } } - if (bb_got_signal == SIGHUP) { + if (sig == SIGHUP) { for (i = 0; i < svnum; i++) if (sv[i].pid) kill(sv[i].pid, SIGTERM); @@ -393,9 +397,8 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) /* SIGHUP or SIGTERM (or SIGUSRn if we are init) */ /* Exit unless we are init */ if (getpid() != 1) - return (SIGHUP == bb_got_signal) ? 111 : EXIT_SUCCESS; + return (SIGHUP == sig) ? 111 : EXIT_SUCCESS; /* init continues to monitor services forever */ - bb_got_signal = 0; } /* for (;;) */ } -- cgit v1.2.3-55-g6feb From dac8d80f77af617effadc50f6be47768685d81b0 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 2 Mar 2015 17:38:18 +0100 Subject: runsvdir: do not block SIGCHLD around poll/sleep There is no reason to do so. We do not even have SIGCHLD handler. function old new delta runsvdir_main 1077 1057 -20 Signed-off-by: Denys Vlasenko --- runit/runsvdir.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'runit') diff --git a/runit/runsvdir.c b/runit/runsvdir.c index 809c48a51..a08af3bae 100644 --- a/runit/runsvdir.c +++ b/runit/runsvdir.c @@ -345,14 +345,12 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) #endif { unsigned deadline = (need_rescan ? 1 : 5); - sig_block(SIGCHLD); #if ENABLE_FEATURE_RUNSVDIR_LOG if (rplog) poll(pfd, 1, deadline*1000); else #endif sleep(deadline); - sig_unblock(SIGCHLD); } #if ENABLE_FEATURE_RUNSVDIR_LOG -- cgit v1.2.3-55-g6feb From 7db2a7c20e8d48ad9cce934c9a6d2ee563d32c84 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 2 Mar 2015 17:39:13 +0100 Subject: runsvdir: if pid==1, also intercept SIGINT for -s SCRIPT function old new delta runsvdir_main 1057 1064 +7 Signed-off-by: Denys Vlasenko --- runit/runsvdir.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'runit') diff --git a/runit/runsvdir.c b/runit/runsvdir.c index a08af3bae..b4c0b2ef0 100644 --- a/runit/runsvdir.c +++ b/runit/runsvdir.c @@ -222,6 +222,7 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) unsigned stampcheck; int i; int need_rescan; + bool i_am_init; char *opt_s_argv[3]; INIT_G(); @@ -232,18 +233,21 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) getopt32(argv, "Ps:", &opt_s_argv[0]); argv += optind; + i_am_init = (getpid() == 1); bb_signals(0 | (1 << SIGTERM) | (1 << SIGHUP) /* For busybox's init, SIGTERM == reboot, - * SIGUSR1 == halt - * SIGUSR2 == poweroff - * so we need to intercept SIGUSRn too. + * SIGUSR1 == halt, + * SIGUSR2 == poweroff, + * Ctlr-ALt-Del sends SIGINT to init, + * so we need to intercept SIGUSRn and SIGINT too. * Note that we do not implement actual reboot * (killall(TERM) + umount, etc), we just pause * respawing and avoid exiting (-> making kernel oops). - * The user is responsible for the rest. */ - | (getpid() == 1 ? ((1 << SIGUSR1) | (1 << SIGUSR2)) : 0) + * The user is responsible for the rest. + */ + | (i_am_init ? ((1 << SIGUSR1) | (1 << SIGUSR2) | (1 << SIGINT)) : 0) , record_signo); svdir = *argv++; @@ -394,7 +398,7 @@ int runsvdir_main(int argc UNUSED_PARAM, char **argv) } /* SIGHUP or SIGTERM (or SIGUSRn if we are init) */ /* Exit unless we are init */ - if (getpid() != 1) + if (!i_am_init) return (SIGHUP == sig) ? 111 : EXIT_SUCCESS; /* init continues to monitor services forever */ -- cgit v1.2.3-55-g6feb