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>

diff --git a/debianutils/start_stop_daemon.c b/debianutils/start_stop_daemon.c
index 43b6fca260429187d758733d334ec09818cea049..fa08f48cf5c967448376fa3ab423e98562b041e0 100644 (file)
--- 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 d2563999a1bc8fbb9295250b96cf0dfc58756ab4..3366df30f38a24aebfda744a3de59b7a39693ee4 100644 (file)
--- 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 c0bea0ed21b3eb7bcc806432627e3c25fbb28b86..1aac27b360adc19ea23e57f695b7c558149cad8e 100644 (file)
--- 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 d07aeef0e12e4783477dd9c8b3db42b22c27646a..be1c1a856cfd9349f986958b325a4ae4de747dcf 100755 (executable)
--- 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