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

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