Re: [PATCH] be paranoid about closed stdin/stdout/stderr

["Avery Pennarun" <apenwarr@gmail.com>]

On 8/26/08, Stephen R. van den Berg <srb@cuci.nl> wrote:
> Well, in general the policy I've used in all the tools I created is that:
>
>  a. If it's a setuid tool, then you need to make sure that you don't step
>    on anything unintendedly.  I.e. for setuid-something programs this is
>    desirable and necessary in order to prevent securityleaks.
>
>  b. Anything else is started in an environment controlled by the user,
>    and if this environment is broken, then that is the user's fault.

In general I'd mostly agree with you, but fd 0/1/2 are super-special
and I've personally been bitten by insane, rare problems that occur
when programs are started with one or more of those fds closed.

The usual case is that you're writing a new daemon.  The generally
accepted behaviour for a daemon is to chdir("/') and then close all
unnecessary open fds, in order to minimize the chance that it will be
holding open any directories or files that would prevent unmounting a
filesystem.  On the other hand, if the daemon then needs to run git
for some reason (who knows! maybe it's a git auto-commit daemon as was
discussed earlier on the list), it needs to open file descriptors
instead.  Such a program might work 99% of the time when git doesn't
happen to print any output.  But if there's ever an error, git would
print to fd#2 on die(), and that could corrupt some random file that
the daemon *or* git was using.  Remember, the situations where the
daemon leaves fd#2 open pointing at *the wrong thing* aren't the real
problem - you could easily say that's the daemon leaving the
environment in an insane state.  The problem situation is when git
opened some random file, and it *happened* to get assigned fd#2, and
then git incorrectly assumed that writing to fd#2 would not corrupt a
file that it opened.

Does this sound rare?  It is!  But it's also hellish to debug when it
happens, precisely because of its rarity.  For example, in one case, I
had this problem because an sfdisk process started by my custom
/sbin/init ran into a minor warning, and printed it to fd#2.
Unfortunately, because /sbin/init had opened sfdisk with fds 0/1/2
closed, fd#2 ended up being the very disk it was partitioning.  The
boot sector ended up getting overwritten with a warning message in
something like 1 out of 100 cases, and the computer wouldn't boot.
ARGH.  Easy to debug, once you think to read the boot sector as
plaintext.  But that's not the first thing you think to do.

Anyway, I personally think that given how incredibly cheap this
operation is to do, and how startlingly painful it is to debug when it
*is* a problem, that it would be nice if every program just did this
by default.  I would personally feel fine if such a thing ended up in
libc or the kernel, although presumably that would violate POSIX.

Yes, it would also be fine to have every *daemon* make sure it opens
/dev/null instead of just closing fd 0/1/2.  But it's harmless to have
both.

(As for the non-builtin git commands, isn't this an advantage of
having everything get run through the main /usr/bin/git wrapper?)

Have fun,

Avery