Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unix.open_flag lacks O_CLOEXEC #5568

Closed
vicuna opened this issue Mar 30, 2012 · 6 comments
Closed

Unix.open_flag lacks O_CLOEXEC #5568

vicuna opened this issue Mar 30, 2012 · 6 comments

Comments

@vicuna
Copy link

@vicuna vicuna commented Mar 30, 2012

Original bug ID: 5568
Reporter: goswin
Status: closed (set by @xavierleroy on 2015-12-11T18:21:28Z)
Resolution: fixed
Priority: normal
Severity: minor
OS: Linux
Version: 3.12.1
Target version: 4.01.0+dev
Fixed in version: 4.01.0+dev
Category: otherlibs
Related to: #5256
Child of: #5569
Monitored by: mehdi @ygrek

Bug description

O_CLOEXEC (Since Linux 2.6.23)
Enable the close-on-exec flag for the new file descriptor.
Specifying this flag permits a program to avoid additional
fcntl(2) F_SETFD operations to set the FD_CLOEXEC flag. Addi-
tionally, use of this flag is essential in some multithreaded
programs since using a separate fcntl(2) F_SETFD operation to
set the FD_CLOEXEC flag does not suffice to avoid race condi-
tions where one thread opens a file descriptor at the same time
as another thread does a fork(2) plus execve(2).

There also some other flags missing but O_CLOEXEC has serious security implications.

@vicuna
Copy link
Author

@vicuna vicuna commented Mar 31, 2012

Comment author: @damiendoligez

See note in #5569.

@vicuna
Copy link
Author

@vicuna vicuna commented Apr 2, 2012

Comment author: gerd

I support this because it avoids a race condition between opening a file and executing a command in a multi-threaded app. If you don't open with O_CLOEXEC, it can happen that the exec() is done between open() and the fcntl() setting the flag, and the file remains open in the child process. See also #5256.

O_CLOEXEC is covered by POSIX.

Of course, this is only a partial solution to the mentioned problem, but this not our issue, but POSIX': The other syscalls creating file descriptors don't have an O_CLOEXEC flag.

@vicuna
Copy link
Author

@vicuna vicuna commented Apr 3, 2012

Comment author: goswin

I would rather consider this a parent of #5569, not a child.

Be advised that this wouldn't be just usefull for Unix.open_file. Other modules (extunix, lwt, ...) are using Unix.open_flags for syscalls that do accept O_CLOEXEC and would benefit from supporting a race-free CLOEXEC, too.

@vicuna
Copy link
Author

@vicuna vicuna commented Aug 1, 2013

Comment author: @xavierleroy

Add O_CLOEXEC flag to Unix.openfile, with emulation if not supported by the OS. Commits r13961 on version/4.01 and r13962 on trunk. Win32 implementation needs testing.

@vicuna
Copy link
Author

@vicuna vicuna commented Aug 3, 2013

Comment author: @avsm

Upstream patches to Lwt/Core have now been applied to OPAM, to fix the builds with the new flag.
https://github.com/OCamlPro/opam-repository/pull/936

@vicuna
Copy link
Author

@vicuna vicuna commented Aug 5, 2013

Comment author: goswin

I'm not sure if emulation is the right thing to do if the OS does not support the flag. You can't emulate it atomically and then you have the race condition back. It might be saver to fail if O_CLOEXEC is not supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant