Hi all,
Am 27.01.2016 um 15:01 schrieb Julien Pauli:
> On Wed, Jan 27, 2016 at 1:03 PM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
>> Hi all,
>>
>> This bug comes to my attention.
>> https://bugs.php.net/bug.php?id=67383
>>
>> This report includes patch that prevents file descriptor leaks.
>> https://bugs.php.net/patch-display.php?bug_id=67383&patch=SOCK_CLOEXEC-and-FD_CLOEXEC&revision=latest
>>
>> There may be cases that CLOEXEC cannot be applied blindly, but it
>> seems this fixes lots of bugs including very old bugs.
>> Why this patch is not merged and closed?
>>
>> Regards,
>>
>> --
>> Yasuo Ohgaki
>> yohgaki@ohgaki.net
>>
>> --
>> PHP Internals - PHP Runtime Development Mailing List
>> To unsubscribe, visit: http://www.php.net/unsub.php
>>
>
> Hi,
>
> Great catch.
>
> We already have many CLOEXEC, but we seem to be missing lots of them.
>
> There is a note however, in bug ticket 67383, about a problem in FPM
> with CLOEXEC, which should be qualified and addressed.
> Just don't apply blindly the patch.
I think this patch is going to far. I agree any socket that is created
by the sapi should be opened with SOCK_CLOEXEC. But in userland it must
be possible to keep a socket open when forking, otherwise it is a big BC
issue.
At least the changes in ext/socket/socket.c,
main/streams/plain_wrapper.c and main/fopen_wrappers.c change userland
socket behavior in a not-compatible way.
Maybe a fcntl wrapper which allows to enable/disable the SOCK_CLOEXEC
flag should be introduced (in the next minor) and later (maybe in 8)
userland sockets could be opened with SOCK_CLOEXEC set (as a security
thing) with a workaround available for manual forking and keeping the
sockets open.
Greets
Dennis
