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. So, IMHO, we should be checking if the log file was created successfuly in (d), and if so, take over stderr with the new created file. Basically, my proposed fix would be replacing the following check in reopen_logs_internal(): /* Take over stderr to catch output into logs */ if (state.fd > 0) { if (dup2(state.fd, 2) == -1) { ... } ... } With: /* Take over stderr to catch output into logs */ if (new_fd != -1) { if (dup2(state.fd, 2) == -1) { ... } ... } By doing so, we properly closes stdout and unblocks read() call from parent shell and nmbd returns correctly.
An email has been sent to ML[1] along with a proposed fix. I'll send a formal patch as soon as I make sure it passes autobuild / gitlab CI. Thanks, Paulo [1] https://marc.info/?i=87lg9pbs6d.fsf@paulo.ac
Created attachment 14430 [details] Proposed fix Follow patch for review & comments. Thanks.
Comment on attachment 14430 [details] Proposed fix Pushed to master.
Created attachment 14445 [details] git-am fix for 4.9.next, 4.8.next, 4.7.next Cherry-picked from master.
Re-assigning to Karolin for inclusion in 4.9.next, 4.8.next, 4.7.next.
(In reply to Jeremy Allison from comment #5) Pushed to autobuild-v4-{9,8,7}-test.
Pushed to all branches. Closing out bug report. Thanks!