(Note that associates of mine were the ones who discovered this bug; I'm just the reporter.) When starting, if sshd isn't in debugging mode, and isn't being started from inetd, it will call the daemon() function to fork, and will then disconnect from the controlling tty. However, sshd makes no attempt to close any open file descriptors it may have inherited. (Perhaps the assumption was that daemon() did that, but it doesn't.) This is a bug; any program that forks itself into the background should always close all open fds. Attached is a patch which solves the problem on Linux and Solaris systems. (I do not know if the patch is appropriate for all operating systems.)
Created attachment 1 [details] patch to make sshd close all open fds when forking
we should probably use sysconf(_SC_OPEN_MAX) move the code to misc.c and share code it with do_child()
daemon() works fine. the hang-on-exit is about fd 0,1,2
It doesn't matter whether or not daemon() or some other helper function does it, but sshd MUST close all fds from 3 to sysconf(_SC_OPEN_MAX) after forking. If the hang-on-exit bug is with fds 0/1/2, then indeed, this bug isn't related to the hang-on-exit bug. But it's still a bug.
daemons call daemon(), they do not care about filedescriptors > 3. btw, i was just told that cat(1) does not close extra filedescriptors, and i don't think there are more important issues.
Let me explain this from the beginning. A process which intends to run as a daemon--that is, run for a long period without intervention, until it is manually killed and/or the system is shut down--should perform certain actions immediately after it is launched. These actions have been enumerated by multiple people in multiple references, including [Stevens]. According to [Stevens] section 13.3, these are the basic rules for coding a daemon: 1. Call fork() and have the parent exit. 2. Call setsid() to create a new session. (On SVR4-based systems, some people recommend calling fork() again and having the parent terminate, which will guarantee that the daemon will never acquire a controlling terminal, without having to use O_NOCTTY whenever opening a terminal device.) 3. Change the current working directory to the root directory. 4. Set the file mode creation mask to 0. 5. Close all unneeded file descriptors, including any that may have been inherited from the parent. The daemon() utility function from openbsd-compat/daemon.c (which sshd calls) performs steps #1 and #2 on sshd's behalf (although it does not perform the additional fork for step #2). (Curiously, immediately after calling openbsd-compat/daemon.c, sshd.c uses the obsolete TIOCNOTTY ioctl() to try to disassociate the controlling terminal. This is redundant; the setsid() call in openbsd-compat/daemon.c will always disassociate any controlling terminal. If anything, sshd should perform the additional fork for SVR4-based systems here.) sshd.c itself performs step #3. Neither sshd.c nor openbsd-compat/daemon.c performs step #4. While sshd.c should arguably perform this step for completeness, I believe that the flow of control is such that sshd will never create any files before changing the umask. Finally, for step #5, openbsd-compat/daemon.c does indeed take care of closing fds 0, 1, and 2, and reopening them to /dev/null (to satisfy library calls that may assume these fds are open). But openbsd-compat/daemon.c makes no attempt to close any other fds it has inherited from its parent. This is a violation of rule #5, and a bug. You claim that this bug isn't important. But this bug wasn't found from code audits; this bug was found because sshd hung a system update program (by inheriting a socket from its parent and never closing it). In other words, this bug DIRECTLY caused sshd to break, and fixing the bug was necessary in order to prevent sshd from breaking. Your assertion that "i was just told that cat(1) does not close extra filedescriptors" is a non sequitur, because cat(1) is not a daemon program. If you'd like, I will rework the patch I supplied to use sysconf(_SC_OPEN_MAX). I'm also willing to add the second fork to sshd, eliminate the redundant TIOCNOTTY ioctl(), and put in a umask() call. This is the last time I will reopen this bug report. References: [Stevens]: Advanced Programming in the UNIX Environment, W. Richard Stevens, Addison-Wesley, 1992.
isn't your software distribution program broken if it exec()s sshd without closing filedescriptors that might cause it break?
The question is irrelevant; regardless of how one chooses to answer it, the answer does not make sshd's behavior (of not making sure all inherited descriptors are closed) any less broken.
a decision has been made to not do this.
http://marc.theaimsgroup.com/?l=bind9-workers&m=103112021703700&w=2
Mass change of RESOLVED bugs to CLOSED