[lxc-devel] [PATCH 2/2] lxc-start shouldn't exit with error, if there is inherited fd's.

[Cedric Le Goater]

Hi,

On 08/25/2011 10:05 AM, Smirnov Vladimir wrote:
> Maybe it's a security reason. This patch fixes my problem. I've talked with 
> Cedric, he said that it's supposed to be restrictive.

I agree with Greg that this behavior is too restrictive in a general 
context. It was inherited from our C/R branch.  

A warning should be enough.

Cheers,

C.


> Other comments in reply to 2nd patch.
> 
> 25.08.2011, 03:28, "Greg Kurz" <gkurz at fr.ibm.com>:
>> On Wed, 2011-08-24 at 14:17 +0400, Vladimir Smirnov wrote:
>>
>>>  Previous patch fixed behaviour with clone, so it's now safe just to set O_CLOEXEC flag on
>>>  all inherited fd's.
>>
>> The only use case I know about where a fd leak matters is
>> checkpoint/restart... I think the behaviour of lxc_check_inherited() is
>> too restrictive. It should only print a warning when it spots a fd leak
>> and return 0.
>>
>> Cc'd Daniel and Cedric to have some more feedback on this topic.
>>
>> And for you Vladimir, would this change fix your troubles ?
>>
>>>  Signed-off-by: Vladimir Smirnov <civil at yandex-team.ru>
>>>  ---
>>>   src/lxc/start.c |   15 +++++++++++++--
>>>   1 files changed, 13 insertions(+), 2 deletions(-)
>>>
>>>  diff --git a/src/lxc/start.c b/src/lxc/start.c
>>>  index b8ceff6..6df70dc 100644
>>>  --- a/src/lxc/start.c
>>>  +++ b/src/lxc/start.c
>>>  @@ -154,6 +154,7 @@ int lxc_check_inherited(int fd_to_ignore)
>>>           while (!readdir_r(dir, &dirent, &direntp)) {
>>>                   char procpath[64];
>>>                   char path[PATH_MAX];
>>>  + int flags;
>>>
>>>                   if (!direntp)
>>>                           break;
>>>  @@ -174,14 +175,24 @@ int lxc_check_inherited(int fd_to_ignore)
>>>                   /*
>>>                    * found inherited fd
>>>                    */
>>>  - ret = -1;
>>>  + flags = fcntl(fd, F_GETFD);
>>>  + if (flags < 0) {
>>>  + ret = -1;
>>>  + ERROR("failed to get flags, fd %d on %s", fd, path);
>>>  + }
>>>  +
>>>  + fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
>>>  + if (flags < 0) {
>>>  + ret = -1;
>>>  + ERROR("failed to set CLOEXEC, fd %d on %s", fd, path);
>>>  + }
>>>
>>>                   snprintf(procpath, sizeof(procpath), "/proc/self/fd/%d", fd);
>>>
>>>                   if (readlink(procpath, path, sizeof(path)) == -1)
>>>                           ERROR("readlink(%s) failed : %m", procpath);
>>>                   else
>>>  - ERROR("inherited fd %d on %s", fd, path);
>>>  + WARN("inherited fd %d on %s", fd, path);
>>>           }
>>>
>>>           if (closedir(dir))
>> --
>> Gregory Kurz                                     gkurz at fr.ibm.com
>> Software Engineer @ IBM/Meiosys                  http://www.ibm.com
>> Tel +33 (0)534 638 479                           Fax +33 (0)561 400 420
>>
>> "Anarchy is about taking complete responsibility for yourself."
>>        Alan Moore.
>