[PATCH 2/2] Xinit: close stdin to avoid leak of file descriptior to the Xorg session.

[Ray Strode]

Hi,

Apparently I wrote this patch like 5 years and promptly forgot about.
Matej tracked me down and asked me to chime in.  I don't really
remember the patch, but it sort of makes sense to me so I'll try to
answer any questions about it.

> What's the point of setsid(2)?  It certainly doesn't fall under the title of the patch.

Looking through version control, the original message in the patch is:

      - start client in its own session with no controlling tty (bug 214649)

This is also a bit terse, sadly, but a little clearer.  The idea is,
when you start an X server, the tty the X server started on is pretty
irrelevant as far as the clients are concerned  Even the X server
itself is going to be running on its own VT switched away from the tty
it was started on.  Given the tty doesn't matter, the path tries to
remove remnants of that tty from the client's view.

The code previously did setpgid(0, 0) which just means "put the client
in its own process group in the same session as the shell it was
started from."  But the shell xinit is started from is irrelevant to
the clients running on the X server.  Process group is roughly
synonymous with "shell job"  Taken in that light, we wouldn't want
xinit to be job %1 and xterm or gnome-session or whatever to be job %2
at the sh prompt the user typed startx from.  That makes no sense and
has weird unix-y ramifications (if the client or one of the clients
descendants tries to read from stdin it's going to get stopped with
SIGTTIN since it might not be in the foreground process group).

By using setsid() we make sure the X session initiated with that first
X client is started in its own terminal session independent of the
shell it was started from.  We still leave stdout/stderr hooked up to
the tty since multiple sessions can output to a tty without much
trouble, and then client output doesn't get sent into the aether.
Redirecting to ~/.xsession-errors would be fine too.

>
> Also, you don't need:
> +         close (STDIN_FILENO);
true, there are times when you should explicitly close before dup2()
(if you need to do proper error checking on close()), but this isn't
one of them

> That is done for you by dup2(2).  Also doing so could result in a problem > if this code were used in a multithreaded application.  If another thread
> called open(2) between the close(2) and the dup2(2), it would get
> STDIN_FILENO as its fd and you would then close it with your dup2(2).
xinit isn't multithreaded, though, and even if it were, this is less
than a dozen lines down from a fork(). so the function itself is
necessarily running in a single thread.

--Ray