From 088fec36fedff2cd50437c95b7fb430abf8d303c Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Mon, 14 Jan 2019 14:45:18 +0100 Subject: [PATCH] start-stop-daemon: create pidfile before parent exits, closes 8596 This removes DAEMON_DOUBLE_FORK flag from bb_daemonize_or_rexec(), as SSD was the only user. Also includes fix for -S: now works without -a and -x, does not print pids (compat with "start-stop-daemon (OpenRC) 0.34.11 (Gentoo Linux)"). function old new delta start_stop_daemon_main 1018 1084 +66 add_interface 99 103 +4 fail_hunk 139 136 -3 bb_daemonize_or_rexec 205 183 -22 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/2 up/down: 70/-25) Total: 45 bytes Signed-off-by: Denys Vlasenko --- debianutils/start_stop_daemon.c | 66 ++++++++++++++++++++----------- include/libbb.h | 10 ++--- libbb/vfork_daemon_rexec.c | 16 ++++---- testsuite/start-stop-daemon.tests | 5 +++ 4 files changed, 62 insertions(+), 35 deletions(-) diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c index 43b6fca26..fa08f48cf 100644 --- a/debianutils/start_stop_daemon.c +++ b/debianutils/start_stop_daemon.c @@ -409,7 +409,7 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) { unsigned opt; char *signame; - char *startas; + char *startas = NULL; char *chuid; #if ENABLE_FEATURE_START_STOP_DAEMON_FANCY // char *retry_arg = NULL; @@ -425,10 +425,11 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) /* -K or -S is required; they are mutually exclusive */ /* -p is required if -m is given */ /* -xpun (at least one) is required if -K is given */ - /* -xa (at least one) is required if -S is given */ +// /* -xa (at least one) is required if -S is given */ +//WRONG: "start-stop-daemon -S -- sleep 5" is a valid invocation /* -q turns off -v */ "\0" - "K:S:K--S:S--K:m?p:K?xpun:S?xa" + "K:S:K--S:S--K:m?p:K?xpun" IF_FEATURE_START_STOP_DAEMON_FANCY("q-v"), LONGOPTS &startas, &cmdname, &signame, &userspec, &chuid, &execname, &pidfile @@ -442,21 +443,34 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) if (signal_nr < 0) bb_show_usage(); } - if (!(opt & OPT_a)) - startas = execname; - if (!execname) /* in case -a is given and -x is not */ + //argc -= optind; + argv += optind; +// ARGS contains zeroth arg if -x/-a is not given, else it starts with 1st arg. +// These will try to execute "[/bin/]sleep 5": +// "start-stop-daemon -S -- sleep 5" +// "start-stop-daemon -S -x /bin/sleep -- 5" +// "start-stop-daemon -S -a sleep -- 5" +// NB: -n option does _not_ behave in this way: this will try to execute "5": +// "start-stop-daemon -S -n sleep -- 5" + if (!execname) { /* -x is not given */ execname = startas; - if (execname) { - G.execname_sizeof = strlen(execname) + 1; - G.execname_cmpbuf = xmalloc(G.execname_sizeof + 1); + if (!execname) { /* neither -x nor -a is given */ + execname = argv[0]; + if (!execname) + bb_show_usage(); + argv++; + } } + if (!startas) /* -a is not given: use -x EXECUTABLE or argv[0] */ + startas = execname; + *--argv = startas; + G.execname_sizeof = strlen(execname) + 1; + G.execname_cmpbuf = xmalloc(G.execname_sizeof + 1); // IF_FEATURE_START_STOP_DAEMON_FANCY( // if (retry_arg) // retries = xatoi_positive(retry_arg); // ) - //argc -= optind; - argv += optind; if (userspec) { user_id = bb_strtou(userspec, NULL, 10); @@ -473,7 +487,7 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) if (G.found_procs) { if (!QUIET) - printf("%s is already running\n%u\n", execname, (unsigned)G.found_procs->pid); + printf("%s is already running\n", execname); return !(opt & OPT_OKNODO); } @@ -482,30 +496,37 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) xstat(execname, &G.execstat); #endif - *--argv = startas; if (opt & OPT_BACKGROUND) { -#if BB_MMU - bb_daemonize(DAEMON_DEVNULL_STDIO + DAEMON_CLOSE_EXTRA_FDS + DAEMON_DOUBLE_FORK); - /* DAEMON_DEVNULL_STDIO is superfluous - - * it's always done by bb_daemonize() */ -#else /* Daemons usually call bb_daemonize_or_rexec(), but SSD can do * without: SSD is not itself a daemon, it _execs_ a daemon. * The usual NOMMU problem of "child can't run indefinitely, * it must exec" does not bite us: we exec anyway. + * + * bb_daemonize(DAEMON_DEVNULL_STDIO | DAEMON_CLOSE_EXTRA_FDS | DAEMON_DOUBLE_FORK) + * can be used on MMU systems, but use of vfork() + * is preferable since we want to create pidfile + * _before_ parent returns, and vfork() on Linux + * ensures that (by blocking parent until exec in the child). */ pid_t pid = xvfork(); if (pid != 0) { - /* parent */ + /* Parent */ /* why _exit? the child may have changed the stack, - * so "return 0" may do bad things */ + * so "return 0" may do bad things + */ _exit(EXIT_SUCCESS); } /* Child */ setsid(); /* detach from controlling tty */ /* Redirect stdio to /dev/null, close extra FDs */ bb_daemon_helper(DAEMON_DEVNULL_STDIO + DAEMON_CLOSE_EXTRA_FDS); -#endif + /* On Linux, session leader can acquire ctty + * unknowingly, by opening a tty. + * Prevent this: stop being a session leader. + */ + pid = xvfork(); + if (pid != 0) + _exit(EXIT_SUCCESS); /* Parent */ } if (opt & OPT_MAKEPID) { /* User wants _us_ to make the pidfile */ @@ -534,6 +555,7 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv) } } #endif - execvp(startas, argv); +//bb_error_msg("HERE %d '%s'%s'", __LINE__, argv[0], argv[1]); + execvp(argv[0], argv); bb_perror_msg_and_die("can't execute '%s'", startas); } diff --git a/include/libbb.h b/include/libbb.h index d2563999a..3366df30f 100644 --- a/include/libbb.h +++ b/include/libbb.h @@ -1201,11 +1201,11 @@ void set_task_comm(const char *comm) FAST_FUNC; * to /dev/null if they are not. */ enum { - DAEMON_CHDIR_ROOT = 1, - DAEMON_DEVNULL_STDIO = 2, - DAEMON_CLOSE_EXTRA_FDS = 4, - DAEMON_ONLY_SANITIZE = 8, /* internal use */ - DAEMON_DOUBLE_FORK = 16, /* double fork to avoid controlling tty */ + DAEMON_CHDIR_ROOT = 1 << 0, + DAEMON_DEVNULL_STDIO = 1 << 1, + DAEMON_CLOSE_EXTRA_FDS = 1 << 2, + DAEMON_ONLY_SANITIZE = 1 << 3, /* internal use */ + //DAEMON_DOUBLE_FORK = 1 << 4, /* double fork to avoid controlling tty */ }; #if BB_MMU enum { re_execed = 0 }; diff --git a/libbb/vfork_daemon_rexec.c b/libbb/vfork_daemon_rexec.c index c0bea0ed2..1aac27b36 100644 --- a/libbb/vfork_daemon_rexec.c +++ b/libbb/vfork_daemon_rexec.c @@ -292,14 +292,14 @@ void FAST_FUNC bb_daemonize_or_rexec(int flags, char **argv) dup2(fd, 0); dup2(fd, 1); dup2(fd, 2); - if (flags & DAEMON_DOUBLE_FORK) { - /* On Linux, session leader can acquire ctty - * unknowingly, by opening a tty. - * Prevent this: stop being a session leader. - */ - if (fork_or_rexec(argv)) - _exit(EXIT_SUCCESS); /* parent */ - } +// if (flags & DAEMON_DOUBLE_FORK) { +// /* On Linux, session leader can acquire ctty +// * unknowingly, by opening a tty. +// * Prevent this: stop being a session leader. +// */ +// if (fork_or_rexec(argv)) +// _exit(EXIT_SUCCESS); /* parent */ +// } } while (fd > 2) { close(fd--); diff --git a/testsuite/start-stop-daemon.tests b/testsuite/start-stop-daemon.tests index d07aeef0e..be1c1a856 100755 --- a/testsuite/start-stop-daemon.tests +++ b/testsuite/start-stop-daemon.tests @@ -16,4 +16,9 @@ testing "start-stop-daemon -a without -x" \ "1\n" \ "" "" +testing "start-stop-daemon without -x and -a" \ + 'start-stop-daemon -S false 2>&1; echo $?' \ + "1\n" \ + "" "" + exit $FAILCOUNT