From 233e6f8f442197527fbe2673bc59eb8ef0afff5a Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Fri, 17 Aug 2018 11:30:16 -0300 Subject: [PATCH] s3: util: Do not take over stderr when there is no log file In case we don't have either a /var/log/samba directory, or pass a non-existent log directory through '-l' option, all commands that are daemonized with '-D' option hang when executed within a subshell. An example on how to trigger that: # rm -r /var/log/samba # s=$(nmbd -D -s /etc/samba/smb.conf -l /foo123) (never returns) So, when the above command is executed within a subshell the following happens: (a) Parent shell creates a pipe, sets write side of it to fd 1 (stdout), call read() on read-side fd, forks off a new child process and then executes nmbd in it. (b) nmbd sets up initial logging to go through fd 1 (stdout) by calling setup_logging(..., DEBUG_DEFAULT_STDOUT). 'state.fd' is now set to 1. (c) reopen_logs() is called by the first time which then calls reopen_logs_internal() (d) in reopen_logs_internal(), it attempts to create log.nmbd file in /foo123 directory and fails because directory doesn't exist. (e) Regardless whether the log file was created or not, it calls dup2(state.fd, 2) which dups fd 1 into fd 2. (f) At some point, fd 0 and 1 are closed and set to /dev/null The problem with that is because parent shell in (a) is still blocked in read() call and the new write side of the pipe is now fd 2 -- after dup2() in (e) -- and remains unclosed. Signed-off-by: Paulo Alcantara --- lib/util/debug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/util/debug.c b/lib/util/debug.c index 141f3a595ca..d41e0f99c77 100644 --- a/lib/util/debug.c +++ b/lib/util/debug.c @@ -1077,8 +1077,11 @@ bool reopen_logs_internal(void) force_check_log_size(); (void)umask(oldumask); - /* Take over stderr to catch output into logs */ - if (state.fd > 0) { + /* + * If log file was opened or created successfully, take over stderr to + * catch output into logs. + */ + if (new_fd != -1) { if (dup2(state.fd, 2) == -1) { /* Close stderr too, if dup2 can't point it - at the logfile. There really isn't much -- 2.18.0