llvm dev - Jan 2017 - O_CLOEXEC-like functionality for llvm::raw_fd_ostream

Hello all,

I am looking into using LLVM streams more extensively in LLDB (which
currently rolls it's own stream classes). One of the things that's
missing for me to be able to do that is the ability to open a file
with the O_CLOEXEC flag (to prevent us leaking file descriptors into
the debugged process).

So I tried adding a F_NonInheritable flag to the raw_fd_ostream
constructor, which would map to O_CLOEXEC on unix, and non-inheritable
handles on windows. However, I encountered a discrepancy in the
current behavior there.

The current behavior on unix is to make the file descriptors
inheritable (as that is the platform default). On windows, the current
behavior is to make the handles *non*-inheritable (again, because of
platform default). Obviously, if we add this flag, we will probably
need to settle on a default behavior which is consistent across all
platforms.

Instead of going with the unix choice of inherit-by-default, I propose
to go with the windows behavior where one has to explicitly opt into
file handle inheritance. I believe that is the safer way, as when one
thinks about creating a process, he usually knows which files/handles
it wants it to inherit (so he can specify it), whereas if he is not
creating a process, it does not matter whether it ends up being
inheritable (and he can avoid it leaking by accident!).

What do you think about this proposal? Does anyone know why this might
be a bad idea?

PS: Currently the test suite passes on windows&linux regardless of
which way I set the default to be.

cheers,
pavel

Pavel Labath via llvm-dev <llvm-dev at lists.llvm.org> writes:
> Hello all,
>
> I am looking into using LLVM streams more extensively in LLDB (which
> currently rolls it's own stream classes). One of the things that's
> missing for me to be able to do that is the ability to open a file
> with the O_CLOEXEC flag (to prevent us leaking file descriptors into
> the debugged process).
>
> So I tried adding a F_NonInheritable flag to the raw_fd_ostream
> constructor, which would map to O_CLOEXEC on unix, and non-inheritable
> handles on windows. However, I encountered a discrepancy in the
> current behavior there.
>
> The current behavior on unix is to make the file descriptors
> inheritable (as that is the platform default). On windows, the current
> behavior is to make the handles *non*-inheritable (again, because of
> platform default). Obviously, if we add this flag, we will probably
> need to settle on a default behavior which is consistent across all
> platforms.
>
> Instead of going with the unix choice of inherit-by-default, I propose
> to go with the windows behavior where one has to explicitly opt into
> file handle inheritance. I believe that is the safer way, as when one
> thinks about creating a process, he usually knows which files/handles
> it wants it to inherit (so he can specify it), whereas if he is not
> creating a process, it does not matter whether it ends up being
> inheritable (and he can avoid it leaking by accident!).
>
> What do you think about this proposal? Does anyone know why this might
> be a bad idea?
I like the idea of non-inheritable by default.

Cheers,
Rafael

Making O_CLOEXEC the default sounds good.

On Tue, Jan 17, 2017 at 4:00 AM, Pavel Labath via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hello all,
>
> I am looking into using LLVM streams more extensively in LLDB (which
> currently rolls it's own stream classes). One of the things that's
> missing for me to be able to do that is the ability to open a file
> with the O_CLOEXEC flag (to prevent us leaking file descriptors into
> the debugged process).
>
> So I tried adding a F_NonInheritable flag to the raw_fd_ostream
> constructor, which would map to O_CLOEXEC on unix, and non-inheritable
> handles on windows. However, I encountered a discrepancy in the
> current behavior there.
>
> The current behavior on unix is to make the file descriptors
> inheritable (as that is the platform default). On windows, the current
> behavior is to make the handles *non*-inheritable (again, because of
> platform default). Obviously, if we add this flag, we will probably
> need to settle on a default behavior which is consistent across all
> platforms.
>
> Instead of going with the unix choice of inherit-by-default, I propose
> to go with the windows behavior where one has to explicitly opt into
> file handle inheritance. I believe that is the safer way, as when one
> thinks about creating a process, he usually knows which files/handles
> it wants it to inherit (so he can specify it), whereas if he is not
> creating a process, it does not matter whether it ends up being
> inheritable (and he can avoid it leaking by accident!).
>
> What do you think about this proposal? Does anyone know why this might
> be a bad idea?
>
> PS: Currently the test suite passes on windows&linux regardless of
> which way I set the default to be.
>
> cheers,
> pavel
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170117/e3b711d8/attachment.html>

Non-inheritable, no doubt.
The created process is getting loads of handles it does not know about. I
had to workaround similar situation before, spawning an
intermediate process just to get rid of inherited handles.


2017-01-17 19:07 GMT+02:00 Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org>:
> Making O_CLOEXEC the default sounds good.
>
> On Tue, Jan 17, 2017 at 4:00 AM, Pavel Labath via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hello all,
>>
>> I am looking into using LLVM streams more extensively in LLDB (which
>> currently rolls it's own stream classes). One of the things
that's
>> missing for me to be able to do that is the ability to open a file
>> with the O_CLOEXEC flag (to prevent us leaking file descriptors into
>> the debugged process).
>>
>> So I tried adding a F_NonInheritable flag to the raw_fd_ostream
>> constructor, which would map to O_CLOEXEC on unix, and non-inheritable
>> handles on windows. However, I encountered a discrepancy in the
>> current behavior there.
>>
>> The current behavior on unix is to make the file descriptors
>> inheritable (as that is the platform default). On windows, the current
>> behavior is to make the handles *non*-inheritable (again, because of
>> platform default). Obviously, if we add this flag, we will probably
>> need to settle on a default behavior which is consistent across all
>> platforms.
>>
>> Instead of going with the unix choice of inherit-by-default, I propose
>> to go with the windows behavior where one has to explicitly opt into
>> file handle inheritance. I believe that is the safer way, as when one
>> thinks about creating a process, he usually knows which files/handles
>> it wants it to inherit (so he can specify it), whereas if he is not
>> creating a process, it does not matter whether it ends up being
>> inheritable (and he can avoid it leaking by accident!).
>>
>> What do you think about this proposal? Does anyone know why this might
>> be a bad idea?
>>
>> PS: Currently the test suite passes on windows&linux regardless of
>> which way I set the default to be.
>>
>> cheers,
>> pavel
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170117/d026a9df/attachment.html>