libbb: use _exit, not exit, in bb_daemonize_or_rexec()

By the time we reach exit in parent, child already exited or execed.
We should not re-run libc cleanup code.

While at it, introduce bb_daemon_helper() and add a few comments.

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
This commit is contained in:
Denys Vlasenko 2017-08-04 23:04:17 +02:00
parent 692eeb81a4
commit b182e9ad60
6 changed files with 22 additions and 15 deletions

View File

@ -99,7 +99,7 @@ static int find_free_vtno(void)
/*xfunc_error_retval = 3; - do we need compat? */ /*xfunc_error_retval = 3; - do we need compat? */
if (ioctl(fd, VT_OPENQRY, &vtno) != 0 || vtno <= 0) if (ioctl(fd, VT_OPENQRY, &vtno) != 0 || vtno <= 0)
bb_perror_msg_and_die("can't find open VT"); bb_perror_msg_and_die("can't find open VT");
// Not really needed, grep for DAEMON_ONLY_SANITIZE // Not really needed, grep for DAEMON_CLOSE_EXTRA_FDS
// if (fd > 2) // if (fd > 2)
// close(fd); // close(fd);
return vtno; return vtno;
@ -155,7 +155,7 @@ int openvt_main(int argc UNUSED_PARAM, char **argv)
/* Grab new VT */ /* Grab new VT */
sprintf(vtname, VC_FORMAT, vtno); sprintf(vtname, VC_FORMAT, vtno);
/* (Try to) clean up stray open fds above fd 2 */ /* (Try to) clean up stray open fds above fd 2 */
bb_daemonize_or_rexec(DAEMON_CLOSE_EXTRA_FDS | DAEMON_ONLY_SANITIZE, NULL); bb_daemon_helper(DAEMON_CLOSE_EXTRA_FDS);
close(STDIN_FILENO); close(STDIN_FILENO);
/*setsid(); - BAD IDEA: after we exit, child is SIGHUPed... */ /*setsid(); - BAD IDEA: after we exit, child is SIGHUPed... */
xopen(vtname, O_RDWR); xopen(vtname, O_RDWR);

View File

@ -516,6 +516,11 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
/* DAEMON_DEVNULL_STDIO is superfluous - /* DAEMON_DEVNULL_STDIO is superfluous -
* it's always done by bb_daemonize() */ * it's always done by bb_daemonize() */
#else #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.
*/
pid_t pid = xvfork(); pid_t pid = xvfork();
if (pid != 0) { if (pid != 0) {
/* parent */ /* parent */
@ -525,12 +530,8 @@ int start_stop_daemon_main(int argc UNUSED_PARAM, char **argv)
} }
/* Child */ /* Child */
setsid(); /* detach from controlling tty */ setsid(); /* detach from controlling tty */
/* Redirect stdio to /dev/null, close extra FDs. /* Redirect stdio to /dev/null, close extra FDs */
* We do not actually daemonize because of DAEMON_ONLY_SANITIZE */ bb_daemon_helper(DAEMON_DEVNULL_STDIO + DAEMON_CLOSE_EXTRA_FDS);
bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO
+ DAEMON_CLOSE_EXTRA_FDS
+ DAEMON_ONLY_SANITIZE,
NULL /* argv, unused */ );
#endif #endif
} }
if (opt & OPT_MAKEPID) { if (opt & OPT_MAKEPID) {

View File

@ -1169,6 +1169,7 @@ enum {
#endif #endif
void bb_daemonize_or_rexec(int flags, char **argv) FAST_FUNC; void bb_daemonize_or_rexec(int flags, char **argv) FAST_FUNC;
void bb_sanitize_stdio(void) FAST_FUNC; void bb_sanitize_stdio(void) FAST_FUNC;
#define bb_daemon_helper(arg) bb_daemonize_or_rexec((arg) | DAEMON_ONLY_SANITIZE, NULL)
/* Clear dangerous stuff, set PATH. Return 1 if was run by different user. */ /* Clear dangerous stuff, set PATH. Return 1 if was run by different user. */
int sanitize_env_if_suid(void) FAST_FUNC; int sanitize_env_if_suid(void) FAST_FUNC;

View File

@ -209,6 +209,9 @@ pid_t FAST_FUNC fork_or_rexec(char **argv)
/* Maybe we are already re-execed and come here again? */ /* Maybe we are already re-execed and come here again? */
if (re_execed) if (re_execed)
return 0; return 0;
/* fflush_all(); ? - so far all callers had no buffered output to flush */
pid = xvfork(); pid = xvfork();
if (pid) /* parent */ if (pid) /* parent */
return pid; return pid;
@ -245,8 +248,11 @@ void FAST_FUNC bb_daemonize_or_rexec(int flags, char **argv)
fd = dup(fd); /* have 0,1,2 open at least to /dev/null */ fd = dup(fd); /* have 0,1,2 open at least to /dev/null */
if (!(flags & DAEMON_ONLY_SANITIZE)) { if (!(flags & DAEMON_ONLY_SANITIZE)) {
/* fflush_all(); - add it in fork_or_rexec() if necessary */
if (fork_or_rexec(argv)) if (fork_or_rexec(argv))
exit(EXIT_SUCCESS); /* parent */ _exit(EXIT_SUCCESS); /* parent */
/* if daemonizing, detach from stdio & ctty */ /* if daemonizing, detach from stdio & ctty */
setsid(); setsid();
dup2(fd, 0); dup2(fd, 0);
@ -258,7 +264,7 @@ void FAST_FUNC bb_daemonize_or_rexec(int flags, char **argv)
* Prevent this: stop being a session leader. * Prevent this: stop being a session leader.
*/ */
if (fork_or_rexec(argv)) if (fork_or_rexec(argv))
exit(EXIT_SUCCESS); /* parent */ _exit(EXIT_SUCCESS); /* parent */
} }
} }
while (fd > 2) { while (fd > 2) {

View File

@ -350,8 +350,8 @@ int login_main(int argc UNUSED_PARAM, char **argv)
/* Mandatory paranoia for suid applet: /* Mandatory paranoia for suid applet:
* ensure that fd# 0,1,2 are opened (at least to /dev/null) * ensure that fd# 0,1,2 are opened (at least to /dev/null)
* and any extra open fd's are closed. * and any extra open fd's are closed.
* (The name of the function is misleading. Not daemonizing here.) */ */
bb_daemonize_or_rexec(DAEMON_ONLY_SANITIZE | DAEMON_CLOSE_EXTRA_FDS, NULL); bb_daemon_helper(DAEMON_CLOSE_EXTRA_FDS);
username[0] = '\0'; username[0] = '\0';
opt = getopt32(argv, "f:h:p", &opt_user, &opt_host); opt = getopt32(argv, "f:h:p", &opt_user, &opt_host);

View File

@ -198,9 +198,8 @@ int lpd_main(int argc UNUSED_PARAM, char *argv[])
q = p; // next line q = p; // next line
} }
// helper should not talk over network. // helper should not talk over network.
// this call reopens stdio fds to "/dev/null" // this call reopens stdio fds to "/dev/null".
// (no daemonization is done) bb_daemon_helper(DAEMON_DEVNULL_STDIO);
bb_daemonize_or_rexec(DAEMON_DEVNULL_STDIO | DAEMON_ONLY_SANITIZE, NULL);
BB_EXECVP_or_die(argv); BB_EXECVP_or_die(argv);
} }