Austin Group Defect Tracker

Aardvark Mark IV

Main | My View | View Issues | Change Log | Docs
  

Viewing Issue Simple Details [ Jump to Notes ] [ Issue History ] [ Print ]
ID Category Severity Type Date Submitted Last Update
0000149 [1003.1(2008)/Issue 7] System Interfaces Editorial Enhancement Request 2009-09-17 06:41 2013-04-16 13:06
Reporter tahonermann View Status public  
Assigned To ajosey
Priority normal Resolution Accepted As Marked  
Status Closed  
Name Tom Honermann
Organization Oracle
User Reference
Section fdwalk
Page Number N/A
Line Number N/A
Interp Status ---
Final Accepted Text Note: 0000648
Summary 0000149: Add fdwalk system interface
Description Code similar to the following is often used before calling one of the 'exec' family of functions to limit open file descriptors for the new process image. Many programs rely on this behavior for security reasons and to avoid exceeding limits on the number of open files.

int           fdesc;
struct rlimit rl;

/* Retrieve the soft limit for the maximum number of open files */
getrlimit(RLIMIT_NOFILE, &rl);

/* Set all file descriptors except stdin, stdout, and stderr to close-on-exec */
for(fdesc=3; fdesc < rl.rlim_cur; ++fdesc)
    {
    int old_flags = fcntl(fdesc, F_GETFD, 0);
    if(old_flags >= 0)
        fcntl(fdesc, F_SETFD, old_flags | FD_CLOEXEC);
    }


The above code functions properly but negatively impacts performance when running with a high soft limit. Consider a soft limit of 256 - at least 253 system calls to 'fcntl' will be made before calling one of the 'exec' functions to replace the process image.

Solaris includes a function named 'fdwalk' which can be used to achieve the same results as the above code with much less overhead. See the Solaris man page for more information: http://docs.sun.com/app/docs/doc/816-5168/fdwalk-3c?a=view. [^] Sample code to achieve the same results as the code above follows.

static int fdwalk_close_on_exec(void *unused, int fd)
{
    if (fd >= 3)
        {
        int old_flags = fcntl(fd, F_GETFD, 0);
        if(old_flags >= 0)
            fcntl(fd, F_SETFD, old_flags | FD_CLOEXEC);
        }
    return (0);
}

...

fdwalk(fdwalk_close_on_exec, NULL);


Use of 'fdwalk' is more efficient since 'fcntl' is only called for file descriptors that are actually open.
Desired Action Add fdwalk to the system interface section of the next standard.
Tags tc1-2008
Attached Files

- Relationships
related to 0000368Closedajosey Hidden file descriptors should be required to have the FD_CLOEXEC flag set and be closed when no longer needed. 
related to 0000370Closedajosey addclose should not cause posix_spawn to fail if closing an already closed fd 
related to 0000411Closedajosey adding atomic FD_CLOEXEC support 

-  Notes
(0000238)
msbrown (manager)
2009-09-24 15:45

 The rules for introducing new material into the standard are provided in austin-112r3 (http://www.opengroup.org/austin/docs/austin_112r3.txt). [^] [^]

The material introduced must have a copyright release from the original owner.

The text needs to be fully formed ... the exact words you would propose being added to the standard.
(0000648)
eblake (manager)
2011-01-13 16:17
edited on: 2011-01-13 16:59

 This was discussed again during the 16 Dec 2010 meeting. Group
consensus was that closefrom( ) cannot be standardized, because the
standard explicitly permits the creation of file descriptors that are
essential to the implementation (for example, posix_trace_*
functionality is often implemented in terms of an inherited fd), and
arbitrarily closing all file descriptors beyond a certain value could
break the trace and render the application potentially non-conforming.
There is no point in standardizing something while declaring that it
cannot be portably used (but implementations like BSD can continue to
provide it as an extension, as is true of any other non-standardized
interface).

The group also discussed that since fdwalk( ) has a callback function
that operates in user-space, it is necessarily non-atomic. It is
possible that closefrom() can be implemented in an atomic manner if it
is not based on top of fdwalk( ). Meanwhile, because fdwalk( ) allows
for discrimination on which file descriptors to close, it is possible
that fdwalk( ) might be usable in a manner that does not invalidate
implementation-specific file descriptors, so a future revision of the
standard may indeed add fdwalk( ), although no one in the meeting was
willing to draft a proposal for fdwalk( ) at this time (again,
implementations like Solaris can continue to provide it as an
extension).

Another suggestion was made for adding a new fcntl( ) command,
fcntl(fd,FD_NEXT,flags), which returns the next open file descriptor
after fd that satisfies the conditions of flags (where flags could
request the next fd in general, the next fd open for reading, the next
fd open for writing, etc.). It has the same limitations as
fdwalk( ) in being non-atomic, but is more efficient than iterating
through every possible file descriptor value and checking to see if
that value maps to an open file. However, unlike closefrom( ) and
fdwalk( ), it lacks prior implementation practice at this time.

A similar request for adding a command to the posix_spawn_file_actions
namespace for closing all file descriptors beyond a certain range has
the same drawbacks as calling closefrom( ) after fork( ). As a side
note, it was mentioned that behavior differs on whether a
posix_spawn_file_actions_addclose( ) that closes an already closed file
descriptor should make posix_spawn( ) fail or should be ignored as a
no-op, and 0000370 was raised to address that point; meanwhile, an
application can use an extra posix_spawn_file_actions_adddup2( ) to
guarantee that a file descriptor will not be already closed, but with
the same risk of non-compliant behavior if doing so overwrites a file
descriptor that must be inherited into the child for compliant behavior.

Revisiting the original complaint: multi-threaded applications are
already aware of the security implications of leaking unintended file
descriptors to a child process. The race is possible when one thread
creates a file descriptor, then another thread calls fork( ) and
exec( ) before the first can use fcntl( ) to mark the file descriptor
as close-on-exec. Since the second thread cannot know which file
descriptors might have suffered from this race, a common approach has
been to try and close all unknown file descriptors after the fork( )
and before the exec( ), rather than relying on the cloexec flag. But
as pointed out above, closing a file descriptor that might be in use
by the application leads to non-conforming results. Instead, if the
standard were to provide ways to guarantee that file descriptors are
atomically created as close-on-exec, then it is possible to write
conforming software that need not iterate over file descriptors after
fork( ), because there is no longer a close-on-exec race that could
leak unintended file descriptors, and by avoiding the iteration
altogether, there is no longer a risk of accidentally closing a file
descriptor essential to the implementation.

Therefore, the rest of this proposal seeks to document the problem
with closing arbitrary file descriptors, and a new bugid will be
opened to propose standardizing some recent interfaces and interface
extensions first appearing in Linux (new interfaces such as pipe2( ),
accept4( ), mkostemps( ), ..., and extensions like fopen(,"we")) to
guarantee the atomic creation of file descriptors with the cloexec
bit already set, as was already done in the standard with O_CLOEXEC
in open( ) and F_DUPFD_CLOEXEC in fcntl( ). See also 0000368 for
a related proposal to require CLOEXEC on hidden descriptors.

Make the following changes in TC1:

At line 22962 [XSH close APPLICATION USAGE], add a paragraph:

Implementations may use file descriptors that must be inherited into
child processes for the child process to remain conforming, such as
for message catalog or tracing purposes. Therefore, an application
that calls close( ) on an arbitrary integer risks non-conforming
behavior, and close( ) can only portably be used on file descriptor
values that the application has obtained through explicit actions, as
well as the three file descriptors corresponding to the standard file
streams. In multi-threaded parent applications, the practice of
calling close( ) in a loop after fork( ) and before an exec call in
order to avoid a race condition of leaking an unintended file
descriptor into a child process, is therefore unsafe, and the race
should instead be combatted by opening all file descriptors with the
FD_CLOEXEC bit set unless the file descriptor is intended to be
inherited across exec.

also, at line 22968 [RATIONALE], add a new paragraph:

The standard developers rejected a proposal to add closefrom( ) to the
standard. Because the standard permits implementations to use
inherited file descriptors as a means of providing a conforming
environment for the child process, it is not possible to standardize
an interface that closes arbitrary file descriptors above a certain
value while still guaranteeing a conforming environment.

At line 24917 [XSH dup2 APPLICATION USAGE], change:

The dup( ) and dup2( ) functions are redundant. Their services are
also provided by the fcntl( ) function. They have been included in
this volume of POSIX.1-2008 primarily for historical reasons, since
many existing applications use them.

to:

The dup( ) function is redundant. Its services are also provided by
the fcntl( ) function. It has been included in this volume of
POSIX.1-2008 primarily for historical reasons, since many existing
applications use it. On the other hand, the dup2( ) function
provides unique services, as no other interface is able to atomically
replace an existing file descriptor.

also, add a paragraph after line 24926:

Implementations may use file descriptors that must be inherited into
child processes for the child process to remain conforming, such as
for message catalog or tracing purposes. Therefore, an application
that calls dup2( ) with an arbitrary integer for filedes2 risks
non-conforming behavior, and dup2( ) can only portably be used to
overwrite file descriptor values that the application has obtained
through explicit actions, or for the three file descriptors
corresponding to the standard file streams. In order to avoid a race
condition of leaking an unintended file descriptor into a child
process, an application should consider opening all file descriptors
with the FD_CLOEXEC bit set unless the file descriptor is intended to
be inherited across exec.

At line 46906 [XSH posix_spawn_file_actions_addclose], add a
paragraph:

Implementations may use file descriptors that must be inherited into
child processes for the child process to remain conforming, such as
for message catalog or tracing purposes. Therefore, an application
that calls posix_spawn_file_actions_addclose( ) with an arbitrary
integer risks non-conforming behavior, and this function can only
portably be used to close file descriptor values that the application
has obtained through explicit actions, or for the three file
descriptors corresponding to the standard file streams. In order to
avoid a race condition of leaking an unintended file descriptor into
a child process, an application should consider opening all file
descriptors with the FD_CLOEXEC bit set unless the file descriptor is
intended to be inherited across exec.

At line 46997 [XSH posix_spawn_file_actions_adddup2], add a
paragraph:

Implementations may use file descriptors that must be inherited into
child processes for the child process to remain conforming, such as
for message catalog or tracing purposes. Therefore, an application
that calls posix_spawn_file_actions_adddup2( ) with an arbitrary
integer for newfiledes risks non-conforming behavior, and this
function can only portably be used to overwrite file descriptor values
that the application has obtained through explicit actions, or for the
three file descriptors corresponding to the standard file streams. In
order to avoid a race condition of leaking an unintended file
descriptor into a child process, an application should consider
opening all file descriptors with the FD_CLOEXEC bit set unless the
file descriptor is intended to be inherited across exec.
(0000652)
eblake (manager)
2011-01-20 16:38

 An interesting point to note is that with POSIX 2008, the definition of
OPEN_MAX was changed (see 0000371) to be the current cap on file
descriptor creation, but not necessarily on already-open descriptors. Thus,
iterating over file descriptors from 0 to OPEN_MAX may miss descriptors that
were created prior to a sysconf() call that reduced OPEN_MAX.

This argues that maybe the next revision of the standard should consider
adding fcntl(fd, F_NEXT), which would return the next open file
descriptor > fd (or -1 if fd is the maximum), and where the result can be
greater than OPEN_MAX, in order to assist in determining which file
descriptors exist in a more reliable manner than just iterating up to
OPEN_MAX.

- Issue History
Date Modified Username Field Change
2009-09-17 06:41 tahonermann New Issue
2009-09-17 06:41 tahonermann Status New => Under Review
2009-09-17 06:41 tahonermann Assigned To => ajosey
2009-09-17 06:41 tahonermann Name => Tom Honermann
2009-09-17 06:41 tahonermann Organization => Oracle
2009-09-17 06:41 tahonermann Section => fdwalk
2009-09-17 06:41 tahonermann Page Number => N/A
2009-09-17 06:41 tahonermann Line Number => N/A
2009-09-17 06:57 tahonermann Issue Monitored: tahonermann
2009-09-24 15:45 msbrown Note Added: 0000238
2009-09-24 15:46 msbrown Interp Status => ---
2009-09-24 15:46 msbrown Status Under Review => Resolved
2009-09-24 15:46 msbrown Resolution Open => Future Enhancement
2010-09-09 15:49 geoffclare Tag Attached: issue8
2011-01-13 16:17 eblake Note Added: 0000648
2011-01-13 16:21 eblake Note Edited: 0000648
2011-01-13 16:24 eblake Status Resolved => Under Review
2011-01-13 16:24 eblake Resolution Future Enhancement => Reopened
2011-01-13 16:36 eblake Note Edited: 0000648
2011-01-13 16:47 msbrown Relationship added related to 0000368
2011-01-13 16:49 eblake Note Edited: 0000648
2011-01-13 16:57 msbrown Status Under Review => Resolved
2011-01-13 16:57 msbrown Resolution Reopened => Accepted
2011-01-13 16:57 msbrown Final Accepted Text => Note: 0000648
2011-01-13 16:57 msbrown Resolution Accepted => Accepted As Marked
2011-01-13 16:58 Don Cragun Tag Detached: issue8
2011-01-13 16:59 Don Cragun Note Edited: 0000648
2011-01-13 17:01 geoffclare Tag Attached: tc1-2008
2011-01-13 17:05 Don Cragun Relationship added related to 0000370
2011-01-20 16:38 eblake Note Added: 0000652
2011-04-20 17:03 eblake Relationship added related to 0000411
2013-04-16 13:06 ajosey Status Resolved => Closed

Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker