[lxc-devel] fixing restoring of tty mode after lxc-start

[Michael Tokarev]

Michel Normand wrote:
> Michael Tokarev a écrit :
>> Michel Normand wrote:
>>> Michael Tokarev a e'crit :
>>>> Hello.
>>>>
>>>> While trying to fix the tty mode after lxc-start, I come across
>>>> lxc_close_all_inherited_fd() routine.  Which closes all fds which
>>>> were inherited when lxc-start started.
>>>>
>>>> It's called this way:
>>>>
>>>>   lxc_start.c:main():
>>>>     lxc_start():
>>>>      lxc_spawn();
>>>>      lxc_close_all_inherited_fd()
>>>>      lxc_poll()
>>>>
>>>> The question is why lxc_close_all_inherited() is needed here.
>>>> And why it is ever needed _outside_ of the container to start
>>>> with.
>>>>
>>>> This call to close_all_inherited_fd() is the reason why tty
>>>> modes are not restored after lxc-start: because stdin is now
>>>> closed and hence nothing to restore.
>>>>
>>>> It looks like the whole inherited_fd thing is only needed in
>>>> lxc_init helper binary, 
>>>
>>> In fact the lxc_close_all_inherited_fd function was defined
>>> to cover the case of a user appli creating pipes before to fork
>>> the lxc-start, and to use these pipes to communicate with the appli
>>> started inside the container.
>>
>> There's no need to close these pipes in lxc_start itself.  For that,
>> it is enough to close them in the child of lxc_start, the one which
>> becomes container's init process.  Which is exactly what I'm talking
>> about here.
> 
> This would not cover all possible usage of the pipe.
> eg: parent on read() of the pipe will not be able to leave
> when the application is closing it's write side if lxc is still
> holding the fds.

Michel, please excuse me, but this makes no sense.

We've two usage cases here:

1.  lxc_start as a separate executable.  If lxc_start is called
  by some application with opened internal pipes that it expects
  to use to communicate with other programs, it is a clean 100%
  bug in that application.  It should perform more careful handling
  of the file descriptors it is interested in, and not pass them to
  random applications it is calling.  If that application worked but
  will stop working because lxc_start (the executable) stopped closing
  all filedescriptors is not lxc_start's fault.  It is not our business
  to clean up after bugs in random applications calling us.

2. In case lxc_start(), the function, is called inside some other
  program (liblxc is a library).  In this case, lxc_start() (the
  function) MUST NOT CLOSE unrelated filedescriptors.  It is, in
  fact, exactly the issue at hand: lxc_start (the executable)
  set up the tty (by saving termio parameters), and expected to do
  some cleanup after the run (by restoring termio), but lxc_start()
  (the function) closed unrelated, not owned by it, filedescriptors
  thus preventing lxc_start (the application) to do their work.

It makes good sense to close all fds in a _child_ spawned by lxc_start(),
to not leak unrelated filedescriptors into the container (it may even be
a security hole).  But for _that_ task, easier and simpler solution may
be proposed instead - if you want to loop over /proc/$pid/fd/, it can
be done inside lxc_spawn() right after spawning the to-be-init process,
without remembering any lists and the like.

But I repeat myself.

Thanks!

/mjt

>>> For that purpose it was required for lxc to not keep a reference
>>> on these fds while lxc code is waiting for the appli to run up to its 
>>> end.
>>> This is the reason why this function is called in two places
>>> in lxc_init.c and start.c
>>
>> Note that lxc_init, like lxc_start's child, runs inside the container.
> 
> yes it is, and it must also release the inherited fds for the
> explained user case, and for the same reason as in start.c
> 
>>
>> Thanks!
>>
>> /mjt
>>
>>
>