Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005569OCamlOCaml otherlibspublic2012-03-30 23:492012-08-06 18:36
Reportergoswin 
Assigned To 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionduplicate 
PlatformOSLinuxOS Version
Product Version3.12.1 
Target VersionFixed in Version 
Summary0005569: missing Unix.dup_cloexec, Unix.get_cloexec and Unix.set_cloexec
DescriptionI'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).
TagsNo tags attached.
Attached Files

- Relationships
related to 0005256feedback Processes opened using Unix.open_process* inherit all opened file descriptors (including sockets) 
parent of 0005568resolved Unix.open_flag lacks O_CLOEXEC 

-  Notes
(0007255)
doligez (administrator)
2012-04-01 00:27

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?
(0007263)
till (reporter)
2012-04-02 16:20

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).
(0007268)
goswin (reporter)
2012-04-03 15:15

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.
(0007269)
xleroy (administrator)
2012-04-03 15:26

(This comment applies to PR#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 PR#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.
(0007273)
goswin (reporter)
2012-04-04 10:26

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.
(0007274)
xleroy (administrator)
2012-04-04 11:51

> And how would you close_all_except?

Please see the discussion in PR#5256. Or Google for "closefrom": you'll see that on many systems (e.g. Linux, BSD, Solaris), there are efficient ways to implement it.
(0007917)
xleroy (administrator)
2012-08-06 18:36

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

- Issue History
Date Modified Username Field Change
2012-03-30 23:49 goswin New Issue
2012-04-01 00:27 doligez Note Added: 0007255
2012-04-01 00:27 doligez Status new => feedback
2012-04-01 00:28 doligez Relationship added parent of 0005568
2012-04-02 16:20 till Note Added: 0007263
2012-04-03 15:15 goswin Note Added: 0007268
2012-04-03 15:15 goswin Status feedback => new
2012-04-03 15:18 xleroy Relationship added related to 0005256
2012-04-03 15:26 xleroy Note Added: 0007269
2012-04-03 15:26 xleroy Status new => feedback
2012-04-04 10:26 goswin Note Added: 0007273
2012-04-04 10:26 goswin Status feedback => new
2012-04-04 11:51 xleroy Note Added: 0007274
2012-04-04 11:51 xleroy Status new => feedback
2012-07-09 17:41 doligez Target Version => 4.01.0+dev
2012-07-31 13:36 doligez Target Version 4.01.0+dev => 4.00.1+dev
2012-08-06 18:36 xleroy Note Added: 0007917
2012-08-06 18:36 xleroy Status feedback => resolved
2012-08-06 18:36 xleroy Resolution open => duplicate
2012-08-06 18:36 xleroy Target Version 4.00.1+dev =>


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker