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

[Jeremy Huddleston]

On Jul 26, 2011, at 11:28 AM, Ray Strode wrote:

> 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.

yick... ok, well I'd suggest making that its own patch rather than squashed into the stdin one... I dunno how I feel about it and will let others chime in... 

>> 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.

Right.  That was the point of my *if* there ;) ... people have a tendency to copy-paste without taking such things into consideration