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

missing Unix.dup_cloexec, Unix.get_cloexec and Unix.set_cloexec #5569

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

missing Unix.dup_cloexec, Unix.get_cloexec and Unix.set_cloexec #5569

vicuna opened this issue Mar 30, 2012 · 7 comments

Comments

@vicuna
Copy link

vicuna commented Mar 30, 2012

Original bug ID: 5569
Reporter: goswin
Status: closed (set by @xavierleroy on 2015-12-11T18:07:34Z)
Resolution: duplicate
Priority: normal
Severity: minor
OS: Linux
Version: 3.12.1
Category: otherlibs
Related to: #5256
Parent of: #5568
Monitored by: mehdi @ygrek

Bug description

I'm missing functions to duplicating a filedescriptor with CLOEXEC flag already set, to query the state of CLOEXEC and to set the CLOEXEC flag. All three are part of fcntl().

   F_DUPFD_CLOEXEC (long; since Linux 2.6.24)
          As for F_DUPFD, but additionally set the close-on-exec flag  for
          the  duplicate  descriptor.  Specifying this flag permits a pro-
          gram to avoid an additional fcntl() F_SETFD operation to set the
          FD_CLOEXEC flag.  For an explanation of why this flag is useful,
          see the description of O_CLOEXEC in open(2).

File descriptor flags
The following commands manipulate the flags associated with a file
descriptor. Currently, only one such flag is defined: FD_CLOEXEC, the
close-on-exec flag. If the FD_CLOEXEC bit is 0, the file descriptor
will remain open across an execve(2), otherwise it will be closed.

   F_GETFD (void)
          Read the file descriptor flags; arg is ignored.

   F_SETFD (long)
          Set the file descriptor flags to the value specified by arg.

Additional information

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).

@vicuna
Copy link
Author

vicuna commented Mar 31, 2012

Comment author: @damiendoligez

For set_cloexec: there is Unix.set_close_on_exec.
For dup_cloexec and O_CLOEXEC: why not use a mutex?
For get_cloexec: do you really need this?

@vicuna
Copy link
Author

vicuna commented Apr 2, 2012

Comment author: till

Those sys call (dup2...) were added because there's a race condition: file descriptors can be captured across a fork between the open and set_cloexec function. I think it's a good idea to add those calls. They can be emulated in systems that do not have dup2,O_CLOEXEC... and we will still be avoiding some potential race conditions by not releasing the lock (at least for dup2).

@vicuna
Copy link
Author

vicuna commented Apr 3, 2012

Comment author: goswin

Sorry for missing the Unix.set_close_on_exec. That indeed was what I asked for.

But as till says the dup_cloexec is needed because dup + set_close_on_exec has a window of vulnerability where FDs can leak.

I also think a Unix.get_close_on_exec would be good. That way you can write an exec call that will raise an exception if you try to pass stdin/stdout/stderr from a FD that will be closed.

@vicuna
Copy link
Author

vicuna commented Apr 3, 2012

Comment author: @xavierleroy

(This comment applies to #5568 also.)

I see that O_CLOEXEC and F_DUPFD_CLOEXEC are specified in POSIX 2008, so there is hope that they will be available on platforms other than Linux, some day.

This said, there are still plenty of ways to create FDs without close-on-exec flag: the socket API comes to mind, but there's also the C libraries that we call from Caml and don't fully control. Looks like a losing battle to me.

I wonder whether the approach advocated in #5256 wouldn't be better overall: provide a close_all_except function, implemented the best we can, and use it as in fork() / close_all_except [stdin;stdout;stderr] / exec(). This would be a lot more robust, even though a race condition remains in the case where exec() is called directly by the program, without forking.

@vicuna
Copy link
Author

vicuna commented Apr 4, 2012

Comment author: goswin

And how would you close_all_except?

for i = 0 to max_int do
if not List.mem i list then close i;
done

There simply is no way to know what FDs are open. You can keep track of FDs opened by stdlib and otherlibs but FDs are also opened by other modules as you say. That is why I think it is esential to add the O_CLOEXEC flag so other modules can add support for O_CLOEXEC, too. It won't happen over night but if we don't start we will never get there.

As for the socket API:

   Since Linux 2.6.27, the type argument serves a second purpose: in addi-
   tion to specifying a socket type, it may include the bitwise OR of  any
   of the following values, to modify the behavior of socket():

   SOCK_NONBLOCK   Set  the  O_NONBLOCK  file  status flag on the new open
                   file description.  Using this flag saves extra calls to
                   fcntl(2) to achieve the same result.

   SOCK_CLOEXEC    Set the close-on-exec (FD_CLOEXEC) flag on the new file
                   descriptor.  See the description of the O_CLOEXEC  flag
                   in open(2) for reasons why this may be useful.

That (SOCK_CLOEXEC) just needs to be added to the Unix socket bindings. Maybe as optional argument.

@vicuna
Copy link
Author

vicuna commented Apr 4, 2012

Comment author: @xavierleroy

And how would you close_all_except?

Please see the discussion in #5256. Or Google for "closefrom": you'll see that on many systems (e.g. Linux, BSD, Solaris), there are efficient ways to implement it.

@vicuna
Copy link
Author

vicuna commented Aug 6, 2012

Comment author: @xavierleroy

This PR is subsumed by #5568 (for open O_CLOEXEC) and #5256 (for the more general discussion of closing the right FDs at exec()-time). Marking as duplicate.

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

No branches or pull requests

1 participant